Re: [PATCHSET v6 0/11] Turn single segment imports into ITER_UBUF

[Jens Axboe <axboe@kernel.dk>]

On 3/29/23 1:44 PM, Linus Torvalds wrote:
> On Wed, Mar 29, 2023 at 11:41 AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> Passes testing, and verified we do the right thing for 1 and multi
>> segments.
> 
> Apart from the pointer casting rant, this looks sane to me.
> 
> I feel like 02/11 has a few potential cleanups:
> 
>  (a) it feels like a few too many "iter.__iov" uses remaining, but
> they mostly (all?) look like assignments.
> 
> I do get the feeling that any time you assign __iov, you should also
> assign "nr_segs", and it worries me a bit that I see one without the
> other. Maybe room for another helper that enforces a "if you set the
> __iov pointer, you must be setting nr_segs too"?
> 
> And maybe I'm just being difficult.

No, I think that's valid, and the cover letter does touch upon that.
The thought of doing an iov assign helper has occurred to me as well.
I just wanted to get general feelings on the direction first, then
do a round of polish when prudent rather than prematurely.

>  (b) I see at least one "iov = iter_iov(from)" that precedes a later
> check for "iter_is_iovec()", which again means that *if* we add some
> debug sanity test to "iter_iov()", it might trigger when it shouldn't?
> 
> The one I see is in snd_pcm_writev(), but I th ink the same thing
> happens in snd_pcm_readv() but just isn't visible in the patch due to
> not having the context lines.

I think that's mostly a patch ordering issue. Should probably just
push the sound and IB patches to the front of the series.

-- 
Jens Axboe