lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 19 Nov 2018 11:31:11 +0800 From: Ming Lei <ming.lei@...hat.com> To: Christoph Hellwig <hch@....de> Cc: Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org, Dave Chinner <dchinner@...hat.com>, Kent Overstreet <kent.overstreet@...il.com>, Mike Snitzer <snitzer@...hat.com>, dm-devel@...hat.com, Alexander Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org, Shaohua Li <shli@...nel.org>, linux-raid@...r.kernel.org, linux-erofs@...ts.ozlabs.org, David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org, "Darrick J . Wong" <darrick.wong@...cle.com>, linux-xfs@...r.kernel.org, Gao Xiang <gaoxiang25@...wei.com>, Theodore Ts'o <tytso@....edu>, linux-ext4@...r.kernel.org, Coly Li <colyli@...e.de>, linux-bcache@...r.kernel.org, Boaz Harrosh <ooo@...ctrozaur.com>, Bob Peterson <rpeterso@...hat.com>, cluster-devel@...hat.com Subject: Re: [PATCH V10 02/19] block: introduce bio_for_each_bvec() On Fri, Nov 16, 2018 at 02:30:28PM +0100, Christoph Hellwig wrote: > > +static inline void __bio_advance_iter(struct bio *bio, struct bvec_iter *iter, > > + unsigned bytes, bool mp) > > I think these magic 'bool np' arguments and wrappers over wrapper > don't help anyone to actually understand the code. I'd vote for > removing as many wrappers as we really don't need, and passing the > actual segment limit instead of the magic bool flag. Something like > this untested patch: I think this way is fine, just a little comment. > > diff --git a/include/linux/bio.h b/include/linux/bio.h > index 277921ad42e7..dcad0b69f57a 100644 > --- a/include/linux/bio.h > +++ b/include/linux/bio.h > @@ -138,30 +138,21 @@ static inline bool bio_full(struct bio *bio) > bvec_for_each_segment(bvl, &((bio)->bi_io_vec[iter_all.idx]), i, iter_all) > > static inline void __bio_advance_iter(struct bio *bio, struct bvec_iter *iter, > - unsigned bytes, bool mp) > + unsigned bytes, unsigned max_segment) The new parameter should have been named as 'max_segment_len' or 'max_seg_len'. > { > iter->bi_sector += bytes >> 9; > > if (bio_no_advance_iter(bio)) > iter->bi_size -= bytes; > else > - if (!mp) > - bvec_iter_advance(bio->bi_io_vec, iter, bytes); > - else > - mp_bvec_iter_advance(bio->bi_io_vec, iter, bytes); > + __bvec_iter_advance(bio->bi_io_vec, iter, bytes, max_segment); > /* TODO: It is reasonable to complete bio with error here. */ > } > > static inline void bio_advance_iter(struct bio *bio, struct bvec_iter *iter, > unsigned bytes) > { > - __bio_advance_iter(bio, iter, bytes, false); > -} > - > -static inline void bio_advance_mp_iter(struct bio *bio, struct bvec_iter *iter, > - unsigned bytes) > -{ > - __bio_advance_iter(bio, iter, bytes, true); > + __bio_advance_iter(bio, iter, bytes, PAGE_SIZE); > } > > #define __bio_for_each_segment(bvl, bio, iter, start) \ > @@ -177,7 +168,7 @@ static inline void bio_advance_mp_iter(struct bio *bio, struct bvec_iter *iter, > for (iter = (start); \ > (iter).bi_size && \ > ((bvl = bio_iter_mp_iovec((bio), (iter))), 1); \ > - bio_advance_mp_iter((bio), &(iter), (bvl).bv_len)) > + __bio_advance_iter((bio), &(iter), (bvl).bv_len, 0)) Even we might pass '-1' for multi-page segment. > > /* returns one real segment(multipage bvec) each time */ > #define bio_for_each_bvec(bvl, bio, iter) \ > diff --git a/include/linux/bvec.h b/include/linux/bvec.h > index 02f26d2b59ad..5e2ed46c1c88 100644 > --- a/include/linux/bvec.h > +++ b/include/linux/bvec.h > @@ -138,8 +138,7 @@ struct bvec_iter_all { > }) > > static inline bool __bvec_iter_advance(const struct bio_vec *bv, > - struct bvec_iter *iter, > - unsigned bytes, bool mp) > + struct bvec_iter *iter, unsigned bytes, unsigned max_segment) > { > if (WARN_ONCE(bytes > iter->bi_size, > "Attempted to advance past end of bvec iter\n")) { > @@ -148,18 +147,18 @@ static inline bool __bvec_iter_advance(const struct bio_vec *bv, > } > > while (bytes) { > - unsigned len; > + unsigned segment_len = mp_bvec_iter_len(bv, *iter); > > - if (mp) > - len = mp_bvec_iter_len(bv, *iter); > - else > - len = bvec_iter_len(bv, *iter); > + if (max_segment) { > + max_segment -= bvec_iter_offset(bv, *iter); > + segment_len = min(segment_len, max_segment); Looks 'max_segment' needs to be constant, shouldn't be updated. If '-1' is passed for multipage case, the above change may become: segment_len = min_t(segment_len, max_seg_len - bvec_iter_offset(bv, *iter)); This way is more clean, but with extra cost of the above line for multipage case. Thanks, Ming
Powered by blists - more mailing lists