Re: [PATCH 2/2] io_uring: introduce non-circular SQ

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

On 10/14/25 19:37, Caleb Sander Mateos wrote:
> On Tue, Oct 14, 2025 at 3:57 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
...>> + * SQEs always start at index 0 in the submission ring instead of using a
>> + * wrap around indexing.
>> + */
>> +#define IORING_SETUP_SQ_REWIND         (1U << 19)
> 
> Keith's mixed-SQE-size patch series is already planning to use this
> flag: https://lore.kernel.org/io-uring/20251013180011.134131-3-kbusch@meta.com/

I'll rebase it as ususual if that gets merged before.
>> -       /*
>> -        * Ensure any loads from the SQEs are done at this point,
>> -        * since once we write the new head, the application could
>> -        * write new data to them.
>> -        */
>> -       smp_store_release(&rings->sq.head, ctx->cached_sq_head);
>> +       if (ctx->flags & IORING_SETUP_SQ_REWIND) {
>> +               ctx->cached_sq_head = 0;
> 
> The only awkward thing about this interface seems to be if
> io_submit_sqes() aborts early without submitting all the requested
> SQEs. Does userspace then need to memmove() the remaining SQEs to the
> start of the ring? It's certainly an unlikely case but something
> userspace has to handle because io_alloc_req() can fail for reasons
> outside its control. Seems like it might simplify the userspace side
> if cached_sq_head wasn't rewound if not all SQEs were consumed.
This kind of special rules is what usually makes interfaces a pain to
work with. What if you want to abort all un-submitted requests
instead? You can empty the queue, but then the next syscall will
still start from the middle. Or what if the application wants to
queue more requests before resubmitting previous ones? There are
reasons b/c the kernel will need to handle it in a less elegant way
than it potentially can otherwise. memmove sounds appropriate.

>> @@ -3678,6 +3687,12 @@ static int io_uring_sanitise_params(struct io_uring_params *p)
>>   {
>>          unsigned flags = p->flags;
>>
>> +       if (flags & IORING_SETUP_SQ_REWIND) {
>> +               if ((flags & IORING_SETUP_SQPOLL) ||
>> +                   !(flags & IORING_SETUP_NO_SQARRAY))
> 
> Is there a reason IORING_SETUP_NO_SQARRAY is required? It seems like
> the implementation would work just fine with the SQ indirection ring;
> the rewind would just apply to the indirection ring instead of the SQE
> array. The cache hit rate benefit would probably be smaller since many
> more SQ indirection entries fit in a single cache line, but I don't
> see a reason to explicitly forbid it.

B/c I don't care about sqarray setups, they are on the way out for soft
deprecation with liburing defaulting to NO_SQARRAY, and once you try
to optimise the kernel IORING_SETUP_SQ_REWIND handling it might turn
out that !NO_SQARRAY is in the way... or not, but you can always allow
it later while limiting it would break uapi. In short, it's weighting
chances of (micro) optimisations in the future vs supporting a case
which is unlikely going to be used.

-- 
Pavel Begunkov