linux-kernel - Re: [PATCH 2/5] writeback: stop periodic/background work on seeing sync works

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]

Message-ID: <20100729162027.GF12690@quack.suse.cz>
Date:	Thu, 29 Jul 2010 18:20:27 +0200
From:	Jan Kara <jack@...e.cz>
To:	Wu Fengguang <fengguang.wu@...el.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>,
	Dave Chinner <david@...morbit.com>,
	Chris Mason <chris.mason@...cle.com>,
	Nick Piggin <npiggin@...e.de>, Rik van Riel <riel@...hat.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Christoph Hellwig <hch@...radead.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Mel Gorman <mel@....ul.ie>, Minchan Kim <minchan.kim@...il.com>
Subject: Re: [PATCH 2/5] writeback: stop periodic/background work on seeing
 sync works

On Thu 29-07-10 19:51:44, Wu Fengguang wrote:
> The periodic/background writeback can run forever. So when any
> sync work is enqueued, increase bdi->sync_works to notify the
> active non-sync works to exit. Non-sync works queued after sync
> works won't be affected.
  Hmm, wouldn't it be simpler logic to just make for_kupdate and
for_background work always yield when there's some other work to do (as
they are livelockable from the definition of the target they have) and
make sure any other work isn't livelockable? The only downside is that
non-livelockable work cannot be "fair" in the sense that we cannot switch
inodes after writing MAX_WRITEBACK_PAGES.
  I even had a patch for this but it's already outdated by now. But I
can refresh it if we decide this is the way to go.

								Honza

> 
> Signed-off-by: Wu Fengguang <fengguang.wu@...el.com>
> ---
>  fs/fs-writeback.c           |   13 +++++++++++++
>  include/linux/backing-dev.h |    6 ++++++
>  mm/backing-dev.c            |    1 +
>  3 files changed, 20 insertions(+)
> 
> --- linux-next.orig/fs/fs-writeback.c	2010-07-29 17:13:23.000000000 +0800
> +++ linux-next/fs/fs-writeback.c	2010-07-29 17:13:49.000000000 +0800
> @@ -80,6 +80,8 @@ static void bdi_queue_work(struct backin
>  
>  	spin_lock(&bdi->wb_lock);
>  	list_add_tail(&work->list, &bdi->work_list);
> +	if (work->for_sync)
> +		atomic_inc(&bdi->wb.sync_works);
>  	spin_unlock(&bdi->wb_lock);
>  
>  	/*
> @@ -633,6 +635,14 @@ static long wb_writeback(struct bdi_writ
>  			break;
>  
>  		/*
> +		 * background/periodic works can run forever, need to abort
> +		 * on seeing any pending sync work, to prevent livelock it.
> +		 */
> +		if (atomic_read(&wb->sync_works) &&
> +		    (work->for_background || work->for_kupdate))
> +			break;
> +
> +		/*
>  		 * For background writeout, stop when we are below the
>  		 * background dirty threshold
>  		 */
> @@ -765,6 +775,9 @@ long wb_do_writeback(struct bdi_writebac
>  
>  		wrote += wb_writeback(wb, work);
>  
> +		if (work->for_sync)
> +			atomic_dec(&wb->sync_works);
> +
>  		/*
>  		 * Notify the caller of completion if this is a synchronous
>  		 * work item, otherwise just free it.
> --- linux-next.orig/include/linux/backing-dev.h	2010-07-29 17:13:23.000000000 +0800
> +++ linux-next/include/linux/backing-dev.h	2010-07-29 17:13:31.000000000 +0800
> @@ -50,6 +50,12 @@ struct bdi_writeback {
>  
>  	unsigned long last_old_flush;		/* last old data flush */
>  
> +	/*
> +	 * sync works queued, background works shall abort on seeing this,
> +	 * to prevent livelocking the sync works
> +	 */
> +	atomic_t sync_works;
> +
>  	struct task_struct	*task;		/* writeback task */
>  	struct list_head	b_dirty;	/* dirty inodes */
>  	struct list_head	b_io;		/* parked for writeback */
> --- linux-next.orig/mm/backing-dev.c	2010-07-29 17:13:23.000000000 +0800
> +++ linux-next/mm/backing-dev.c	2010-07-29 17:13:31.000000000 +0800
> @@ -257,6 +257,7 @@ static void bdi_wb_init(struct bdi_write
>  
>  	wb->bdi = bdi;
>  	wb->last_old_flush = jiffies;
> +	atomic_set(&wb->sync_works, 0);
>  	INIT_LIST_HEAD(&wb->b_dirty);
>  	INIT_LIST_HEAD(&wb->b_io);
>  	INIT_LIST_HEAD(&wb->b_more_io);
> 
> 
> --
> 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/