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.
--
Jens Axboe
