Re: [PATCH 5.19 2/3] Revert "io_uring: add buffer selection support to IORING_OP_NOP"

[Dylan Yudaken <dylany@fb.com>]

On Tue, 2022-06-14 at 12:26 -0600, Jens Axboe wrote:
> On 6/14/22 12:21 PM, Dylan Yudaken wrote:
> > On Tue, 2022-06-14 at 17:51 +0100, Pavel Begunkov wrote:
> > > This reverts commit 3d200242a6c968af321913b635fc4014b238cba4.
> > > 
> > > Buffer selection with nops was used for debugging and
> > > benchmarking
> > > but
> > > is useless in real life. Let's revert it before it's released.
> > > 
> > > Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> > > ---
> > >  fs/io_uring.c | 15 +--------------
> > >  1 file changed, 1 insertion(+), 14 deletions(-)
> > > 
> > > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > > index bf556f77d4ab..1b95c6750a81 100644
> > > --- a/fs/io_uring.c
> > > +++ b/fs/io_uring.c
> > > @@ -1114,7 +1114,6 @@ static const struct io_op_def io_op_defs[]
> > > = {
> > >         [IORING_OP_NOP] = {
> > >                 .audit_skip             = 1,
> > >                 .iopoll                 = 1,
> > > -               .buffer_select          = 1,
> > >         },
> > >         [IORING_OP_READV] = {
> > >                 .needs_file             = 1,
> > > @@ -5269,19 +5268,7 @@ static int io_nop_prep(struct io_kiocb
> > > *req,
> > > const struct io_uring_sqe *sqe)
> > >   */
> > >  static int io_nop(struct io_kiocb *req, unsigned int
> > > issue_flags)
> > >  {
> > > -       unsigned int cflags;
> > > -       void __user *buf;
> > > -
> > > -       if (req->flags & REQ_F_BUFFER_SELECT) {
> > > -               size_t len = 1;
> > > -
> > > -               buf = io_buffer_select(req, &len, issue_flags);
> > > -               if (!buf)
> > > -                       return -ENOBUFS;
> > > -       }
> > > -
> > > -       cflags = io_put_kbuf(req, issue_flags);
> > > -       __io_req_complete(req, issue_flags, 0, cflags);
> > > +       __io_req_complete(req, issue_flags, 0, 0);
> > >         return 0;
> > >  }
> > >  
> > 
> > The liburing test case I added in "buf-ring: add tests that cycle
> > through the provided buffer ring" relies on this.
> 
> Good point.
> 
> > I don't mind either way if this is kept or that liburing patch is
> > reverted, but it should be consistent. What do you think?
> 
> It was useful for benchmarking as well, but it'd be a trivial patch
> to
> do for targeted testing.
> 
> I'm fine with killing it, but can also be persuaded not to ;-)
> 

I guess it's better to kill the liburing test which can always be made
to work with something other than NOP, than keeping code in the kernel
just for a liburing test..

I can send a revert now