[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20100723181521.GC20540@quack.suse.cz>
Date: Fri, 23 Jul 2010 20:15:21 +0200
From: Jan Kara <jack@...e.cz>
To: Wu Fengguang <fengguang.wu@...el.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Dave Chinner <david@...morbit.com>, Jan Kara <jack@...e.cz>,
Christoph Hellwig <hch@...radead.org>,
Mel Gorman <mel@....ul.ie>,
Chris Mason <chris.mason@...cle.com>,
Jens Axboe <jens.axboe@...cle.com>,
LKML <linux-kernel@...r.kernel.org>,
linux-fsdevel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 4/6] writeback: sync expired inodes first in background
writeback
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.
Honza
> CC: Jan Kara <jack@...e.cz>
> Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
> ---
> 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@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
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/
Powered by blists - more mailing lists