Re: [PATCH 1/4] fs: make do_loop_readv_writev() deal with ITER_UBUF

[Linus Torvalds <torvalds@linux-foundation.org>]

On Mon, Mar 27, 2023 at 11:04 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -748,10 +748,21 @@ static ssize_t do_loop_readv_writev(struct file *filp, struct iov_iter *iter,
>         if (flags & ~RWF_HIPRI)
>                 return -EOPNOTSUPP;
>
> +       if (WARN_ON_ONCE(iter->iter_type != ITER_IOVEC &&
> +                        iter->iter_type != ITER_UBUF))
> +               return -EINVAL;

Hmm. I think it might actually be nicer for the "iter_is_ubuf(iter)"
case to be outside the loop entirely, and be done before this
WARN_ON_ONCE().

If it's a single ITER_UBUF, that code really shouldn't loop at all -
it's literally just the old case of "call ->read/write with a single
buffer".

So i think I'd prefer this patch to be something along the lines of
this instead:

  --- a/fs/read_write.c
  +++ b/fs/read_write.c
  @@ -748,6 +748,19 @@ static ssize_t do_loop_readv_writev(struct file
*filp, struct iov_iter *iter,
        if (flags & ~RWF_HIPRI)
                return -EOPNOTSUPP;

  +     /* Single user buffer? */
  +     if (iter_is_ubuf(iter)) {
  +             void __user *addr = iter->ubuf + iter->iov_offset;
  +             size_t len = iov_iter_count(iter);
  +
  +             if (type == READ)
  +                     return filp->f_op->read(filp, addr, len, ppos);
  +             return filp->f_op->write(filp, addr, len, ppos);
  +     }
  +
  +     if (WARN_ON_ONCE(!iter_is_iovec(iter)))
  +             return -EINVAL;
  +
        while (iov_iter_count(iter)) {
                struct iovec iovec = iov_iter_iovec(iter);
                ssize_t nr;

which keeps the existing iovec loop, and just adds that "is it a
simple buffer" case at the top (and the WARN_ON_ONCE() for any other
iter type).

Hmm?

                         Linus