On Thu, Sep 11, 2025 at 7:26 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> 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.
Sounds great, I will make these changes you suggested here and in your
comments on the other patches too.
Thank you for reviewing this patchset.
>
> > +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 '='.
>
