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

[Jens Axboe <axboe@kernel.dk>]

On 1/24/26 3:44 AM, Pavel Begunkov wrote:
> 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.

If you have time, would you mind checking the current repo and sending a
patch? Looks like I messed up a bit and committed some of these bits
with:

commit 5d1c94f754f3bca156d84b4ebcaf2f211dcd09f2
Author: Jens Axboe <axboe@kernel.dk>
Date:   Thu Jan 22 17:11:53 2026 -0700

    test/sqe-mixed-noop: add SQ_REWIND testing

inadvertently as I was doing some testing.

-- 
Jens Axboe