On 3/28/23 1:16?PM, Linus Torvalds wrote: > On Tue, Mar 28, 2023 at 12:05?PM Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> 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
