Re: [PATCH block/for-linus] writeback: fix syncing of I_DIRTY_TIME inodes
From: Jan Kara <jack@suse.cz>
Date: 2015-08-14 11:14:09
Also in:
linux-fsdevel, lkml
Hello, On Thu 13-08-15 18:44:15, Tejun Heo wrote:
e79729123f63 ("writeback: don't issue wb_writeback_work if clean")
updated writeback path to avoid kicking writeback work items if there
are no inodes to be written out; unfortunately, the avoidance logic
was too aggressive and made sync_inodes_sb() skip I_DIRTY_TIME inodes.
This patch fixes the breakage by
* Removing bdi_has_dirty_io() shortcut from bdi_split_work_to_wbs().
The callers are already testing the condition.
* Removing bdi_has_dirty_io() shortcut from sync_inodes_sb() so that
it always calls into bdi_split_work_to_wbs().
* Making bdi_split_work_to_wbs() consider the b_dirty_time list for
WB_SYNC_ALL writebacks.
Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: e79729123f63 ("writeback: don't issue wb_writeback_work if clean")
Cc: Ted Ts'o <redacted>
Cc: Jan Kara <jack@suse.com>So the patch looks good to me. But the fact that is fixes Eryu's problem means there is something fishy going on. Either inodes get wrongly attached to b_dirty_time list or bdi_has_dirty_io() somehow misbehaves only temporarily and we don't catch it with the debug patch. Can we add a test to wb_has_dirty_io() to also check whether it matches bdi_has_dirty_io()? Since Eryu doesn't use lazytime (I assume, Eryu, please speak up if you do), we could also warn if b_dirty_time lists get non-empty. Hmm? Honza
quoted hunk ↗ jump to hunk
--- Hello, So, this fixes I_DIRTY_TIME syncing problem for ext4 but AFAICS xfs doesn't even use the generic inode metadata writeback path, so this most likely won't do anything for the originally reported problem. I'll post another patch for debugging. Thanks. fs/fs-writeback.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)--- a/fs/fs-writeback.c +++ b/fs/fs-writeback.c@@ -844,14 +844,15 @@ static void bdi_split_work_to_wbs(struct struct wb_iter iter; might_sleep(); - - if (!bdi_has_dirty_io(bdi)) - return; restart: rcu_read_lock(); bdi_for_each_wb(wb, bdi, &iter, next_blkcg_id) { - if (!wb_has_dirty_io(wb) || - (skip_if_busy && writeback_in_progress(wb))) + /* SYNC_ALL writes out I_DIRTY_TIME too */ + if (!wb_has_dirty_io(wb) && + (base_work->sync_mode == WB_SYNC_NONE || + list_empty(&wb->b_dirty_time))) + continue; + if (skip_if_busy && writeback_in_progress(wb)) continue; base_work->nr_pages = wb_split_bdi_pages(wb, nr_pages);@@ -899,8 +900,7 @@ static void bdi_split_work_to_wbs(struct { might_sleep(); - if (bdi_has_dirty_io(bdi) && - (!skip_if_busy || !writeback_in_progress(&bdi->wb))) { + if (!skip_if_busy || !writeback_in_progress(&bdi->wb)) { base_work->auto_free = 0; base_work->single_wait = 0; base_work->single_done = 0;@@ -2275,8 +2275,8 @@ void sync_inodes_sb(struct super_block * }; struct backing_dev_info *bdi = sb->s_bdi; - /* Nothing to do? */ - if (!bdi_has_dirty_io(bdi) || bdi == &noop_backing_dev_info) + /* bdi_has_dirty() ignores I_DIRTY_TIME but we can't, always kick wbs */ + if (bdi == &noop_backing_dev_info) return; WARN_ON(!rwsem_is_locked(&sb->s_umount));
-- Jan Kara [off-list ref] SUSE Labs, CR _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs