| Message ID | 60aaa6caab3d061cf7194716c27a10920b5bd7ad.1606212786.git.asml.silence@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [5.11] block: optimise for_each_bvec() advance | expand |
On Tue, Nov 24, 2020 at 10:21:23AM +0000, Pavel Begunkov wrote: > Because of how for_each_bvec() works it never advances across multiple > entries at a time, so bvec_iter_advance() is an overkill. Add > specialised bvec_iter_advance_single() that is faster. It also handles > zero-len bvecs, so can kill bvec_iter_skip_zero_bvec(). bvec_iter_advance_single needs a comment describing how it can be used. Also can you take a look at the other callers and see who can be switched over? If you are not sure ask the relevant maintainers.
On Tue, Nov 24, 2020 at 10:21:23AM +0000, Pavel Begunkov wrote: > Because of how for_each_bvec() works it never advances across multiple > entries at a time, so bvec_iter_advance() is an overkill. Add > specialised bvec_iter_advance_single() that is faster. It also handles > zero-len bvecs, so can kill bvec_iter_skip_zero_bvec(). > > text data bss dec hex filename > before: > 23977 805 0 24782 60ce lib/iov_iter.o > before, bvec_iter_advance() w/o WARN_ONCE() > 22886 600 0 23486 5bbe ./lib/iov_iter.o > after: > 21862 600 0 22462 57be lib/iov_iter.o > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> > --- > include/linux/bvec.h | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/include/linux/bvec.h b/include/linux/bvec.h > index 2efec10bf792..4a304dfafa18 100644 > --- a/include/linux/bvec.h > +++ b/include/linux/bvec.h > @@ -121,18 +121,24 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv, > return true; > } > > -static inline void bvec_iter_skip_zero_bvec(struct bvec_iter *iter) > +static inline void bvec_iter_advance_single(const struct bio_vec *bv, > + struct bvec_iter *iter, unsigned int bytes) > { > - iter->bi_bvec_done = 0; > - iter->bi_idx++; > + unsigned int done = iter->bi_bvec_done + bytes; > + > + if (done == bv[iter->bi_idx].bv_len) { > + done = 0; > + iter->bi_idx++; > + } > + iter->bi_bvec_done = done; > + iter->bi_size -= bytes; > } > > #define for_each_bvec(bvl, bio_vec, iter, start) \ > for (iter = (start); \ > (iter).bi_size && \ > ((bvl = bvec_iter_bvec((bio_vec), (iter))), 1); \ > - (bvl).bv_len ? (void)bvec_iter_advance((bio_vec), &(iter), \ > - (bvl).bv_len) : bvec_iter_skip_zero_bvec(&(iter))) > + bvec_iter_advance_single((bio_vec), &(iter), (bvl).bv_len)) > > /* for iterating one bio from start to end */ > #define BVEC_ITER_ALL_INIT (struct bvec_iter) \ > -- > 2.24.0 > Looks fine, Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks, Ming
On 24/11/2020 11:37, Ming Lei wrote: > On Tue, Nov 24, 2020 at 10:21:23AM +0000, Pavel Begunkov wrote: >> Because of how for_each_bvec() works it never advances across multiple >> entries at a time, so bvec_iter_advance() is an overkill. Add >> specialised bvec_iter_advance_single() that is faster. It also handles >> zero-len bvecs, so can kill bvec_iter_skip_zero_bvec(). >> [...] > > Looks fine, > > Reviewed-by: Ming Lei <ming.lei@redhat.com> Thanks Ming
On 24/11/2020 11:22, Christoph Hellwig wrote: > On Tue, Nov 24, 2020 at 10:21:23AM +0000, Pavel Begunkov wrote: >> Because of how for_each_bvec() works it never advances across multiple >> entries at a time, so bvec_iter_advance() is an overkill. Add >> specialised bvec_iter_advance_single() that is faster. It also handles >> zero-len bvecs, so can kill bvec_iter_skip_zero_bvec(). > > bvec_iter_advance_single needs a comment describing how it can be agree > used. Also can you take a look at the other callers and see who > can be switched over? If you are not sure ask the relevant maintainers. There are bio_advanced*(), that are used all across block layer. Considering that all that is inlined it's going to be a good win. I'll resend it as a part of a series converting some users. For others like net, I'd rather wait until it lands.
diff --git a/include/linux/bvec.h b/include/linux/bvec.h index 2efec10bf792..4a304dfafa18 100644 --- a/include/linux/bvec.h +++ b/include/linux/bvec.h @@ -121,18 +121,24 @@ static inline bool bvec_iter_advance(const struct bio_vec *bv, return true; } -static inline void bvec_iter_skip_zero_bvec(struct bvec_iter *iter) +static inline void bvec_iter_advance_single(const struct bio_vec *bv, + struct bvec_iter *iter, unsigned int bytes) { - iter->bi_bvec_done = 0; - iter->bi_idx++; + unsigned int done = iter->bi_bvec_done + bytes; + + if (done == bv[iter->bi_idx].bv_len) { + done = 0; + iter->bi_idx++; + } + iter->bi_bvec_done = done; + iter->bi_size -= bytes; } #define for_each_bvec(bvl, bio_vec, iter, start) \ for (iter = (start); \ (iter).bi_size && \ ((bvl = bvec_iter_bvec((bio_vec), (iter))), 1); \ - (bvl).bv_len ? (void)bvec_iter_advance((bio_vec), &(iter), \ - (bvl).bv_len) : bvec_iter_skip_zero_bvec(&(iter))) + bvec_iter_advance_single((bio_vec), &(iter), (bvl).bv_len)) /* for iterating one bio from start to end */ #define BVEC_ITER_ALL_INIT (struct bvec_iter) \
Because of how for_each_bvec() works it never advances across multiple entries at a time, so bvec_iter_advance() is an overkill. Add specialised bvec_iter_advance_single() that is faster. It also handles zero-len bvecs, so can kill bvec_iter_skip_zero_bvec(). text data bss dec hex filename before: 23977 805 0 24782 60ce lib/iov_iter.o before, bvec_iter_advance() w/o WARN_ONCE() 22886 600 0 23486 5bbe ./lib/iov_iter.o after: 21862 600 0 22462 57be lib/iov_iter.o Signed-off-by: Pavel Begunkov <asml.silence@gmail.com> --- include/linux/bvec.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)