| Message ID | 92ae9f3333c9a7e66214568d08f45664261c899c.1715067055.git.ritesh.list@gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | iomap: Optimize read_folio | expand |
On Tue, May 07, 2024 at 02:25:43PM +0530, Ritesh Harjani (IBM) wrote: > iomap_readpage_iter() handles "uptodate blocks" and "not uptodate blocks" > within a folio separately. This makes iomap_read_folio() to call into > ->iomap_begin() to request for extent mapping even though it might already > have an extent which is not fully processed. > This happens when we either have a large folio or with bs < ps. In these > cases we can have sub blocks which can be uptodate (say for e.g. due to > previous writes). With iomap_read_folio_iter(), this is handled more > efficiently by not calling ->iomap_begin() call until all the sub blocks > with the current folio are processed. > > iomap_read_folio_iter() handles multiple sub blocks within a given > folio but it's implementation logic is similar to how > iomap_readahead_iter() handles multiple folios within a single mapped > extent. Both of them iterate over a given range of folio/mapped extent > and call iomap_readpage_iter() for reading. > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > cc: Ojaswin Mujoo <ojaswin@linux.ibm.com> I like this improved changelog, it'e easier to understand why _read_folio_iter needs to exist. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/iomap/buffered-io.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 9f79c82d1f73..a9bd74ee7870 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -444,6 +444,24 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter, > return pos - orig_pos + plen; > } > > +static loff_t iomap_read_folio_iter(const 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) > + return ret; > + } > + > + return done; > +} > + > int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > { > struct iomap_iter iter = { > @@ -459,7 +477,7 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > trace_iomap_readpage(iter.inode, 1); > > while ((ret = iomap_iter(&iter, ops)) > 0) > - iter.processed = iomap_readpage_iter(&iter, &ctx, 0); > + iter.processed = iomap_read_folio_iter(&iter, &ctx); > > if (ret < 0) > folio_set_error(folio); > -- > 2.44.0 > >
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
On Tue 07-05-24 14:25:43, Ritesh Harjani (IBM) wrote: > iomap_readpage_iter() handles "uptodate blocks" and "not uptodate blocks" > within a folio separately. This makes iomap_read_folio() to call into > ->iomap_begin() to request for extent mapping even though it might already > have an extent which is not fully processed. > This happens when we either have a large folio or with bs < ps. In these > cases we can have sub blocks which can be uptodate (say for e.g. due to > previous writes). With iomap_read_folio_iter(), this is handled more > efficiently by not calling ->iomap_begin() call until all the sub blocks > with the current folio are processed. > > iomap_read_folio_iter() handles multiple sub blocks within a given > folio but it's implementation logic is similar to how > iomap_readahead_iter() handles multiple folios within a single mapped > extent. Both of them iterate over a given range of folio/mapped extent > and call iomap_readpage_iter() for reading. > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > cc: Ojaswin Mujoo <ojaswin@linux.ibm.com> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/iomap/buffered-io.c | 20 +++++++++++++++++++- > 1 file changed, 19 insertions(+), 1 deletion(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 9f79c82d1f73..a9bd74ee7870 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -444,6 +444,24 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter, > return pos - orig_pos + plen; > } > > +static loff_t iomap_read_folio_iter(const 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) > + return ret; > + } > + > + return done; > +} > + > int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > { > struct iomap_iter iter = { > @@ -459,7 +477,7 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) > trace_iomap_readpage(iter.inode, 1); > > while ((ret = iomap_iter(&iter, ops)) > 0) > - iter.processed = iomap_readpage_iter(&iter, &ctx, 0); > + iter.processed = iomap_read_folio_iter(&iter, &ctx); > > if (ret < 0) > folio_set_error(folio); > -- > 2.44.0 >
On Tue, 07 May 2024 14:25:43 +0530, Ritesh Harjani (IBM) wrote: > iomap_readpage_iter() handles "uptodate blocks" and "not uptodate blocks" > within a folio separately. This makes iomap_read_folio() to call into > ->iomap_begin() to request for extent mapping even though it might already > have an extent which is not fully processed. > This happens when we either have a large folio or with bs < ps. In these > cases we can have sub blocks which can be uptodate (say for e.g. due to > previous writes). With iomap_read_folio_iter(), this is handled more > efficiently by not calling ->iomap_begin() call until all the sub blocks > with the current folio are processed. > > [...] Applied to the vfs.iomap branch of the vfs/vfs.git tree. Patches in the vfs.iomap branch should appear in linux-next soon. Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it. It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated. Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch. tree: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git branch: vfs.iomap [2/2] iomap: Optimize iomap_read_folio https://git.kernel.org/vfs/vfs/c/20b686c56bd0
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 9f79c82d1f73..a9bd74ee7870 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -444,6 +444,24 @@ static loff_t iomap_readpage_iter(const struct iomap_iter *iter, return pos - orig_pos + plen; } +static loff_t iomap_read_folio_iter(const 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) + return ret; + } + + return done; +} + int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) { struct iomap_iter iter = { @@ -459,7 +477,7 @@ int iomap_read_folio(struct folio *folio, const struct iomap_ops *ops) trace_iomap_readpage(iter.inode, 1); while ((ret = iomap_iter(&iter, ops)) > 0) - iter.processed = iomap_readpage_iter(&iter, &ctx, 0); + iter.processed = iomap_read_folio_iter(&iter, &ctx); if (ret < 0) folio_set_error(folio);
iomap_readpage_iter() handles "uptodate blocks" and "not uptodate blocks" within a folio separately. This makes iomap_read_folio() to call into ->iomap_begin() to request for extent mapping even though it might already have an extent which is not fully processed. This happens when we either have a large folio or with bs < ps. In these cases we can have sub blocks which can be uptodate (say for e.g. due to previous writes). With iomap_read_folio_iter(), this is handled more efficiently by not calling ->iomap_begin() call until all the sub blocks with the current folio are processed. iomap_read_folio_iter() handles multiple sub blocks within a given folio but it's implementation logic is similar to how iomap_readahead_iter() handles multiple folios within a single mapped extent. Both of them iterate over a given range of folio/mapped extent and call iomap_readpage_iter() for reading. Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> cc: Ojaswin Mujoo <ojaswin@linux.ibm.com> --- fs/iomap/buffered-io.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) -- 2.44.0