On Wed, May 18, 2022 at 04:36:55PM -0700, Stefan Roesch wrote:
> This adds async buffered write support to iomap. The support is focused
> on the changes necessary to support XFS with iomap.
>
> Support for other filesystems might require additional changes.
What would those other changes be? Inline data support should not
matter here, so I guess it is buffer_head support? Please spell out
the actual limitations instead of the use case. Preferably including
asserts in the code to catch the case of a file system trying to use
the now supported cases.
>
> Signed-off-by: Stefan Roesch <shr@xxxxxx>
> ---
> fs/iomap/buffered-io.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 6b06fd358958..b029e2b10e07 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -580,12 +580,18 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
> size_t from = offset_in_folio(folio, pos), to = from + len;
> size_t poff, plen;
> gfp_t gfp = GFP_NOFS | __GFP_NOFAIL;
> + bool no_wait = (iter->flags & IOMAP_NOWAIT);
> +
> + if (no_wait)
Does thi flag really buy us anything? My preference woud be to see
the IOMAP_NOWAIT directy as that is easier for me to read than trying to
figure out what no_wait actually means.
> + gfp = GFP_NOWAIT;
>
> if (folio_test_uptodate(folio))
> return 0;
> folio_clear_error(folio);
>
> iop = iomap_page_create_gfp(iter->inode, folio, nr_blocks, gfp);
And maybe the btter iomap_page_create inteface would be one that passes
the flags so that we can centralize the gfp_t selection.
> @@ -602,6 +608,8 @@ static int __iomap_write_begin(const struct iomap_iter *iter, loff_t pos,
> if (WARN_ON_ONCE(iter->flags & IOMAP_UNSHARE))
> return -EIO;
> folio_zero_segments(folio, poff, from, to, poff + plen);
> + } else if (no_wait) {
> + return -EAGAIN;
> } else {
> int status = iomap_read_folio_sync(block_start, folio,
> poff, plen, srcmap);
That's a somewhat unnatural code flow. I'd much prefer:
} else {
int status;
if (iter->flags & IOMAP_NOWAIT)
return -EAGAIN;
iomap_read_folio_sync(block_start, folio,
poff, plen, srcmap);
Or maybe even pass the iter to iomap_read_folio_sync and just do the
IOMAP_NOWAIT check there.
