On Mon, Nov 6, 2023 at 2:51 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
>
> On 11/6/23 7:32 AM, Jens Axboe wrote:
> > On 11/5/23 3:30 PM, Dylan Yudaken wrote:
> >> For addr: this field is not used, since buffer select is forced. But by forcing
> >> it to be zero it leaves open future uses of the field.
> >>
> >> len is actually usable, you could imagine that you want to receive
> >> multishot up to a certain length.
> >> However right now this is not how it is implemented, and it seems
> >> safer to force this to be zero.
> >>
> >> Fixes: fc68fcda0491 ("io_uring/rw: add support for IORING_OP_READ_MULTISHOT")
> >> Signed-off-by: Dylan Yudaken <dyudaken@xxxxxxxxx>
> >> ---
> >> io_uring/rw.c | 7 +++++++
> >> 1 file changed, 7 insertions(+)
> >>
> >> diff --git a/io_uring/rw.c b/io_uring/rw.c
> >> index 1c76de483ef6..ea86498d8769 100644
> >> --- a/io_uring/rw.c
> >> +++ b/io_uring/rw.c
> >> @@ -111,6 +111,13 @@ int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
> >> rw->len = READ_ONCE(sqe->len);
> >> rw->flags = READ_ONCE(sqe->rw_flags);
> >>
> >> + if (req->opcode == IORING_OP_READ_MULTISHOT) {
> >> + if (rw->addr)
> >> + return -EINVAL;
> >> + if (rw->len)
> >> + return -EINVAL;
> >> + }
> >
> > Should we just put these in io_read_mshot_prep() instead? Ala the below.
> > In general I think it'd be nice to have a core prep_rw, and then each
> > variant will have its own prep. Then we can get away from random opcode
> > checking in there.
> >
> > I do agree with the change in general, just think we can tweak it a bit
> > to make it a bit cleaner.
>
> Sent out two cleanups that take it in this direction in general, fwiw.
Yes - I think this approach is better, will rebase on these
