2023-05-22 00:25:40

by Damien Le Moal

[permalink] [raw]
Subject: Re: [PATCH 10/13] fs: factor out a direct_write_fallback helper

On 5/19/23 18:35, Christoph Hellwig wrote:
> Add a helper dealing with handling the syncing of a buffered write fallback
> for direct I/O.
>
> Signed-off-by: Christoph Hellwig <[email protected]>

Looks OK. One comment below.

Reviewed-by: Damien Le Moal <[email protected]>

> + /*
> + * We need to ensure that the page cache pages are written to disk and
> + * invalidated to preserve the expected O_DIRECT semantics.
> + */
> + end = pos + buffered_written - 1;
> + err = filemap_write_and_wait_range(mapping, pos, end);
> + if (err < 0) {
> + /*
> + * We don't know how much we wrote, so just return the number of
> + * bytes which were direct-written
> + */
> + return err;
> + }
> + invalidate_mapping_pages(mapping, pos >> PAGE_SHIFT, end >> PAGE_SHIFT);
> + return direct_written + buffered_written;

Why not adding here something like:

if (buffered_written != iov_iter_count(from))
return -EIO;

return direct_written + buffered_written;

to have the same semantic as plain DIO ?

--
Damien Le Moal
Western Digital Research