On Mon, Sep 8, 2025 at 10:14 PM Gao Xiang <hsiangkao@xxxxxxxxxxxxxxxxx> wrote:
>
> On 2025/9/9 02:51, Joanne Koong wrote:
> > There is no longer a dependency on CONFIG_BLOCK in the iomap read and
> > readahead logic. Move this logic out of the CONFIG_BLOCK guard. This
> > allows non-block-based filesystems to use iomap for reads/readahead.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > ---
> > fs/iomap/buffered-io.c | 151 +++++++++++++++++++++--------------------
> > 1 file changed, 76 insertions(+), 75 deletions(-)
> >
> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> > index f673e03f4ffb..c424e8c157dd 100644
> > --- a/fs/iomap/buffered-io.c
> > +++ b/fs/iomap/buffered-io.c
> > @@ -358,81 +358,6 @@ void iomap_finish_folio_read(struct folio *folio, size_t off, size_t len,
> > }
> > +
> > +/**
> > + * Read in a folio range asynchronously through bios.
> > + *
> > + * This should only be used for read/readahead, not for buffered writes.
> > + * Buffered writes must read in the folio synchronously.
> > + */
> > +static int iomap_read_folio_range_bio_async(const struct iomap_iter *iter,
> > + struct iomap_read_folio_ctx *ctx, loff_t pos, size_t plen)
> > +{
> > + struct folio *folio = ctx->cur_folio;
> > + const struct iomap *iomap = &iter->iomap;
> > + size_t poff = offset_in_folio(folio, pos);
> > + loff_t length = iomap_length(iter);
> > + sector_t sector;
> > + struct bio *bio = ctx->private;
> > +
> > + iomap_start_folio_read(folio, plen);
> > +
> > + sector = iomap_sector(iomap, pos);
> > + if (!bio || bio_end_sector(bio) != sector ||
> > + !bio_add_folio(bio, folio, plen, poff)) {
> > + gfp_t gfp = mapping_gfp_constraint(folio->mapping, GFP_KERNEL);
> > + gfp_t orig_gfp = gfp;
> > + unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);
> > +
> > + if (bio)
> > + submit_bio(bio);
> > +
> > + if (ctx->rac) /* same as readahead_gfp_mask */
> > + gfp |= __GFP_NORETRY | __GFP_NOWARN;
> > + bio = bio_alloc(iomap->bdev, bio_max_segs(nr_vecs),
> > + REQ_OP_READ, gfp);
> > + /*
> > + * If the bio_alloc fails, try it again for a single page to
> > + * avoid having to deal with partial page reads. This emulates
> > + * what do_mpage_read_folio does.
> > + */
> > + if (!bio)
> > + bio = bio_alloc(iomap->bdev, 1, REQ_OP_READ, orig_gfp);
> > + if (ctx->rac)
> > + bio->bi_opf |= REQ_RAHEAD;
> > + bio->bi_iter.bi_sector = sector;
> > + bio->bi_end_io = iomap_read_end_io;
> > + bio_add_folio_nofail(bio, folio, plen, poff);
> > + ctx->private = bio;
>
> Yes, I understand some way is needed to isolate bio from non-bio
> based filesystems, and I also agree `bio` shouldn't be stashed
> into `iter->private` since it's just an abuse usage as mentioned
> in:
> https://lore.kernel.org/r/20250903203031.GM1587915@frogsfrogsfrogs
> https://lore.kernel.org/r/aLkskcgl3Z91oIVB@xxxxxxxxxxxxx
>
> However, the naming of `(struct iomap_read_folio_ctx)->private`
> really makes me feel confused because the `private` name in
> `read_folio_ctx` is much like a filesystem read context instead
> of just be used as `bio` internally in iomap for block-based
> filesystems.
>
> also the existing of `iter->private` makes the naming of
> `ctx->private` more confusing at least in my view.
Do you think "ctx->data" would be better? Or is there something else
you had in mind?
Thanks,
Joanne
>
> Thanks,
> Gao Xiang
