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

Pavel Begunkov
2024-10-09T19:46:57Z

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.

> and yeah just put it in the io_uring_prep_recv_zc() or whatever helper.
> That would seem to be a lot more consistent with other users, no?

--
Pavel Begunkov