Re: [PATCH v1 01/16] iomap: move async bio read logic into helper function

[Joanne Koong <joannelkoong@gmail.com>]

On Wed, Sep 3, 2025 at 11:00 PM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Sep 03, 2025 at 01:16:59PM -0700, Darrick J. Wong wrote:
> > On Fri, Aug 29, 2025 at 04:56:12PM -0700, Joanne Koong wrote:
> > > Move the iomap_readpage_iter() async bio read logic into a separate
> > > helper function. This is needed to make iomap read/readahead more
> > > generically usable, especially for filesystems that do not require
> > > CONFIG_BLOCK.
> > >
> > > Rename iomap_read_folio_range() to iomap_read_folio_range_sync() to
> > > diferentiate between the synchronous and asynchronous bio folio read
> > > calls.
> >
> > Hrmm.  Readahead is asynchronous, whereas reading in data as part of an
> > unaligned write to a file must be synchronous.  How about naming it
> > iomap_readahead_folio_range() ?
> >
> > Oh wait, iomap_read_folio also calls iomap_readpage_iter, which uses the
> > readahead paths to fill out a folio, but then waits for the folio lock
> > to drop, which effectively makes it ... a synchronous user of
> > asynchronous code.
> >
> > Bleh, naming is hard.  Though the code splitting seems fine...
>
> Maybe we can look at it from a different angle - the code split out
> isn't really about async vs sync, but about actually using a bio
> to read data from a block device.  Which is also kinda important
> for what Joanne is trying to do.  So I'd encode that in the name,
> e.g. iomap_read_folio_range_bio to mimici the naming used for
> iomap_dio_bio_iter in the direct I/O code.
>
I think the issue is that the iomap_read_folio_range() function used
for buffered writes also uses a bio, so there still needs to be some
way to differentiate between the two. Maybe making the async one the
default "iomap_read_folio_range" and then the synchronous one
"iomap_read_folio_range_sync"?