Re: [PATCH 2/6] io_uring/mpscq: add lockless multi-producer, single-consumer FIFO queue

On 6/12/26 8:40 PM, Caleb Sander Mateos wrote:
>> diff --git a/io_uring/mpscq.h b/io_uring/mpscq.h
>> new file mode 100644
>> index 000000000000..bc482d10e0f3
>> --- /dev/null
>> +++ b/io_uring/mpscq.h
>> @@ -0,0 +1,118 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef IOU_MPSCQ_H
>> +#define IOU_MPSCQ_H
> 
> #include <linux/io_uring_types.h> so this header can compile on its own?

Sure, we can do that.

>> +static inline struct llist_node *mpscq_pop(struct mpscq *q,
>> +                                          struct llist_node **headp)
>> +{
>> +       struct llist_node *head = *headp, *next;
>> +
>> +       if (head == &q->stub) {
>> +               head = READ_ONCE(head->next);
>> +               if (!head)
>> +                       return NULL;
>> +               *headp = head;
>> +       }
>> +       next = READ_ONCE(head->next);
>> +       if (next) {
>> +               *headp = next;
>> +               return head;
>> +       }
>> +       /*
>> +        * 'head' is the last linked node, it can only be handed out once the
>> +        * stub has taken its place as the tail. If the cmpxchg fails, a
>> +        * producer has made a new node the tail but hasn't linked 'head' to
>> +        * it yet - bail and let the caller retry.
>> +        */
>> +       q->stub.next = NULL;
> 
> I think this could be moved before *headp = head. That way it only
> runs once each time the queue becomes nonempty rather than on every
> attempt to switch tail back to &stub. And it would keep next =
> READ_ONCE(head->next) and try_cmpxchg(&q->tail, &head, &q->stub))
> closer together, reducing the window where the consumer could lose the
> race to pop the last element.

That's a nice observation! Yes, that looks correct to me, I'll fold it
in.

> Other that that,
> Reviewed-by: Caleb Sander Mateos <[email protected]>

Thanks!

-- 
Jens Axboe