| Message ID | 21b78c2f256e513b9eb3f22c7c1f55fc88992600.1606957658.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iov_iter: optimise bvec iov_iter_advance() | expand |
> @@ -1077,6 +1077,20 @@ void iov_iter_advance(struct iov_iter *i, size_t size) > i->count -= size; > return; > } > + if (iov_iter_is_bvec(i)) { > + struct bvec_iter bi; > + > + bi.bi_size = i->count; > + bi.bi_bvec_done = i->iov_offset; > + bi.bi_idx = 0; > + bvec_iter_advance(i->bvec, &bi, size); > + > + i->bvec += bi.bi_idx; > + i->nr_segs -= bi.bi_idx; > + i->count = bi.bi_size; > + i->iov_offset = bi.bi_bvec_done; > + return; > + } > iterate_and_advance(i, size, v, 0, 0, 0) I like the idea, but whu not avoid the on-stack bvec_iter and just open code this entirely using a new helper? E.g. static void bio_iov_iter_advance(struct iov_iter *i, size_t bytes) { unsigned int cnt; i->count -= bytes; bytes += i->iov_offset; for (cnt = 0; bytes && bytes >= i->bvec[cnt].bv_len; cnt++) bytes -= i->bvec[cnt].bv_len; i->iov_offset = bytes; i->bvec += cnt; i->nr_segs -= cnt; }
On 03/12/2020 09:14, Christoph Hellwig wrote: >> @@ -1077,6 +1077,20 @@ void iov_iter_advance(struct iov_iter *i, size_t size) >> i->count -= size; >> return; >> } >> + if (iov_iter_is_bvec(i)) { >> + struct bvec_iter bi; >> + >> + bi.bi_size = i->count; >> + bi.bi_bvec_done = i->iov_offset; >> + bi.bi_idx = 0; >> + bvec_iter_advance(i->bvec, &bi, size); >> + >> + i->bvec += bi.bi_idx; >> + i->nr_segs -= bi.bi_idx; >> + i->count = bi.bi_size; >> + i->iov_offset = bi.bi_bvec_done; >> + return; >> + } >> iterate_and_advance(i, size, v, 0, 0, 0) > > I like the idea, but whu not avoid the on-stack bvec_iter and just > open code this entirely using a new helper? E.g. It's inlined and the on-stack iter is completely optimised out. Frankly, I'd rather not open-code bvec_iter_advance(), at least for this chunk to be findable from bvec.h, e.g. grep bvec_iter and bvec_for_each. Though, I don't like myself that preamble/postamble. > > static void bio_iov_iter_advance(struct iov_iter *i, size_t bytes) > { > unsigned int cnt; > > i->count -= bytes; > > bytes += i->iov_offset; > for (cnt = 0; bytes && bytes >= i->bvec[cnt].bv_len; cnt++) > bytes -= i->bvec[cnt].bv_len; > i->iov_offset = bytes; > > i->bvec += cnt; > i->nr_segs -= cnt; > } >
On Thu, Dec 03, 2020 at 11:48:56AM +0000, Pavel Begunkov wrote: > It's inlined and the on-stack iter is completely optimised out. Frankly, > I'd rather not open-code bvec_iter_advance(), at least for this chunk to > be findable from bvec.h, e.g. grep bvec_iter and bvec_for_each. Though, > I don't like myself that preamble/postamble. Ok. I still think we at least want a separate helper instead of bloating the main iov_iter_advance.
On 09/12/2020 08:33, Christoph Hellwig wrote: > On Thu, Dec 03, 2020 at 11:48:56AM +0000, Pavel Begunkov wrote: >> It's inlined and the on-stack iter is completely optimised out. Frankly, >> I'd rather not open-code bvec_iter_advance(), at least for this chunk to >> be findable from bvec.h, e.g. grep bvec_iter and bvec_for_each. Though, >> I don't like myself that preamble/postamble. > > Ok. I still think we at least want a separate helper instead of > bloating the main iov_iter_advance. I agree on that, just haven't got back to it yet.
diff --git a/lib/iov_iter.c b/lib/iov_iter.c index 1635111c5bd2..7fbdd9ef3ff0 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -1077,6 +1077,20 @@ void iov_iter_advance(struct iov_iter *i, size_t size) i->count -= size; return; } + if (iov_iter_is_bvec(i)) { + struct bvec_iter bi; + + bi.bi_size = i->count; + bi.bi_bvec_done = i->iov_offset; + bi.bi_idx = 0; + bvec_iter_advance(i->bvec, &bi, size); + + i->bvec += bi.bi_idx; + i->nr_segs -= bi.bi_idx; + i->count = bi.bi_size; + i->iov_offset = bi.bi_bvec_done; + return; + } iterate_and_advance(i, size, v, 0, 0, 0) } EXPORT_SYMBOL(iov_iter_advance);
iov_iter_advance() is heavily used, but implemented through generic iteration. As bvecs have a specifically crafted advance() function, i.e. bvec_iter_advance(), which is faster and slimmer, use it instead. Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- lib/iov_iter.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)