Re: [PATCH V11 03/19] block: introduce bio_for_each_bvec()

From: Christoph Hellwig
Date: Thu Nov 22 2018 - 05:23:14 EST


On Thu, Nov 22, 2018 at 06:15:28PM +0800, Ming Lei wrote:
> > while (bytes) {
> > - unsigned segment_len = segment_iter_len(bv, *iter);
> > -
> > - if (max_seg_len < BVEC_MAX_LEN)
> > - segment_len = min_t(unsigned, segment_len,
> > - max_seg_len -
> > - bvec_iter_offset(bv, *iter));
> > + unsigned iter_len = bvec_iter_len(bv, *iter);
> > + unsigned len = min(bytes, iter_len);
>
> It may not work to always use bvec_iter_len() here, and 'segment_len'
> should be max length of the passed 'bv', however we don't know if it is
> single-page or mutli-page bvec if no one tells us.

The plain revert I sent isn't totally correct in your current tree
indeed because of the odd change that segment_iter_len now is the
full bvec len, so it should be changed to that until we switch to the
sane naming scheme used elsewhere.

But except for that, it will work. The bvec we operate on (part of the
array in the bio pointed to by the iter) is always a multi-page capable
one. We only fake up a single page on for users using segment_iter_len.

Think of what you are doing in the (I think mostly hypothetical) case
that we call bvec_iter_advance with a bytes > PAGE_SIZE on a segment
small than page size.

In this case we will limit the current iteration to the while loop
until we git a page boundary. This means we will not hit the condition
moving to the next (real, in the array) bvec at the end of the loop,
and just go on into the next iteration of the loop with the reduced
amount of bytes. Depending on how large byte is this might repeat
a few more times. Then on the final one we hit the limit and move
to the next (real, in the array) bvec.

Now if we don't have the segment limit we do exactly the same thing,
just a whole lot more efficiently in a single loop iteration.
tree where segment_iter_len is the