linux-kernel - [PATCH] bdi_sync_writeback should WB_SYNC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090925141014.GB15853@think>
Date:	Fri, 25 Sep 2009 10:10:14 -0400
From:	Chris Mason <chris.mason@...cle.com>
To:	linux-kernel@...r.kernel.org, jens.axboe@...cle.com, jack@...e.cz
Subject: [PATCH] bdi_sync_writeback should WB_SYNC_NONE first

At unmount time, we do writeback in two stages.  First we call
sync_filesystems with wait == 0, and then we call it with wait == 1.

When wait == 1, WB_SYNC_ALL is used.  WB_SYNC_ALL will pass wait == 1 to
the filesystem write_inode function if the inode was I_DIRTY_SYNC, and
the filesystem write_inode function is then expected to commit the
running transaction.

The new bdi threads try to keep this two stage writeback, but the
problem is that they do it by calling bdi_writeback_all, which just
kicks a few procs here and there and returns.

The end result is that btrfs is getting stuck in a loop where it commits
the transaction for every dirty inode, and unmount takes forever.

This patch is one possible fix. It just changes bdi_sync_writeback to
always do a WB_SYNC_NONE run synchronously before the WB_SYNC_ALL run.
I'm not sure I've got the bdi calling conventions right, but we need
something along these lines.

We could also make a synchronous form of bdi_writeback_all, but unmount
really isn't the common case so I think this patch is sufficient.

Signed-off-by: Chris Mason <chris.mason@...cle.com>

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 8e1e5e1..27f8e0e 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -225,7 +225,7 @@ static void bdi_sync_writeback(struct backing_dev_info *bdi,
 {
 	struct wb_writeback_args args = {
 		.sb		= sb,
-		.sync_mode	= WB_SYNC_ALL,
+		.sync_mode	= WB_SYNC_NONE,
 		.nr_pages	= LONG_MAX,
 		.range_cyclic	= 0,
 	};
@@ -236,6 +236,13 @@ static void bdi_sync_writeback(struct backing_dev_info *bdi,

 	bdi_queue_work(bdi, &work);
 	bdi_wait_on_work_clear(&work);
+
+	args.sync_mode = WB_SYNC_ALL;
+	args.nr_pages = LONG_MAX;
+
+	work.state = WS_USED | WS_ONSTACK;
+	bdi_queue_work(bdi, &work);
+	bdi_wait_on_work_clear(&work);
 }

 /**
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/