On Sun, Apr 7, 2019 at 3:37 PM Ming Lei <tom.leiming@xxxxxxxxx> wrote: > > On Sun, Apr 7, 2019 at 2:53 PM Christoph Hellwig <hch@xxxxxx> wrote: > > > > The change itself looks fine to be, but a few comments on semingly > > unrelated changes: > > > > > +#define bio_for_each_segment_all(bvl, bio, i, iter_all) \ > > > + for (i = 0, bvl = bvec_init_iter_all(&iter_all); \ > > > + iter_all.idx < (bio)->bi_vcnt && \ > > > + (mp_bvec_advance(&((bio)->bi_io_vec[iter_all.idx]), \ > > > + &iter_all), 1); i++) > > > > Instead of the complicated expression inside the for loop test > > expression can't we move the index check into mp_bvec_advance and let > > it return a bool? > > OK, will move index check into mp_bvec_advance. oops, I recall the above line, because: 1) the index check expression is quite straight-forward 2) it has been in my todo list to re-use bvec_advance() to re-write iterate_bvec() given bvec_advance() is much light-weight than for_each_bvec(). If we move the index check into bvec_advance(), the helper has to be moved to bio.h, then we can't use that for iterate_bvec(). thanks, Ming Lei