On Wed, 29 Oct 2008 01:47:24 +1100
npiggin@xxxxxxx wrote:
> Chris Mason notices do_sync_mapping_range didn't actually ask for data
> integrity writeout. Unfortunately, it is advertised as being usable for
> data integrity operations.
>
> This is a data interity bug.
>
> Signed-off-by: Nick Piggin <npiggin@xxxxxxx>
> ---
> Index: linux-2.6/fs/sync.c
> ===================================================================
> --- linux-2.6.orig/fs/sync.c
> +++ linux-2.6/fs/sync.c
> @@ -269,7 +269,7 @@ int do_sync_mapping_range(struct address
>
> if (flags & SYNC_FILE_RANGE_WRITE) {
> ret = __filemap_fdatawrite_range(mapping, offset, endbyte,
> - WB_SYNC_NONE);
> + WB_SYNC_ALL);
> if (ret < 0)
> goto out;
> }
>
Really?
Some thought did go into the code which you're "fixing". If the caller
is using sync_file_range() for integrity then the caller has done a
SYNC_FILE_RANGE_WAIT_BEFORE.
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.
And if we _do_ make the above change, we don't need to run the
wait_on_page_writeback_range() if userspace asked for
SYNC_FILE_RANGE_WAIT_BEFORE|SYNC_FILE_RANGE_WRITE, yes?
IOW, I don't think enough thought (or at least description of that
thought) has gone into this one.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
