Re: [PATCH 3/6] writeback: make nr_to_write a per-file limit

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

On Wed, May 04, 2011 at 05:42:21PM +0800, Christoph Hellwig wrote:
> On Wed, May 04, 2011 at 05:17:10PM +0800, Wu Fengguang wrote:
> > This ensures large dirty files can be written in the full 4MB writeback
> > chunk size, rather than whatever remained quota in wbc->nr_to_write.
> 
> I like the high-level idea, but the implementation of overriding
> nr_to_write and then copying it back seems rather ugly.
> 
> The basic problem seems to be that struct writeback_control is
> designed to control writeback of a single file, but we keep abuse it
> for writing multiple files in writeback_sb_inodes and its callers.
> 
> It seems like we should only build the struct writeback_control from
> struct wb_writeback_work down in writeback_sb_inodes, even if that
> means passing some more information to it either in struct
> wb_writeback_work or on the stack.

Yes it's very reasonable and possible according to your notes in
another email.

> Then writeback_sb_inodes can do something like
> 
> 	if (wbc.sync_mode == WB_SYNC_NONE)
> 		wbc.nr_to_write = min(MAX_WRITEBACK_PAGES, work->nr_pages);

I like the min() idea. However it have the side effect of (very possible)
smallish IO from balance_dirty_pages(), which may call us with small
->nr_pages. 

We may explicitly do "write_chunk = max(MAX_WRITEBACK_PAGES, write_chunk)"
in balance_dirty_pages() to retain the old behavior.

> 	else
> 		wbc.nr_to_write = LONG_MAX;
> 
> for each inode it writes.

Thanks,
Fengguang