| Message ID | 20250219175050.83986-2-bfoster@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iomap: incremental advance conversion -- phase 2 | expand |
On Wed, Feb 19, 2025 at 12:50:39PM -0500, Brian Foster wrote: > iomap buffered read advances the iter via iter.processed. To > continue separating iter advance from return status, update > iomap_readpage_iter() to advance the iter instead of returning the > number of bytes processed. In turn, drop the offset parameter and > sample the updated iter->pos at the start of the function. Update > the callers to loop based on remaining length in the current > iteration instead of number of bytes processed. > > Signed-off-by: Brian Foster <bfoster@redhat.com> > Reviewed-by: Christoph Hellwig <hch@lst.de> > --- > fs/iomap/buffered-io.c | 45 +++++++++++++++++++----------------------- > 1 file changed, 20 insertions(+), 25 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index ec227b45f3aa..215866ba264d 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -366,15 +366,14 @@ static inline bool iomap_block_needs_zeroing(const struct iomap_iter *iter, > pos >= i_size_read(iter->inode); > } > > -static loff_t iomap_readpage_iter(const struct iomap_iter *iter, > - struct iomap_readpage_ctx *ctx, loff_t offset) > +static loff_t iomap_readpage_iter(struct iomap_iter *iter, I wonder, do we really need to return loff_t from some of these functions now? I thought the only return codes were the -EIO/0 from iomap_iter_advance? --D > + struct iomap_readpage_ctx *ctx) > { > const struct iomap *iomap = &iter->iomap; > - loff_t pos = iter->pos + offset; > - loff_t length = iomap_length(iter) - offset; > + loff_t pos = iter->pos; > + loff_t length = iomap_length(iter); > struct folio *folio = ctx->cur_folio; > struct iomap_folio_state *ifs; > - loff_t orig_pos = pos; > size_t poff, plen; > sector_t sector; > > @@ -438,25 +437,22 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter, > * we can skip trailing ones as they will be handled in the next > * iteration. > */ > - return pos - orig_pos + plen; > + length = pos - iter->pos + plen; > + return iomap_iter_advance(iter, &length); > } > > -static loff_t iomap_read_folio_iter(const struct iomap_iter *iter, > +static loff_t iomap_read_folio_iter(struct iomap_iter *iter, > struct iomap_readpage_ctx *ctx) > { > - struct folio *folio = ctx->cur_folio; > - size_t offset = offset_in_folio(folio, iter->pos); > - loff_t length = min_t(loff_t, folio_size(folio) - offset, > - iomap_length(iter)); > - loff_t done, ret; > - > - for (done = 0; done < length; done += ret) { > - ret = iomap_readpage_iter(iter, ctx, done); > - if (ret <= 0) > + loff_t ret; > + > + while (iomap_length(iter)) { > + ret = iomap_readpage_iter(iter, ctx); > + if (ret) > return ret; > } > > - return done; > + return 0; > } > > int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > @@ -493,15 +489,14 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > } > EXPORT_SYMBOL_GPL(iomap_read_folio); > > -static loff_t iomap_readahead_iter(const struct iomap_iter *iter, > +static loff_t iomap_readahead_iter(struct iomap_iter *iter, > struct iomap_readpage_ctx *ctx) > { > - loff_t length = iomap_length(iter); > - loff_t done, ret; > + loff_t ret; > > - for (done = 0; done < length; done += ret) { > + while (iomap_length(iter)) { > if (ctx->cur_folio && > - offset_in_folio(ctx->cur_folio, iter->pos + done) == 0) { > + offset_in_folio(ctx->cur_folio, iter->pos) == 0) { > if (!ctx->cur_folio_in_bio) > folio_unlock(ctx->cur_folio); > ctx->cur_folio = NULL; > @@ -510,12 +505,12 @@ static loff_t iomap_readahead_iter(const struct iomap_iter *iter, > ctx->cur_folio = readahead_folio(ctx->rac); > ctx->cur_folio_in_bio = false; > } > - ret = iomap_readpage_iter(iter, ctx, done); > - if (ret <= 0) > + ret = iomap_readpage_iter(iter, ctx); > + if (ret) > return ret; > } > > - return done; > + return 0; > } > > /** > -- > 2.48.1 > >
On Wed, Feb 19, 2025 at 02:22:35PM -0800, Darrick J. Wong wrote: > On Wed, Feb 19, 2025 at 12:50:39PM -0500, Brian Foster wrote: > > iomap buffered read advances the iter via iter.processed. To > > continue separating iter advance from return status, update > > iomap_readpage_iter() to advance the iter instead of returning the > > number of bytes processed. In turn, drop the offset parameter and > > sample the updated iter->pos at the start of the function. Update > > the callers to loop based on remaining length in the current > > iteration instead of number of bytes processed. > > > > Signed-off-by: Brian Foster <bfoster@redhat.com> > > Reviewed-by: Christoph Hellwig <hch@lst.de> > > --- > > fs/iomap/buffered-io.c | 45 +++++++++++++++++++----------------------- > > 1 file changed, 20 insertions(+), 25 deletions(-) > > > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > > index ec227b45f3aa..215866ba264d 100644 > > --- a/fs/iomap/buffered-io.c > > +++ b/fs/iomap/buffered-io.c > > @@ -366,15 +366,14 @@ static inline bool iomap_block_needs_zeroing(const struct iomap_iter *iter, > > pos >= i_size_read(iter->inode); > > } > > > > -static loff_t iomap_readpage_iter(const struct iomap_iter *iter, > > - struct iomap_readpage_ctx *ctx, loff_t offset) > > +static loff_t iomap_readpage_iter(struct iomap_iter *iter, > > I wonder, do we really need to return loff_t from some of these > functions now? I thought the only return codes were the -EIO/0 from > iomap_iter_advance? And that's provided in the last patch so Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --D > > --D > > > + struct iomap_readpage_ctx *ctx) > > { > > const struct iomap *iomap = &iter->iomap; > > - loff_t pos = iter->pos + offset; > > - loff_t length = iomap_length(iter) - offset; > > + loff_t pos = iter->pos; > > + loff_t length = iomap_length(iter); > > struct folio *folio = ctx->cur_folio; > > struct iomap_folio_state *ifs; > > - loff_t orig_pos = pos; > > size_t poff, plen; > > sector_t sector; > > > > @@ -438,25 +437,22 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter, > > * we can skip trailing ones as they will be handled in the next > > * iteration. > > */ > > - return pos - orig_pos + plen; > > + length = pos - iter->pos + plen; > > + return iomap_iter_advance(iter, &length); > > } > > > > -static loff_t iomap_read_folio_iter(const struct iomap_iter *iter, > > +static loff_t iomap_read_folio_iter(struct iomap_iter *iter, > > struct iomap_readpage_ctx *ctx) > > { > > - struct folio *folio = ctx->cur_folio; > > - size_t offset = offset_in_folio(folio, iter->pos); > > - loff_t length = min_t(loff_t, folio_size(folio) - offset, > > - iomap_length(iter)); > > - loff_t done, ret; > > - > > - for (done = 0; done < length; done += ret) { > > - ret = iomap_readpage_iter(iter, ctx, done); > > - if (ret <= 0) > > + loff_t ret; > > + > > + while (iomap_length(iter)) { > > + ret = iomap_readpage_iter(iter, ctx); > > + if (ret) > > return ret; > > } > > > > - return done; > > + return 0; > > } > > > > int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > > @@ -493,15 +489,14 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > > } > > EXPORT_SYMBOL_GPL(iomap_read_folio); > > > > -static loff_t iomap_readahead_iter(const struct iomap_iter *iter, > > +static loff_t iomap_readahead_iter(struct iomap_iter *iter, > > struct iomap_readpage_ctx *ctx) > > { > > - loff_t length = iomap_length(iter); > > - loff_t done, ret; > > + loff_t ret; > > > > - for (done = 0; done < length; done += ret) { > > + while (iomap_length(iter)) { > > if (ctx->cur_folio && > > - offset_in_folio(ctx->cur_folio, iter->pos + done) == 0) { > > + offset_in_folio(ctx->cur_folio, iter->pos) == 0) { > > if (!ctx->cur_folio_in_bio) > > folio_unlock(ctx->cur_folio); > > ctx->cur_folio = NULL; > > @@ -510,12 +505,12 @@ static loff_t iomap_readahead_iter(const struct iomap_iter *iter, > > ctx->cur_folio = readahead_folio(ctx->rac); > > ctx->cur_folio_in_bio = false; > > } > > - ret = iomap_readpage_iter(iter, ctx, done); > > - if (ret <= 0) > > + ret = iomap_readpage_iter(iter, ctx); > > + if (ret) > > return ret; > > } > > > > - return done; > > + return 0; > > } > > > > /** > > -- > > 2.48.1 > > > > >
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index ec227b45f3aa..215866ba264d 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -366,15 +366,14 @@ static inline bool iomap_block_needs_zeroing(const struct iomap_iter *iter, pos >= i_size_read(iter->inode); } -static loff_t iomap_readpage_iter(const struct iomap_iter *iter, - struct iomap_readpage_ctx *ctx, loff_t offset) +static loff_t iomap_readpage_iter(struct iomap_iter *iter, + struct iomap_readpage_ctx *ctx) { const struct iomap *iomap = &iter->iomap; - loff_t pos = iter->pos + offset; - loff_t length = iomap_length(iter) - offset; + loff_t pos = iter->pos; + loff_t length = iomap_length(iter); struct folio *folio = ctx->cur_folio; struct iomap_folio_state *ifs; - loff_t orig_pos = pos; size_t poff, plen; sector_t sector; @@ -438,25 +437,22 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter, * we can skip trailing ones as they will be handled in the next * iteration. */ - return pos - orig_pos + plen; + length = pos - iter->pos + plen; + return iomap_iter_advance(iter, &length); } -static loff_t iomap_read_folio_iter(const struct iomap_iter *iter, +static loff_t iomap_read_folio_iter(struct iomap_iter *iter, struct iomap_readpage_ctx *ctx) { - struct folio *folio = ctx->cur_folio; - size_t offset = offset_in_folio(folio, iter->pos); - loff_t length = min_t(loff_t, folio_size(folio) - offset, - iomap_length(iter)); - loff_t done, ret; - - for (done = 0; done < length; done += ret) { - ret = iomap_readpage_iter(iter, ctx, done); - if (ret <= 0) + loff_t ret; + + while (iomap_length(iter)) { + ret = iomap_readpage_iter(iter, ctx); + if (ret) return ret; } - return done; + return 0; } int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) @@ -493,15 +489,14 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) } EXPORT_SYMBOL_GPL(iomap_read_folio); -static loff_t iomap_readahead_iter(const struct iomap_iter *iter, +static loff_t iomap_readahead_iter(struct iomap_iter *iter, struct iomap_readpage_ctx *ctx) { - loff_t length = iomap_length(iter); - loff_t done, ret; + loff_t ret; - for (done = 0; done < length; done += ret) { + while (iomap_length(iter)) { if (ctx->cur_folio && - offset_in_folio(ctx->cur_folio, iter->pos + done) == 0) { + offset_in_folio(ctx->cur_folio, iter->pos) == 0) { if (!ctx->cur_folio_in_bio) folio_unlock(ctx->cur_folio); ctx->cur_folio = NULL; @@ -510,12 +505,12 @@ static loff_t iomap_readahead_iter(const struct iomap_iter *iter, ctx->cur_folio = readahead_folio(ctx->rac); ctx->cur_folio_in_bio = false; } - ret = iomap_readpage_iter(iter, ctx, done); - if (ret <= 0) + ret = iomap_readpage_iter(iter, ctx); + if (ret) return ret; } - return done; + return 0; } /**