Re: [PATCH 0/6] writeback: moving expire targets for background/kupdate works

[Wu Fengguang <fengguang.wu@intel.com>]

On Tue, Apr 19, 2011 at 02:38:23PM +0800, Dave Chinner wrote:
> On Tue, Apr 19, 2011 at 11:00:03AM +0800, Wu Fengguang wrote:
> > 
> > Andrew,
> > 
> > This aims to reduce possible pageout() calls by making the flusher
> > concentrate a bit more on old/expired dirty inodes.
> 
> In what situation is this a problem? Can you demonstrate how you
> trigger it? And then how much improvement does this patchset make?

As Mel put it, "it makes sense to write old pages first to reduce the
chances page reclaim is initiating IO."

In last year's LSF, Rik presented the situation with a graph:

LRU head                                 [*] dirty page
[                          *              *      * *  *  * * * * * *]

Ideally, most dirty pages should lie close to the LRU tail instead of
LRU head. That requires the flusher thread to sync old/expired inodes
first (as there are obvious correlations between inode age and page
age), and to give fair opportunities to newly expired inodes rather
than sticking with some large eldest inodes (as larger inodes have
weaker correlations in the inode<=>page ages).

This patchset helps the flusher to meet both the above requirements.

The measurable improvements will depend a lot on the workload.  Mel
once did some tests and observed it to help (but as large as his
forward flush patches ;)

https://lkml.org/lkml/2010/7/28/124

> > Patches 04, 05 have been updated since last post, please review.
> > The concerns from last review have been addressed.
> > 
> > It runs fine on simple workloads over ext3/4, xfs, btrfs and NFS.
> 
> But it starts propagating new differences between background and
> kupdate style writeback. We've been trying to reduce the number of
> permutations of writeback behaviour, so it seems to me to be wrong
> to further increase the behavioural differences. Indeed, why do we
> need "for kupdate" style writeback and "background" writeback
> anymore - can' we just use background style writeback for both?

This patchset actually brings the background work semantic/behavior
closer to the kupdate work.

The two type of works have different termination rules: one is the 30s
dirty expire time, another is the background_thresh in number of dirty
pages. So they have to be treated differently when selecting the inodes
to sync.

This "if" could possibly be eliminated later, but should be done
carefully in an independent patch, preferably after this patchset is
confirmed to work reliably in upstream.

-       if (wbc->for_kupdate || wbc->for_background) {
                expire_interval = msecs_to_jiffies(dirty_expire_interval * 10);
                older_than_this = jiffies - expire_interval;
-       }

Thanks,
Fengguang