Re: [PATCH v2 11/16] iomap: add caller-provided callbacks for read and readahead

Christoph Hellwig
2025-09-11T11:26:20Z

On Mon, Sep 08, 2025 at 11:51:17AM -0700, Joanne Koong wrote:
> + - ``read_folio_range``: Called to read in the range (read can be done
> + synchronously or asynchronously). This must be provided by the caller.

As far as I can tell, the interface is always based on an asynchronous
operation, but doesn't preclude completing it right away. So the above
is a little misleading.

> + struct iomap_read_folio_ctx ctx = {
> + .ops = &iomap_read_bios_ops,
> + .cur_folio = folio,
> + };
>
> + return iomap_read_folio(&blkdev_iomap_ops, &ctx);

> + struct iomap_read_folio_ctx ctx = {
> + .ops = &iomap_read_bios_ops,
> + .rac = rac,
> + };
> +
> + iomap_readahead(&blkdev_iomap_ops, &ctx);

Can you add iomap_bio_read_folio and iomap_bio_readahead inline helpers
to reduce this boilerplate code duplicated in various file systems?

> -static void iomap_submit_read_bio(struct iomap_read_folio_ctx *ctx)
> +static int iomap_submit_read_bio(struct iomap_read_folio_ctx *ctx)
> {
> struct bio *bio = ctx->private;
>
> if (bio)
> submit_bio(bio);
> +
> + return 0;

Submission interfaces that can return errors both synchronously and
asynchronously are extremely error probe. I'd be much happier if this
interface could not return errors.

> +const struct iomap_read_ops iomap_read_bios_ops = {
> + .read_folio_range = iomap_read_folio_range_bio_async,
> + .read_submit = iomap_submit_read_bio,
> +};

Please use tabs to align struct initializers before the '='.