Re: [PATCH 1/3 v2] mm: Stop background writeback if there is other work queued for the thread

[Andrew Morton <akpm@linux-foundation.org>]

On Thu,  5 Aug 2010 20:53:17 +0200
Jan Kara <jack@suse.cz> wrote:

> Background writeback and kupdate-style writeback

What the heck's the difference between "Background writeback" and
"kupdate-style" writeback?  afacit "background" means "not due to
kupdate, but due to a vmscan poke or something like that".  But the
terms aren't defined anywhere and the wb_writeback_work fields are
uncommented and the functions are undocumented and no wonder we keep
making such a mess of this code.

> are easily livelockable (from
> a definition of their target).

Please fully describe the livelock scenario(s).

> This is inconvenient because it can make sync(1)
> stall forever waiting on its queued work to be finished.

And please fully describe the reason for the stall of sync(1).


Because if these things _are_ described then others are in a better
position to review your proposed fix and they are in a better position
to propose alternative fixes, no?

>  Generally, if someone
> has a particular requirement for writeback he needs, it makes sense to give it
> preference over a generic background dirty page cleaning. As soon as that work
> is done, flusher thread will return back to background cleaning if it is
> needed. So lets just interrupt background and kupdate writeback if there is
> some other work to do to fix the livelocking problem.
> 
> CC: hch@infradead.org
> Reviewed-by: Wu Fengguang <fengguang.wu@intel.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/fs-writeback.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
> index d5be169..542471e 100644
> --- a/fs/fs-writeback.c
> +++ b/fs/fs-writeback.c
> @@ -633,6 +633,14 @@ static long wb_writeback(struct bdi_writeback *wb,
>  			break;
>  
>  		/*
> +		 * Background writeout and kupdate-style writeback are
> +		 * easily livelockable. Stop them if there is other work
> +		 * to do so that e.g. sync can proceed.
> +		 */
> +		if ((work->for_background || work->for_kupdate) &&
> +		    !list_empty(&wb->bdi->work_list))
> +			break;
> +		/*

So what happens if an application sits in a loop doing write&fsync to a
file?  The vm's call for help gets ignored and your data doesn't get
written back for three days??