Re: [patch 9/9] mm: do_sync_mapping_range integrity fix

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

On Fri, 31 Oct 2008 10:16:16 +0100 Nick Piggin <npiggin@suse.de> wrote:

>  
> > So all we need to guarantee here is that
> > __filemap_fdatawrite_range(WB_SYNC_NONE) will start writeout on all
> > dirty pages in the range.  Probably that gets broken lower down as part
> > of various hacks^woptimisations have gone in, but which ones, and
> > where?  Perhaps _this_ (if it's there) is what should be fixed.
> 
> WB_SYNC_NONE has never (until this was introduced) been used for data
> integrity AFAIKS. There is code littered throughout fs/ which assumes
> WB_SYNC_NONE ~= "efficiency / background writeback". At least
> definitely the buffer.c "trylock" will cause dirty pages to be
> skipped.


The change seriously wrecks sync_file_range(SYNC_FILE_RANGE_WRITE), the
whole point of which is to as-nonblockingly-as-possible shove some data
into the queues.  This is useful.

Perhaps we could use WB_SYNC_ALL if SYNC_FILE_RANGE_WAIT_BEFORE was
specified, or WB_SYNC_NONE if it was not (need to think about
SYNC_FILE_RANGE_WAIT_AFTER, too).

That's a bit grubby, because userspace could do

	sync_file_range(SYNC_FILE_RANGE_WAIT_BEFORE);
	sync_file_range(SYNC_FILE_RANGE_WRITE);

expecting it to have the same behaviour as

	sync_file_range(SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE);



How the hell did that stupid sync_mode thing get into
writeback_control?  :( We should get rid of WB_SYNC_FOO and migrate to
better-defined writeback_control fields.