Re: [PATCH 5/8] IB/hfi1: make hfi1_write_iter() deal with ITER_UBUF iov_iter

[Jens Axboe <axboe@kernel.dk>]

On 3/28/23 1:16?PM, Linus Torvalds wrote:
> On Tue, Mar 28, 2023 at 12:05?PM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> But it's not like adding a 'struct iovec' explicitly to the members
>> just as extra "code documentation" would be wrong.
>>
>> I don't think it really helps, though, since you have to have that
>> other explicit structure there anyway to get the member names right.
> 
> Actually, thinking a bit more about it, adding a
> 
>     const struct iovec xyzzy;
> 
> member might be a good idea just to avoid a cast. Then that
> iter_ubuf_to_iov() macro becomes just
> 
>    #define iter_ubuf_to_iov(iter) (&(iter)->xyzzy)
> 
> and that looks much nicer (plus still acts kind of as a "code comment"
> to clarify things).

I went down this path, and it _mostly_ worked out. You can view the
series here, I'll send it out when I've actually tested it:

https://git.kernel.dk/cgit/linux-block/log/?h=iter-ubuf

A few mental notes I made along the way:

- The IB/sound changes are now just replacing an inappropriate
  iter_is_iovec() with iter->user_backed. That's nice and simple.

- The iov_iter_iovec() case becomes a bit simpler. Or so I thought,
  because we still need to add in the offset so we can't just use
  out embedded iovec for that. The above branch is just using the
  iovec, but I don't think this is right.

- Looks like it exposed a block bug, where the copy in
  bio_alloc_map_data() was obvious garbage but happened to work
  before.

I'm still inclined to favor this approach over the previous, even if the
IB driver is a pile of garbage and lighting it a bit more on fire would
not really hurt.

Opinions? Or do you want me to just send it out for easier reading


-- 
Jens Axboe