From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752654Ab1EJCO0 (ORCPT ); Mon, 9 May 2011 22:14:26 -0400 Received: from mga01.intel.com ([192.55.52.88]:15441 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751701Ab1EJCOY (ORCPT ); Mon, 9 May 2011 22:14:24 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.64,343,1301900400"; d="scan'208";a="690454648" Date: Tue, 10 May 2011 10:14:21 +0800 From: Wu Fengguang To: Jan Kara Cc: Andrew Morton , LKML , Dave Chinner , Christoph Hellwig , "linux-fsdevel@vger.kernel.org" Subject: Re: [PATCH 02/17] writeback: update dirtied_when for synced inode to prevent livelock Message-ID: <20110510021421.GA6758@localhost> References: <20110506030821.523093711@intel.com> <20110506031612.043390260@intel.com> <20110506163312.GH18982@quack.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20110506163312.GH18982@quack.suse.cz> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Signed-off-by: Wu Fengguang --- 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()