On 2025/9/9 23:33, Joanne Koong wrote:

On Mon, Sep 8, 2025 at 10:14 PM Gao Xiang [off-list ref] wrote:

quoted

On 2025/9/9 02:51, Joanne Koong wrote:

quoted

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@gmail.com>
---
   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 (local)
https://lore.kernel.org/r/aLkskcgl3Z91oIVB@infradead.org (local)

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?

At least it sounds better on my side, but anyway it's just
my own overall thought.  If other folks have different idea,
I don't have strong opinion, I just need something for my own
as previous said.

Thanks,
Gao Xiang

Thanks,
Joanne

quoted

Thanks,
Gao Xiang