Re: [PATCH liburing 0/2] Add support for IORING_SETUP_SQ_REWIND

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

On 1/23/26 19:04, Jens Axboe wrote:
> On 1/23/26 7:14 AM, Pavel Begunkov wrote:
>> On 1/22/26 23:05, Jens Axboe wrote:
>>> On 1/22/26 3:59 PM, Jens Axboe wrote:
>>>>
>>>> On Wed, 21 Jan 2026 22:23:20 +0000, Pavel Begunkov wrote:
>>>>> Add liburing support and tests for IORING_SETUP_SQ_REWIND.
>>>>>
>>>>> Pavel Begunkov (2):
>>>>>     src/queue: Add support for non circular SQ
>>>>>     tests: add SETUP_SQ_REWIND tests
>>>>>
>>>>> src/include/liburing.h          |  5 ++++-
>>>>>    src/include/liburing/io_uring.h | 12 ++++++++++++
>>>>>    src/queue.c                     |  5 +++++
>>>>>    test/test.h                     |  2 ++
>>>>>    4 files changed, 23 insertions(+), 1 deletion(-)
>>>>>
>>>>> [...]
>>>>
>>>> Applied, thanks!
>>>>
>>>> [1/2] src/queue: Add support for non circular SQ
>>>>         commit: c22129cf0b8c936eb478d920ef84e53d89c6a5cc
>>>> [2/2] tests: add SETUP_SQ_REWIND tests
>>>>         commit: 346c063d16bda52f02d00feb744aafe35b4002a9
>>>
>>> Hmm I do think you're missing some spots though, no?
>>>
>>> diff --git a/src/include/liburing.h b/src/include/liburing.h
>>> index 987b28aaf99e..016be1e80ef2 100644
>>> --- a/src/include/liburing.h
>>> +++ b/src/include/liburing.h
>>> @@ -1702,8 +1702,13 @@ IOURINGINLINE unsigned io_uring_load_sq_head(const struct io_uring *ring)
>>>    IOURINGINLINE unsigned io_uring_sq_ready(const struct io_uring *ring)
>>>        LIBURING_NOEXCEPT
>>>    {
>>> +    unsigned head = 0;
>>> +
>>> +    if (!(ring->flags & IORING_SETUP_SQ_REWIND))
>>> +        head = io_uring_load_sq_head(ring);
>>
>> The head should already be zero. Actually, sounds like the get_sqe
>> hunk from the patch is not needed either.
> 
> Yeah agree, I think they are both false alarms. Might warrant a comment
> though. Or maybe we just fold it into io_uring_load_sq_head()?

What I'm implying is that the

+	if (!(ring->flags & IORING_SETUP_SQ_REWIND))
+		head = io_uring_load_sq_head(ring);
+

change from the original patch for normal 64B _io_uring_get_sqe()
doesn't seem to be necessary. I need to take a look, but that's
a good thing since the function is somewhat frequently called and
inlined.

That would leave __io_uring_flush_sq() to be the only place
checking the flag, so maybe comments would better to be put
there.

-- 
Pavel Begunkov