linux-kernel - Re: [PATCH 02/17] writeback: update dirtied_when for synced inode to prevent livelock

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-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 May 2011 10:14:21 +0800
From:	Wu Fengguang <fengguang.wu@...el.com>
To:	Jan Kara <jack@...e.cz>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Dave Chinner <david@...morbit.com>,
	Christoph Hellwig <hch@...radead.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 02/17] writeback: update dirtied_when for synced inode
 to prevent livelock

On Sat, May 07, 2011 at 12:33:12AM +0800, Jan Kara wrote:
> On Fri 06-05-11 11:08:23, Wu Fengguang wrote:
> > With the more aggressive "keep writeback as long as we wrote something"
> > logic in wb_writeback(), the "use LONG_MAX .nr_to_write" trick in commit
> > b9543dac5bbc ("writeback: avoid livelocking WB_SYNC_ALL writeback") is
> > no longer enough to stop sync livelock.
> > 
> > The fix is to explicitly update .dirtied_when on synced inodes, so that
> > they are no longer considered for writeback in the next round.
>   The changelog doesn't make sense now when the patch is in the second
> place in the series... Also as we discussed in the previous iteration of
> the patches, I thought you'll move the condition before mapping_tagged()
> test. I.e. the code would be (comment updated):
> 	if (!(inode->i_state & I_FREEING)) {
> 		/*
> 		 * Sync livelock prevention: Each inode is tagged and
> 		 * synced in one shot. So we can unconditionally update its
> 		 * dirty time to prevent syncing it again. Note that time
> 		 * ordering of b_dirty list will be kept because the
> 		 * following code either removes the inode from b_dirty
> 		 * or calls redirty_tail().
> 		 */
> 		if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_sync)
> 			inode->dirtied_when = jiffies;
> 		if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {

It looks good, thanks!  Here is the updated patch with your
reviewed-by. Note that I retains a modified paragraph in the changelog
to clarify its relationship to the other two patches.

Thanks,
Fengguang
---
Subject: writeback: update dirtied_when for synced inode to prevent livelock
Date: Wed Apr 27 19:05:21 CST 2011

Explicitly update .dirtied_when on synced inodes, so that they are no
longer considered for writeback in the next round.

We'll do more aggressive "keep writeback as long as we wrote something"
logic in wb_writeback(). Then the "use LONG_MAX .nr_to_write" trick in
commit b9543dac5b ("writeback: avoid livelocking WB_SYNC_ALL writeback")
will no longer be enough to stop sync livelock.

Reviewed-by: Jan Kara <jack@...e.cz>
Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
---
 fs/fs-writeback.c |    9 +++++++++
 1 file changed, 9 insertions(+)

--- linux-next.orig/fs/fs-writeback.c	2011-05-10 09:50:07.000000000 +0800
+++ linux-next/fs/fs-writeback.c	2011-05-10 10:03:00.000000000 +0800
@@ -419,6 +419,15 @@ writeback_single_inode(struct inode *ino
 	spin_lock(&inode->i_lock);
 	inode->i_state &= ~I_SYNC;
 	if (!(inode->i_state & I_FREEING)) {
+		/*
+		 * Sync livelock prevention. Each inode is tagged and synced in
+		 * one shot, so we can unconditionally update its dirty time to
+		 * prevent syncing it again. Note that time ordering of b_dirty
+		 * list will be kept because the following code either removes
+		 * the inode from b_dirty or calls redirty_tail().
+		 */
+		if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_sync)
+			inode->dirtied_when = jiffies;
 		if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
 			/*
 			 * We didn't write back all the pages.  nfs_writepages()
--
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/