From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754543Ab0GZLwJ (ORCPT ); Mon, 26 Jul 2010 07:52:09 -0400 Received: from mga11.intel.com ([192.55.52.93]:20235 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754077Ab0GZLwG (ORCPT ); Mon, 26 Jul 2010 07:52:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.55,261,1278313200"; d="scan'208";a="821632210" Date: Mon, 26 Jul 2010 19:51:53 +0800 From: Wu Fengguang To: Jan Kara Cc: Andrew Morton , Dave Chinner , Christoph Hellwig , Mel Gorman , Chris Mason , Jens Axboe , LKML , "linux-fsdevel@vger.kernel.org" , "linux-mm@kvack.org" Subject: Re: [PATCH 4/6] writeback: sync expired inodes first in background writeback Message-ID: <20100726115153.GF6284@localhost> References: <20100722050928.653312535@intel.com> <20100722061822.906037624@intel.com> <20100723181521.GC20540@quack.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100723181521.GC20540@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, Jul 24, 2010 at 02:15:21AM +0800, Jan Kara wrote: > On Thu 22-07-10 13:09:32, Wu Fengguang wrote: > > A background flush work may run for ever. So it's reasonable for it to > > mimic the kupdate behavior of syncing old/expired inodes first. > > > > The policy is > > - enqueue all newly expired inodes at each queue_io() time > > - retry with halfed expire interval until get some inodes to sync > Hmm, this logic looks a bit arbitrary to me. What I actually don't like > very much about this that when there aren't inodes older than say 2 > seconds, you'll end up queueing just inodes between 2s and 1s. So I'd > rather just queue inodes older than the limit and if there are none, just > queue all other dirty inodes. You are proposing - expire_interval >>= 1; + expire_interval = 0; IMO this does not really simplify code or concept. If we can get the "smoother" behavior in original patch without extra cost, why not? Thanks, Fengguang > > CC: Jan Kara > > Signed-off-by: Wu Fengguang > > --- > > fs/fs-writeback.c | 20 ++++++++++++++------ > > 1 file changed, 14 insertions(+), 6 deletions(-) > > > > --- linux-next.orig/fs/fs-writeback.c 2010-07-22 12:56:42.000000000 +0800 > > +++ linux-next/fs/fs-writeback.c 2010-07-22 13:07:51.000000000 +0800 > > @@ -217,14 +217,14 @@ static void move_expired_inodes(struct l > > struct writeback_control *wbc) > > { > > unsigned long expire_interval = 0; > > - unsigned long older_than_this; > > + unsigned long older_than_this = 0; /* reset to kill gcc warning */ > > LIST_HEAD(tmp); > > struct list_head *pos, *node; > > struct super_block *sb = NULL; > > struct inode *inode; > > int do_sb_sort = 0; > > > > - if (wbc->for_kupdate) { > > + if (wbc->for_kupdate || wbc->for_background) { > > expire_interval = msecs_to_jiffies(dirty_expire_interval * 10); > > older_than_this = jiffies - expire_interval; > > } > > @@ -232,8 +232,15 @@ static void move_expired_inodes(struct l > > while (!list_empty(delaying_queue)) { > > inode = list_entry(delaying_queue->prev, struct inode, i_list); > > if (expire_interval && > > - inode_dirtied_after(inode, older_than_this)) > > - break; > > + inode_dirtied_after(inode, older_than_this)) { > > + if (wbc->for_background && > > + list_empty(dispatch_queue) && list_empty(&tmp)) { > > + expire_interval >>= 1; > > + older_than_this = jiffies - expire_interval; > > + continue; > > + } else > > + break; > > + } > > if (sb && sb != inode->i_sb) > > do_sb_sort = 1; > > sb = inode->i_sb; > > @@ -521,7 +528,8 @@ void writeback_inodes_wb(struct bdi_writ > > > > wbc->wb_start = jiffies; /* livelock avoidance */ > > spin_lock(&inode_lock); > > - if (!wbc->for_kupdate || list_empty(&wb->b_io)) > > + > > + if (!(wbc->for_kupdate || wbc->for_background) || list_empty(&wb->b_io)) > > queue_io(wb, wbc); > > > > while (!list_empty(&wb->b_io)) { > > @@ -550,7 +558,7 @@ static void __writeback_inodes_sb(struct > > > > wbc->wb_start = jiffies; /* livelock avoidance */ > > spin_lock(&inode_lock); > > - if (!wbc->for_kupdate || list_empty(&wb->b_io)) > > + if (!(wbc->for_kupdate || wbc->for_background) || list_empty(&wb->b_io)) > > queue_io(wb, wbc); > > writeback_sb_inodes(sb, wb, wbc, true); > > spin_unlock(&inode_lock); > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > Jan Kara > SUSE Labs, CR