Re: [PATCH 4/4] io_uring/rw: pre-mapped rw attributes

[Pavel Begunkov <asml.silence@gmail.com>]

On 1/6/25 11:13, lizetao wrote:
...
>>   /* sqe->attr_type_mask flags */
>> -#define IORING_RW_ATTR_FLAG_PI	(1U << 0)
>> +#define IORING_RW_ATTR_FLAG_PI		(1UL << 0)
>> +#define IORING_RW_ATTR_REGISTERED	(1UL << 63)
> Why use (1UL << 63) instead of (1UL << 1) here?

They serve different purposes, think how you'd be iterating
more attribute types, calculating the total size, etc.

>> diff --git a/io_uring/rw.c b/io_uring/rw.c index dc1acaf95db1..b1db4595788b
>> 100644
>> --- a/io_uring/rw.c
>> +++ b/io_uring/rw.c
>> @@ -271,10 +271,17 @@ static int io_prep_rw_pi(struct io_kiocb *req, struct
>> io_rw *rw, int ddir,
>>   	size_t pi_len;
>>   	int ret;
>>
>> -	if (copy_from_user(&__pi_attr, u64_to_user_ptr(attr_ptr),
>> -	    sizeof(pi_attr)))
>> -		return -EFAULT;
>> -	pi_attr = &__pi_attr;
>> +	if (attr_type_mask & IORING_RW_ATTR_REGISTERED) {
>> +		pi_attr = io_args_get_ptr(&req->ctx->sqe_args, attr_ptr,
>> +					  sizeof(pi_attr));
> Here pi_attr is just pointer, so maybe sizeof(__pi_attr) or sizeof(struct io_uring_attr_pi) will be better.

Good catch, and that's an issue from 3/4. As noted it's not even
tested and posted to discuss the API. I'd even prefer them to be
thrown away, and for Anuj / Kanchan to take over if that's
interesting.

-- 
Pavel Begunkov