Re: [PATCH v1 12/15] io_uring/zcrx: add io_recvzc request

Jens Axboe
2024-10-09T19:51:01Z

On 10/9/24 1:47 PM, Pavel Begunkov wrote:
> On 10/9/24 20:42, Jens Axboe wrote:
>> On 10/9/24 1:27 PM, Pavel Begunkov wrote:
>>>>>>> + /* All data completions are posted as aux CQEs. */
>>>>>>> + req->flags |= REQ_F_APOLL_MULTISHOT;
>>>>>>
>>>>>> This puzzles me a bit...
>>>>>
>>>>> Well, it's a multishot request. And that flag protects from cq
>>>>> locking rules violations, i.e. avoiding multishot reqs from
>>>>> posting from io-wq.
>>>>
>>>> Maybe make it more like the others and require that
>>>> IORING_RECV_MULTISHOT is set then, and set it based on that?
>>>
>>> if (IORING_RECV_MULTISHOT)
>>> return -EINVAL;
>>> req->flags |= REQ_F_APOLL_MULTISHOT;
>>>
>>> It can be this if that's the preference. It's a bit more consistent,
>>> but might be harder to use. Though I can just hide the flag behind
>>> liburing helpers, would spare from neverending GH issues asking
>>> why it's -EINVAL'ed
>>
>> Maybe I'm missing something, but why not make it:
>>
>> /* multishot required */
>> if (!(flags & IORING_RECV_MULTISHOT))
>> return -EINVAL;
>> req->flags |= REQ_F_APOLL_MULTISHOT;
>
> Right, that's what I meant before spewing a non sensible snippet.

ok phew, I was scratching my head there for a bit... All good then.

--
Jens Axboe