On Mon, Sep 22, 2025 at 05:23:45PM -0700, Joanne Koong wrote:
> The purpose of "struct iomap_read_folio_ctx->cur_folio_in_bio" is to
> track folio ownership to know who is responsible for unlocking it.
> Rename "cur_folio_in_bio" to "cur_folio_owned" to better reflect this
> purpose and so that this can be generically used later on by filesystems
> that are not block-based.
>
> Since "struct iomap_read_folio_ctx" will be made a public interface
> later on when read/readahead takes in caller-provided callbacks, track
> the folio ownership state internally instead of exposing it in "struct
> iomap_read_folio_ctx" to make the interface simpler for end users.
>
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
Looks good to me now,
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> ---
> fs/iomap/buffered-io.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 09e65771a947..34df1cddf65c 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -362,7 +362,6 @@ static void iomap_read_end_io(struct bio *bio)
>
> struct iomap_read_folio_ctx {
> struct folio *cur_folio;
> - bool cur_folio_in_bio;
> void *read_ctx;
> struct readahead_control *rac;
> };
> @@ -386,7 +385,6 @@ static void iomap_bio_read_folio_range(const struct iomap_iter *iter,
> sector_t sector;
> struct bio *bio = ctx->read_ctx;
>
> - ctx->cur_folio_in_bio = true;
> if (ifs) {
> spin_lock_irq(&ifs->state_lock);
> ifs->read_bytes_pending += plen;
> @@ -423,7 +421,7 @@ static void iomap_bio_read_folio_range(const struct iomap_iter *iter,
> }
>
> static int iomap_read_folio_iter(struct iomap_iter *iter,
> - struct iomap_read_folio_ctx *ctx)
> + struct iomap_read_folio_ctx *ctx, bool *folio_owned)
> {
> const struct iomap *iomap = &iter->iomap;
> loff_t pos = iter->pos;
> @@ -460,6 +458,7 @@ static int iomap_read_folio_iter(struct iomap_iter *iter,
> folio_zero_range(folio, poff, plen);
> iomap_set_range_uptodate(folio, poff, plen);
> } else {
> + *folio_owned = true;
> iomap_bio_read_folio_range(iter, ctx, pos, plen);
> }
>
> @@ -482,16 +481,22 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops)
> struct iomap_read_folio_ctx ctx = {
> .cur_folio = folio,
> };
> + /*
> + * If an IO helper takes ownership of the folio, it is responsible for
> + * unlocking it when the read completes.
> + */
> + bool folio_owned = false;
> int ret;
>
> trace_iomap_readpage(iter.inode, 1);
>
> while ((ret = iomap_iter(&iter, ops)) > 0)
> - iter.status = iomap_read_folio_iter(&iter, &ctx);
> + iter.status = iomap_read_folio_iter(&iter, &ctx,
> + &folio_owned);
>
> iomap_bio_submit_read(&ctx);
>
> - if (!ctx.cur_folio_in_bio)
> + if (!folio_owned)
> folio_unlock(folio);
>
> /*
> @@ -504,14 +509,15 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops)
> EXPORT_SYMBOL_GPL(iomap_read_folio);
>
> static int iomap_readahead_iter(struct iomap_iter *iter,
> - struct iomap_read_folio_ctx *ctx)
> + struct iomap_read_folio_ctx *ctx,
> + bool *cur_folio_owned)
> {
> int ret;
>
> while (iomap_length(iter)) {
> if (ctx->cur_folio &&
> offset_in_folio(ctx->cur_folio, iter->pos) == 0) {
> - if (!ctx->cur_folio_in_bio)
> + if (!*cur_folio_owned)
> folio_unlock(ctx->cur_folio);
> ctx->cur_folio = NULL;
> }
> @@ -519,9 +525,9 @@ static int iomap_readahead_iter(struct iomap_iter *iter,
> ctx->cur_folio = readahead_folio(ctx->rac);
> if (WARN_ON_ONCE(!ctx->cur_folio))
> return -EINVAL;
> - ctx->cur_folio_in_bio = false;
> + *cur_folio_owned = false;
> }
> - ret = iomap_read_folio_iter(iter, ctx);
> + ret = iomap_read_folio_iter(iter, ctx, cur_folio_owned);
> if (ret)
> return ret;
> }
> @@ -554,15 +560,21 @@ void iomap_readahead(struct readahead_control *rac, const struct iomap_ops *ops)
> struct iomap_read_folio_ctx ctx = {
> .rac = rac,
> };
> + /*
> + * If an IO helper takes ownership of the folio, it is responsible for
> + * unlocking it when the read completes.
> + */
> + bool cur_folio_owned = false;
>
> trace_iomap_readahead(rac->mapping->host, readahead_count(rac));
>
> while (iomap_iter(&iter, ops) > 0)
> - iter.status = iomap_readahead_iter(&iter, &ctx);
> + iter.status = iomap_readahead_iter(&iter, &ctx,
> + &cur_folio_owned);
>
> iomap_bio_submit_read(&ctx);
>
> - if (ctx.cur_folio && !ctx.cur_folio_in_bio)
> + if (ctx.cur_folio && !cur_folio_owned)
> folio_unlock(ctx.cur_folio);
> }
> EXPORT_SYMBOL_GPL(iomap_readahead);
> --
> 2.47.3
>
>
