Re: [PATCH for-next v7 4/5] block: add helper to map bvec iterator for passthrough

[Christoph Hellwig <hch@lst.de>]

On Thu, Sep 22, 2022 at 08:53:31PM +0530, Kanchan Joshi wrote:
>> blk_rq_map_user_iov really should be able to detect that it is called
>> on a bvec iter and just do the right thing rather than needing different
>> helpers.
>
> I too explored that possibility, but found that it does not. It maps the
> user-pages into bio either directly or by doing that copy (in certain odd
> conditions) but does not know how to deal with existing bvec.

What do you mean with existing bvec?  We allocate a brand new bio here
that we want to map the next chunk of the iov_iter to, and that
is exactly what blk_rq_map_user_iov does.  What blk_rq_map_user_iov
currently does not do is to implement this mapping efficiently
for ITER_BVEC iters, but that is something that could and should
be fixed.

> And it really felt cleaner to me write a new function rather than 
> overloading the blk_rq_map_user_iov with multiple if/else canals.

No.  The whole point of the iov_iter is to support this "overload".

> But iov_iter_gap_alignment does not work on bvec iters. Line #1274 below

So we'll need to fix it.

> 1264 unsigned long iov_iter_gap_alignment(const struct iov_iter *i)
> 1265 {
> 1266         unsigned long res = 0;
> 1267         unsigned long v = 0;
> 1268         size_t size = i->count;
> 1269         unsigned k;
> 1270
> 1271         if (iter_is_ubuf(i))
> 1272                 return 0;
> 1273
> 1274         if (WARN_ON(!iter_is_iovec(i)))
> 1275                 return ~0U;
>
> Do you see a way to overcome this. Or maybe this can be revisted as we
> are not missing a lot?

We just need to implement the equivalent functionality for bvecs.  It
isn't really hard, it just wasn't required so far.