Re: [PATCH 2/2] bio: optimise bvec iteration

[Pavel Begunkov <asml.silence@gmail.com>]

On 26/11/2020 12:32, Pavel Begunkov wrote:
> On 26/11/2020 10:02, Christoph Hellwig wrote:
>> On Tue, Nov 24, 2020 at 05:58:13PM +0000, Pavel Begunkov wrote:
>>> __bio_for_each_bvec(), __bio_for_each_segment() and bio_copy_data_iter()
>>> fall under conditions of bvec_iter_advance_single(), which is a faster
>>> and slimmer version of bvec_iter_advance(). Add
>>> bio_advance_iter_single() and convert them.
>>
>> Are you sure about bio_advance_iter()?  That API looks like it might
> 
> Both those listed bio_for_each*() pass bvl.bv_len, which is truncated to
> current segment by bio_iter_iovec() (i.e. bvec_iter_bvec) or
> mp_bvec_iter_bvec().
> 
> And just to note that I didn't change bio_advance_iter() but added a
> new function.

> There is always space for stupid mistakes, but I'm sure. What makes you
> to think opposite? I may have missed it.

Christoph, any doubts left?

>> not always be limited to a single segment, and might at least need a
>> WARN_ON_ONCE to make sure it is not abused.
> 
> I thought twice about converting other places as you commented before,
> and it looks saner to not do that exactly for that reason. I prefer
> to leave *_single() versions for rare but high impact cases like
> for_each()s.
> And as it's contained I decided to not add overhead on WARN_ONCE(),
> e.g. with inlining and w/o string dedup it grows .data section much.
> 

-- 
Pavel Begunkov