Re: [PATCH 1/1] io_uring/zctx: separate notification user_data

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

On 2/16/26 17:27, Jens Axboe wrote:
...
>> There are already 6, it'll be 7th. I also have one or two more in mind,
>> that's already over the half. The same was probably thought about
>> sqe->flags, and even though it's twice as many bits for net, those
>> are taken faster as potential cost of redesign is lower.
>>
>> Fwiw, the code is nastier as well, more branchy and away from
>> other notification init because of dependency on reading the
>> flags.
>>
>> @@ -1331,7 +1333,7 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>   
>>       zc->done_io = 0;
>>   
>> -    if (unlikely(READ_ONCE(sqe->__pad2[0]) || READ_ONCE(sqe->addr3)))
>> +    if (unlikely(READ_ONCE(sqe->__pad2[0])))
>>           return -EINVAL;
>>       /* we don't support IOSQE_CQE_SKIP_SUCCESS just yet */
>>       if (req->flags & REQ_F_CQE_SKIP)
>> @@ -1358,6 +1360,13 @@ int io_send_zc_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>>           }
>>       }
>>   
>> +    if (zc->flags & IORING_SEND_ZC_NOTIF_USER_DATA) {
>> +        notif->cqe.user_data = READ_ONCE(sqe->addr3);
>> +    } else {
>> +        if (READ_ONCE(sqe->addr3))
>> +            return -EINVAL;
>> +    }
>> +
> 
> I think just remove the else part here - addr3 is valid now that
> IORING_SEND_ZC_NOTIF_USER_DATA is supported, and if you mess it up in
> your applications, you'll find this via development anyway. Since addr3
> == 0 is a valid value, it doesn't make much sense to check for it being
> non-zero.

Gating it on a separate flag but not checking when not set makes
it only more confusing in terms of why would you do a flag in
the first place.

It's not like a flags field where any value set would be an
> -EINVAL case. Doesn't even exclude having another flag for using addr3
> for something else anyway.

You can override the behaviour with another flag in either case,
but realistically it's better to avoid as it's always messy,
unless the features are clearly exclusive.

I know there is no way to convince you, but v2 already degraded
the uapi as per requested, can we have that one? The "else" branch
doesn't make the api worse, on the opposite.

-- 
Pavel Begunkov