On Thu, Sep 11, 2025 at 7:13 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote:
>
> > +static void iomap_read_folio_range_bio_async(const struct iomap_iter *iter,
> > + struct iomap_readpage_ctx *ctx, loff_t pos, size_t plen)
>
> The _async here feels very misplaced, because pretty much everyting in
> the area except for the odd write_begin helper is async, and the postfix
> does not match the method name.
>
> Also as a general discussion for naming, having common prefixed for sets
> of related helpers is nice. Maybe use iomap_bio_* for all the bio
> helpers were're adding here? We can then fix the direct I/O code to
> match that later.
Great point, I'll change this to "iomap_bio_read_folio_range()" for
the async version and then "iomap_bio_read_folio_range_sync()" for the
synchronous version.
>
> > {
> > + struct folio *folio = ctx->cur_folio;
> > const struct iomap *iomap = &iter->iomap;
> > - loff_t pos = iter->pos;
>
> Looking at the caller, it seems we should not need the pos argument if
> we adjust pos just after calculating count at the beginning of the loop.
> I think that would be a much better interface.
>
Sounds good, in that case I think we should do the same for the the
buffered writes ->read_folio_range() api later too then to have it
match
Thanks,
Joanne
