On Mon, Sep 22, 2025 at 05:23:48PM -0700, Joanne Koong wrote:
> Non-block-based filesystems will be using iomap read/readahead. If they
> handle reading in ranges asynchronously and fulfill those read requests
> on an ongoing basis (instead of all together at the end), then there is
> the possibility that the read on the folio may be prematurely ended if
> earlier async requests complete before the later ones have been issued.
>
> For example if there is a large folio and a readahead request for 16
> pages in that folio, if doing readahead on those 16 pages is split into
> 4 async requests and the first request is sent off and then completed
> before we have sent off the second request, then when the first request
> calls iomap_finish_folio_read(), ifs->read_bytes_pending would be 0,
> which would end the read and unlock the folio prematurely.
>
> To mitigate this, a "bias" is added to ifs->read_bytes_pending before
> the first range is forwarded to the caller and removed after the last
> range has been forwarded.
>
> iomap writeback does this with their async requests as well to prevent
> prematurely ending writeback.
I'm still waiting for responses to the old draft of this patch in the v3
thread.
--D
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> ---
> fs/iomap/buffered-io.c | 48 ++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index 81ba0cc7705a..354819facfac 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -430,6 +430,38 @@ const struct iomap_read_ops iomap_bio_read_ops = {
> };
> EXPORT_SYMBOL_GPL(iomap_bio_read_ops);
>
> +/*
> + * Add a bias to ifs->read_bytes_pending to prevent the read on the folio from
> + * being ended prematurely.
> + *
> + * Otherwise, if the ranges are read asynchronously and read requests are
> + * fulfilled on an ongoing basis, there is the possibility that the read on the
> + * folio may be prematurely ended if earlier async requests complete before the
> + * later ones have been issued.
> + */
> +static void iomap_read_add_bias(struct iomap_iter *iter, struct folio *folio)
> +{
> + ifs_alloc(iter->inode, folio, iter->flags);
> + iomap_start_folio_read(folio, 1);
> +}
> +
> +static void iomap_read_remove_bias(struct folio *folio, bool folio_owned)
> +{
> + struct iomap_folio_state *ifs = folio->private;
> + bool end_read, uptodate;
> +
> + if (ifs) {
> + spin_lock_irq(&ifs->state_lock);
> + ifs->read_bytes_pending--;
> + end_read = !ifs->read_bytes_pending && folio_owned;
> + if (end_read)
> + uptodate = ifs_is_fully_uptodate(folio, ifs);
> + spin_unlock_irq(&ifs->state_lock);
> + if (end_read)
> + folio_end_read(folio, uptodate);
> + }
> +}
> +
> static int iomap_read_folio_iter(struct iomap_iter *iter,
> struct iomap_read_folio_ctx *ctx, bool *folio_owned)
> {
> @@ -448,8 +480,6 @@ static int iomap_read_folio_iter(struct iomap_iter *iter,
> return iomap_iter_advance(iter, length);
> }
>
> - ifs_alloc(iter->inode, folio, iter->flags);
> -
> length = min_t(loff_t, length,
> folio_size(folio) - offset_in_folio(folio, pos));
> while (length) {
> @@ -505,6 +535,8 @@ int iomap_read_folio(const struct iomap_ops *ops,
>
> trace_iomap_readpage(iter.inode, 1);
>
> + iomap_read_add_bias(&iter, folio);
> +
> while ((ret = iomap_iter(&iter, ops)) > 0)
> iter.status = iomap_read_folio_iter(&iter, ctx,
> &folio_owned);
> @@ -512,6 +544,8 @@ int iomap_read_folio(const struct iomap_ops *ops,
> if (ctx->ops->submit_read)
> ctx->ops->submit_read(ctx);
>
> + iomap_read_remove_bias(folio, folio_owned);
> +
> if (!folio_owned)
> folio_unlock(folio);
>
> @@ -533,6 +567,8 @@ static int iomap_readahead_iter(struct iomap_iter *iter,
> while (iomap_length(iter)) {
> if (ctx->cur_folio &&
> offset_in_folio(ctx->cur_folio, iter->pos) == 0) {
> + iomap_read_remove_bias(ctx->cur_folio,
> + *cur_folio_owned);
> if (!*cur_folio_owned)
> folio_unlock(ctx->cur_folio);
> ctx->cur_folio = NULL;
> @@ -541,6 +577,7 @@ 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;
> + iomap_read_add_bias(iter, ctx->cur_folio);
> *cur_folio_owned = false;
> }
> ret = iomap_read_folio_iter(iter, ctx, cur_folio_owned);
> @@ -590,8 +627,11 @@ void iomap_readahead(const struct iomap_ops *ops,
> if (ctx->ops->submit_read)
> ctx->ops->submit_read(ctx);
>
> - if (ctx->cur_folio && !cur_folio_owned)
> - folio_unlock(ctx->cur_folio);
> + if (ctx->cur_folio) {
> + iomap_read_remove_bias(ctx->cur_folio, cur_folio_owned);
> + if (!cur_folio_owned)
> + folio_unlock(ctx->cur_folio);
> + }
> }
> EXPORT_SYMBOL_GPL(iomap_readahead);
>
> --
> 2.47.3
>
>
