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

Christoph Hellwig
2022-09-23T15:29:51Z

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.