mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Jens Axboe <axboe@kernel.dk>, Jan Kara <jack@suse.com>
Cc: Eryu Guan <eguan@redhat.com>,
	xfs@oss.sgi.com, axboe@fb.com, Dave Chinner <david@fromorbit.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-team@fb.com
Subject: [PATCH v2 block/for-linus] writeback: sync_inodes_sb() must write out I_DIRTY_TIME inodes and always call wait_sb_inodes()
Date: Tue, 25 Aug 2015 14:11:52 -0400	[thread overview]
Message-ID: <20150825181152.GA26785@mtj.duckdns.org> (raw)
In-Reply-To: <20150813224415.GG4496@mtj.duckdns.org>

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 broke sync_inodes_sb().

* sync_inodes_sb() must write out I_DIRTY_TIME inodes but I_DIRTY_TIME
  inodes dont't contribute to bdi/wb_has_dirty_io() tests and were
  being skipped over.

* inodes are taken off wb->b_dirty/io/more_io lists after writeback
  starts on them.  sync_inodes_sb() skipping wait_sb_inodes() when
  bdi_has_dirty_io() breaks it by making it return while writebacks
  are in-flight.

This patch fixes the breakages 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() and wait_sb_inodes().

* Making bdi_split_work_to_wbs() consider the b_dirty_time list for
  WB_SYNC_ALL writebacks.

Kudos to Eryu, Dave and Jan for tracking down the issue.

Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: e79729123f63 ("writeback: don't issue wb_writeback_work if clean")
Link: http://lkml.kernel.org/g/20150812101204.GE17933@dhcp-13-216.nay.redhat.com
Reported-and-bisected-by: Eryu Guan <eguan@redhat.com>
Cc: Dave Chinner <david@fromorbit.com>
Cc: Jan Kara <jack@suse.com>
Cc: Ted Ts'o <tytso@google.com>
---
 fs/fs-writeback.c |   22 +++++++++++++---------
 1 file changed, 13 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,12 @@ 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)
+	/*
+	 * Can't skip on !bdi_has_dirty() because we should wait for !dirty
+	 * inodes under writeback and I_DIRTY_TIME inodes ignored by
+	 * bdi_has_dirty() need to be written out too.
+	 */
+	if (bdi == &noop_backing_dev_info)
 		return;
 	WARN_ON(!rwsem_is_locked(&sb->s_umount));
 

  parent reply	other threads:[~2015-08-25 18:11 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20150812101204.GE17933@dhcp-13-216.nay.redhat.com>
     [not found] ` <20150813004435.GN3902@dastard>
2015-08-13 22:44   ` [PATCH block/for-linus] writeback: fix syncing of I_DIRTY_TIME inodes Tejun Heo
2015-08-14 11:14     ` Jan Kara
2015-08-14 15:14       ` Damien Wyart
2015-08-17 20:00         ` Tejun Heo
2015-08-18  5:33           ` Damien Wyart
2015-08-17 20:02       ` Tejun Heo
2015-08-18  9:16         ` Jan Kara
2015-08-18 17:47           ` Tejun Heo
2015-08-18 19:54             ` Tejun Heo
2015-08-18 21:56               ` Dave Chinner
2015-08-20  6:12                 ` Eryu Guan
2015-08-20 14:36                   ` Eryu Guan
2015-08-20 14:37                     ` Eryu Guan
2015-08-20 16:55                       ` Tejun Heo
2015-08-20 23:04                         ` Dave Chinner
2015-08-24 18:10                           ` Tejun Heo
2015-08-24 22:27                             ` Dave Chinner
2015-08-24 22:53                               ` Tejun Heo
2015-08-21 10:20                 ` Eryu Guan
2015-08-22  0:30                   ` Dave Chinner
2015-08-22  4:46                     ` Eryu Guan
2015-08-24  1:11                       ` Dave Chinner
2015-08-24  3:18                         ` Eryu Guan
2015-08-24  6:24                           ` Dave Chinner
2015-08-24  8:34                             ` Eryu Guan
2015-08-24  8:55                               ` Dave Chinner
2015-08-24  9:19                             ` Jan Kara
2015-08-24 14:51                               ` Tejun Heo
2015-08-24 17:11                                 ` Tejun Heo
2015-08-24 19:08                                   ` Jan Kara
2015-08-24 19:32                                     ` Tejun Heo
2015-08-24 21:09                                       ` Jan Kara
2015-08-24 21:45                                         ` Tejun Heo
2015-08-24 22:54                                           ` Tejun Heo
2015-08-24 22:57                                           ` Dave Chinner
2015-08-25 18:11     ` Tejun Heo [this message]
2015-08-25 20:37       ` [PATCH v2 block/for-linus] writeback: sync_inodes_sb() must write out I_DIRTY_TIME inodes and always call wait_sb_inodes() Jens Axboe
2015-08-26  9:00       ` 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=20150825181152.GA26785@mtj.duckdns.org \
    --to=tj@kernel.org \
    --cc=axboe@fb.com \
    --cc=axboe@kernel.dk \
    --cc=david@fromorbit.com \
    --cc=eguan@redhat.com \
    --cc=jack@suse.com \
    --cc=kernel-team@fb.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=xfs@oss.sgi.com \
    /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

all inboxes | Powered by JetHome®