From: Jan Kara <jack@suse.cz>
To: Zheng Liu <gnehzuil.liu@gmail.com>
Cc: Jan Kara <jack@suse.cz>, Ted Tso <tytso@mit.edu>,
linux-ext4@vger.kernel.org
Subject: Re: [PATCH 21/29] ext4: Split extent conversion lists to reserved & unreserved parts
Date: Wed, 8 May 2013 13:23:55 +0200 [thread overview]
Message-ID: <20130508112355.GC30550@quack.suse.cz> (raw)
In-Reply-To: <20130508070335.GC20599@gmail.com>
On Wed 08-05-13 15:03:35, Zheng Liu wrote:
> On Mon, Apr 08, 2013 at 11:32:26PM +0200, Jan Kara wrote:
> > Now that we have extent conversions with reserved transaction, we have
> > to prevent extent conversions without reserved transaction (from DIO
> > code) to block these (as that would effectively void any transaction
> > reservation we did). So split lists, work items, and work queues to
> > reserved and unreserved parts.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
>
> I got a build error that looks like this.
>
> fs/ext4/page-io.c: In function ‘ext4_ioend_shutdown’:
> fs/ext4/page-io.c:60: error: ‘struct ext4_inode_info’ has no member
> named ‘i_unwritten_work’
>
> I guess the reason is that when this patch set is sent out,
> ext4_io_end_shutdown() hasn't be added. So please add the code
> like this. Otherwise the patch looks good to me.
> Reviewed-by: Zheng Liu <wenqing.lz@taobao.com>
Yeah, I've already rebased the series on top of current Linus's tree and
I've notice this problem as well. It should be fixed by now. I didn't post
the rebased series yet because I'm looking into some xfstests failures I
hit when testing it...
Honza
>
> Regards,
> - Zheng
>
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 3fea79e..f9ecc4f 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -57,8 +57,10 @@ void ext4_ioend_shutdown(struct inode *inode)
> * We need to make sure the work structure is finished being
> * used before we let the inode get destroyed.
> */
> - if (work_pending(&EXT4_I(inode)->i_unwritten_work))
> - cancel_work_sync(&EXT4_I(inode)->i_unwritten_work);
> + if (work_pending(&EXT4_I(inode)->i_rsv_conversion_work))
> + cancel_work_sync(&EXT4_I(inode)->i_rsv_conversion_work);
> + if (work_pending(&EXT4_I(inode)->i_unrsv_conversion_work))
> + cancel_work_sync(&EXT4_I(inode)->i_unrsv_conversion_work);
> }
>
> static void ext4_release_io_end(ext4_io_end_t *io_end)
>
> > ---
> > fs/ext4/ext4.h | 25 +++++++++++++++++-----
> > fs/ext4/page-io.c | 59 ++++++++++++++++++++++++++++++++++------------------
> > fs/ext4/super.c | 38 ++++++++++++++++++++++++---------
> > 3 files changed, 84 insertions(+), 38 deletions(-)
> >
> > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> > index 65adf0d..a594a94 100644
> > --- a/fs/ext4/ext4.h
> > +++ b/fs/ext4/ext4.h
> > @@ -889,12 +889,22 @@ struct ext4_inode_info {
> > qsize_t i_reserved_quota;
> > #endif
> >
> > - /* completed IOs that might need unwritten extents handling */
> > - struct list_head i_completed_io_list;
> > + /* Lock protecting lists below */
> > spinlock_t i_completed_io_lock;
> > + /*
> > + * Completed IOs that need unwritten extents handling and have
> > + * transaction reserved
> > + */
> > + struct list_head i_rsv_conversion_list;
> > + /*
> > + * Completed IOs that need unwritten extents handling and don't have
> > + * transaction reserved
> > + */
> > + struct list_head i_unrsv_conversion_list;
> > atomic_t i_ioend_count; /* Number of outstanding io_end structs */
> > atomic_t i_unwritten; /* Nr. of inflight conversions pending */
> > - struct work_struct i_unwritten_work; /* deferred extent conversion */
> > + struct work_struct i_rsv_conversion_work;
> > + struct work_struct i_unrsv_conversion_work;
> >
> > spinlock_t i_block_reservation_lock;
> >
> > @@ -1257,8 +1267,10 @@ struct ext4_sb_info {
> > struct flex_groups *s_flex_groups;
> > ext4_group_t s_flex_groups_allocated;
> >
> > - /* workqueue for dio unwritten */
> > - struct workqueue_struct *dio_unwritten_wq;
> > + /* workqueue for unreserved extent convertions (dio) */
> > + struct workqueue_struct *unrsv_conversion_wq;
> > + /* workqueue for reserved extent conversions (buffered io) */
> > + struct workqueue_struct *rsv_conversion_wq;
> >
> > /* timer for periodic error stats printing */
> > struct timer_list s_err_report;
> > @@ -2599,7 +2611,8 @@ extern int ext4_put_io_end(ext4_io_end_t *io_end);
> > extern void ext4_put_io_end_defer(ext4_io_end_t *io_end);
> > extern void ext4_io_submit_init(struct ext4_io_submit *io,
> > struct writeback_control *wbc);
> > -extern void ext4_end_io_work(struct work_struct *work);
> > +extern void ext4_end_io_rsv_work(struct work_struct *work);
> > +extern void ext4_end_io_unrsv_work(struct work_struct *work);
> > extern void ext4_io_submit(struct ext4_io_submit *io);
> > extern int ext4_bio_write_page(struct ext4_io_submit *io,
> > struct page *page,
> > diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> > index e8ee4da..8bff3b3 100644
> > --- a/fs/ext4/page-io.c
> > +++ b/fs/ext4/page-io.c
> > @@ -103,20 +103,17 @@ static int ext4_end_io(ext4_io_end_t *io)
> > return ret;
> > }
> >
> > -static void dump_completed_IO(struct inode *inode)
> > +static void dump_completed_IO(struct inode *inode, struct list_head *head)
> > {
> > #ifdef EXT4FS_DEBUG
> > struct list_head *cur, *before, *after;
> > ext4_io_end_t *io, *io0, *io1;
> >
> > - if (list_empty(&EXT4_I(inode)->i_completed_io_list)) {
> > - ext4_debug("inode %lu completed_io list is empty\n",
> > - inode->i_ino);
> > + if (list_empty(head))
> > return;
> > - }
> >
> > - ext4_debug("Dump inode %lu completed_io list\n", inode->i_ino);
> > - list_for_each_entry(io, &EXT4_I(inode)->i_completed_io_list, list) {
> > + ext4_debug("Dump inode %lu completed io list\n", inode->i_ino);
> > + list_for_each_entry(io, head, list) {
> > cur = &io->list;
> > before = cur->prev;
> > io0 = container_of(before, ext4_io_end_t, list);
> > @@ -137,16 +134,23 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)
> > unsigned long flags;
> >
> > BUG_ON(!(io_end->flag & EXT4_IO_END_UNWRITTEN));
> > - wq = EXT4_SB(io_end->inode->i_sb)->dio_unwritten_wq;
> > -
> > spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> > - if (list_empty(&ei->i_completed_io_list))
> > - queue_work(wq, &ei->i_unwritten_work);
> > - list_add_tail(&io_end->list, &ei->i_completed_io_list);
> > + if (io_end->handle) {
> > + wq = EXT4_SB(io_end->inode->i_sb)->rsv_conversion_wq;
> > + if (list_empty(&ei->i_rsv_conversion_list))
> > + queue_work(wq, &ei->i_rsv_conversion_work);
> > + list_add_tail(&io_end->list, &ei->i_rsv_conversion_list);
> > + } else {
> > + wq = EXT4_SB(io_end->inode->i_sb)->unrsv_conversion_wq;
> > + if (list_empty(&ei->i_unrsv_conversion_list))
> > + queue_work(wq, &ei->i_unrsv_conversion_work);
> > + list_add_tail(&io_end->list, &ei->i_unrsv_conversion_list);
> > + }
> > spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
> > }
> >
> > -static int ext4_do_flush_completed_IO(struct inode *inode)
> > +static int ext4_do_flush_completed_IO(struct inode *inode,
> > + struct list_head *head)
> > {
> > ext4_io_end_t *io;
> > struct list_head unwritten;
> > @@ -155,8 +159,8 @@ static int ext4_do_flush_completed_IO(struct inode *inode)
> > int err, ret = 0;
> >
> > spin_lock_irqsave(&ei->i_completed_io_lock, flags);
> > - dump_completed_IO(inode);
> > - list_replace_init(&ei->i_completed_io_list, &unwritten);
> > + dump_completed_IO(inode, head);
> > + list_replace_init(head, &unwritten);
> > spin_unlock_irqrestore(&ei->i_completed_io_lock, flags);
> >
> > while (!list_empty(&unwritten)) {
> > @@ -172,21 +176,34 @@ static int ext4_do_flush_completed_IO(struct inode *inode)
> > }
> >
> > /*
> > - * work on completed aio dio IO, to convert unwritten extents to extents
> > + * work on completed IO, to convert unwritten extents to extents
> > */
> > -void ext4_end_io_work(struct work_struct *work)
> > +void ext4_end_io_rsv_work(struct work_struct *work)
> > {
> > struct ext4_inode_info *ei = container_of(work, struct ext4_inode_info,
> > - i_unwritten_work);
> > - ext4_do_flush_completed_IO(&ei->vfs_inode);
> > + i_rsv_conversion_work);
> > + ext4_do_flush_completed_IO(&ei->vfs_inode, &ei->i_rsv_conversion_list);
> > +}
> > +
> > +void ext4_end_io_unrsv_work(struct work_struct *work)
> > +{
> > + struct ext4_inode_info *ei = container_of(work, struct ext4_inode_info,
> > + i_unrsv_conversion_work);
> > + ext4_do_flush_completed_IO(&ei->vfs_inode, &ei->i_unrsv_conversion_list);
> > }
> >
> > int ext4_flush_unwritten_io(struct inode *inode)
> > {
> > - int ret;
> > + int ret, err;
> > +
> > WARN_ON_ONCE(!mutex_is_locked(&inode->i_mutex) &&
> > !(inode->i_state & I_FREEING));
> > - ret = ext4_do_flush_completed_IO(inode);
> > + ret = ext4_do_flush_completed_IO(inode,
> > + &EXT4_I(inode)->i_rsv_conversion_list);
> > + err = ext4_do_flush_completed_IO(inode,
> > + &EXT4_I(inode)->i_unrsv_conversion_list);
> > + if (!ret)
> > + ret = err;
> > ext4_unwritten_wait(inode);
> > return ret;
> > }
> > diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> > index 09ff724..916c4fb 100644
> > --- a/fs/ext4/super.c
> > +++ b/fs/ext4/super.c
> > @@ -747,8 +747,10 @@ static void ext4_put_super(struct super_block *sb)
> > ext4_unregister_li_request(sb);
> > dquot_disable(sb, -1, DQUOT_USAGE_ENABLED | DQUOT_LIMITS_ENABLED);
> >
> > - flush_workqueue(sbi->dio_unwritten_wq);
> > - destroy_workqueue(sbi->dio_unwritten_wq);
> > + flush_workqueue(sbi->unrsv_conversion_wq);
> > + flush_workqueue(sbi->rsv_conversion_wq);
> > + destroy_workqueue(sbi->unrsv_conversion_wq);
> > + destroy_workqueue(sbi->rsv_conversion_wq);
> >
> > if (sbi->s_journal) {
> > err = jbd2_journal_destroy(sbi->s_journal);
> > @@ -856,13 +858,15 @@ static struct inode *ext4_alloc_inode(struct super_block *sb)
> > ei->i_reserved_quota = 0;
> > #endif
> > ei->jinode = NULL;
> > - INIT_LIST_HEAD(&ei->i_completed_io_list);
> > + INIT_LIST_HEAD(&ei->i_rsv_conversion_list);
> > + INIT_LIST_HEAD(&ei->i_unrsv_conversion_list);
> > spin_lock_init(&ei->i_completed_io_lock);
> > ei->i_sync_tid = 0;
> > ei->i_datasync_tid = 0;
> > atomic_set(&ei->i_ioend_count, 0);
> > atomic_set(&ei->i_unwritten, 0);
> > - INIT_WORK(&ei->i_unwritten_work, ext4_end_io_work);
> > + INIT_WORK(&ei->i_rsv_conversion_work, ext4_end_io_rsv_work);
> > + INIT_WORK(&ei->i_unrsv_conversion_work, ext4_end_io_unrsv_work);
> >
> > return &ei->vfs_inode;
> > }
> > @@ -3867,12 +3871,20 @@ no_journal:
> > * The maximum number of concurrent works can be high and
> > * concurrency isn't really necessary. Limit it to 1.
> > */
> > - EXT4_SB(sb)->dio_unwritten_wq =
> > - alloc_workqueue("ext4-dio-unwritten", WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
> > - if (!EXT4_SB(sb)->dio_unwritten_wq) {
> > - printk(KERN_ERR "EXT4-fs: failed to create DIO workqueue\n");
> > + EXT4_SB(sb)->rsv_conversion_wq =
> > + alloc_workqueue("ext4-rsv-conversion", WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
> > + if (!EXT4_SB(sb)->rsv_conversion_wq) {
> > + printk(KERN_ERR "EXT4-fs: failed to create workqueue\n");
> > ret = -ENOMEM;
> > - goto failed_mount_wq;
> > + goto failed_mount4;
> > + }
> > +
> > + EXT4_SB(sb)->unrsv_conversion_wq =
> > + alloc_workqueue("ext4-unrsv-conversion", WQ_MEM_RECLAIM | WQ_UNBOUND, 1);
> > + if (!EXT4_SB(sb)->unrsv_conversion_wq) {
> > + printk(KERN_ERR "EXT4-fs: failed to create workqueue\n");
> > + ret = -ENOMEM;
> > + goto failed_mount4;
> > }
> >
> > /*
> > @@ -4019,7 +4031,10 @@ failed_mount4a:
> > sb->s_root = NULL;
> > failed_mount4:
> > ext4_msg(sb, KERN_ERR, "mount failed");
> > - destroy_workqueue(EXT4_SB(sb)->dio_unwritten_wq);
> > + if (EXT4_SB(sb)->rsv_conversion_wq)
> > + destroy_workqueue(EXT4_SB(sb)->rsv_conversion_wq);
> > + if (EXT4_SB(sb)->unrsv_conversion_wq)
> > + destroy_workqueue(EXT4_SB(sb)->unrsv_conversion_wq);
> > failed_mount_wq:
> > if (sbi->s_journal) {
> > jbd2_journal_destroy(sbi->s_journal);
> > @@ -4464,7 +4479,8 @@ static int ext4_sync_fs(struct super_block *sb, int wait)
> > struct ext4_sb_info *sbi = EXT4_SB(sb);
> >
> > trace_ext4_sync_fs(sb, wait);
> > - flush_workqueue(sbi->dio_unwritten_wq);
> > + flush_workqueue(sbi->rsv_conversion_wq);
> > + flush_workqueue(sbi->unrsv_conversion_wq);
> > /*
> > * Writeback quota in non-journalled quota case - journalled quota has
> > * no dirty dquots
> > --
> > 1.7.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2013-05-08 11:23 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-08 21:32 [PATCH 00/22 v1] Fixes and improvements in ext4 writeback path Jan Kara
2013-04-08 21:32 ` [PATCH 01/29] ext4: Make ext4_bio_write_page() use BH_Async_Write flags instead page pointers from ext4_io_end Jan Kara
2013-04-10 18:05 ` Dmitry Monakhov
2013-04-11 13:38 ` Zheng Liu
2013-04-12 3:50 ` Theodore Ts'o
2013-04-08 21:32 ` [PATCH 02/29] ext4: Use io_end for multiple bios Jan Kara
2013-04-11 5:10 ` Dmitry Monakhov
2013-04-11 14:04 ` Zheng Liu
2013-04-12 3:55 ` Theodore Ts'o
2013-04-08 21:32 ` [PATCH 03/29] ext4: Clear buffer_uninit flag when submitting IO Jan Kara
2013-04-11 14:08 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 04/29] jbd2: Reduce journal_head size Jan Kara
2013-04-11 14:10 ` Zheng Liu
2013-04-12 4:04 ` Theodore Ts'o
2013-04-08 21:32 ` [PATCH 05/29] jbd2: Don't create journal_head for temporary journal buffers Jan Kara
2013-04-12 8:01 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 06/29] jbd2: Remove journal_head from descriptor buffers Jan Kara
2013-04-12 8:10 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 07/29] jbd2: Refine waiting for shadow buffers Jan Kara
2013-05-03 14:16 ` Zheng Liu
2013-05-03 20:44 ` Jan Kara
2013-04-08 21:32 ` [PATCH 08/29] jbd2: Remove outdated comment Jan Kara
2013-05-03 14:20 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 09/29] jbd2: Cleanup needed free block estimates when starting a transaction Jan Kara
2013-05-05 8:17 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 10/29] jbd2: Fix race in t_outstanding_credits update in jbd2_journal_extend() Jan Kara
2013-05-05 8:37 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 11/29] jbd2: Remove unused waitqueues Jan Kara
2013-05-05 8:41 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 12/29] jbd2: Transaction reservation support Jan Kara
2013-05-05 9:39 ` Zheng Liu
2013-05-06 12:49 ` Jan Kara
2013-05-07 5:22 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 13/29] ext4: Provide wrappers for transaction reservation calls Jan Kara
2013-05-05 11:51 ` Zheng Liu
2013-05-05 11:58 ` Zheng Liu
2013-05-06 12:51 ` Jan Kara
2013-04-08 21:32 ` [PATCH 14/29] ext4: Stop messing with nr_to_write in ext4_da_writepages() Jan Kara
2013-05-05 12:40 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 15/29] ext4: Deprecate max_writeback_mb_bump sysfs attribute Jan Kara
2013-05-05 12:47 ` Zheng Liu
2013-05-06 12:55 ` Jan Kara
2013-04-08 21:32 ` [PATCH 16/29] ext4: Improve writepage credit estimate for files with indirect blocks Jan Kara
2013-05-07 5:39 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 17/29] ext4: Better estimate credits needed for ext4_da_writepages() Jan Kara
2013-05-07 6:33 ` Zheng Liu
2013-05-07 14:17 ` Jan Kara
2013-04-08 21:32 ` [PATCH 18/29] ext4: Restructure writeback path Jan Kara
2013-05-08 3:48 ` Zheng Liu
2013-05-08 11:20 ` Jan Kara
2013-04-08 21:32 ` [PATCH 19/29] ext4: Remove buffer_uninit handling Jan Kara
2013-05-08 6:56 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 20/29] ext4: Use transaction reservation for extent conversion in ext4_end_io Jan Kara
2013-05-08 6:57 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 21/29] ext4: Split extent conversion lists to reserved & unreserved parts Jan Kara
2013-05-08 7:03 ` Zheng Liu
2013-05-08 11:23 ` Jan Kara [this message]
2013-05-08 11:49 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 22/29] ext4: Defer clearing of PageWriteback after extent conversion Jan Kara
2013-05-08 7:08 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 23/29] ext4: Protect extent conversion after DIO with i_dio_count Jan Kara
2013-05-08 7:08 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 24/29] ext4: Remove wait for unwritten extent conversion from ext4_ext_truncate() Jan Kara
2013-05-08 7:35 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 25/29] ext4: Use generic_file_fsync() in ext4_file_fsync() in nojournal mode Jan Kara
2013-05-08 7:37 ` Zheng Liu
2013-05-08 11:29 ` Jan Kara
2013-04-08 21:32 ` [PATCH 26/29] ext4: Remove i_mutex from ext4_file_sync() Jan Kara
2013-05-08 7:41 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 27/29] ext4: Remove wait for unwritten extents in ext4_ind_direct_IO() Jan Kara
2013-05-08 7:55 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 28/29] ext4: Don't wait for extent conversion in ext4_ext_punch_hole() Jan Kara
2013-05-08 7:56 ` Zheng Liu
2013-04-08 21:32 ` [PATCH 29/29] ext4: Remove ext4_ioend_wait() Jan Kara
2013-05-08 7:57 ` Zheng Liu
2013-05-08 11:32 ` Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130508112355.GC30550@quack.suse.cz \
--to=jack@suse.cz \
--cc=gnehzuil.liu@gmail.com \
--cc=linux-ext4@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).