Re: [PATCH 4/6] io_uring: switch normal task_work to a mpscq

On 6/15/26 12:33 PM, Caleb Sander Mateos wrote:
> On Sat, Jun 13, 2026 at 5:08?AM Jens Axboe <[email protected]> wrote:
>>
>> On 6/12/26 8:26 PM, Caleb Sander Mateos wrote:
>>> On Fri, Jun 12, 2026 at 12:37?PM Jens Axboe <[email protected]> wrote:
>>>>
>>>> On 6/12/26 12:59 PM, Caleb Sander Mateos wrote:
>>>>>> @@ -236,10 +262,14 @@ void io_req_normal_work_add(struct io_kiocb *req)
>>>>>>                 return;
>>>>>>         }
>>>>>>
>>>>>> +       /* task_work must only be added once */
>>>>>> +       if (test_and_set_bit(0, &tctx->tw_pending))
>>>>>> +               return;
>>>>>
>>>>> Is tw_pending necessary? How come the task_work_add() exclusivity
>>>>> isn't already provided by the mpscq_push() check above?
>>>>
>>>> It is, because the transition from empty -> not-empty no longer works
>>>> for that, as the mpscq emtpies one-by-one rather than with a delete-all
>>>> kind of primitive.
>>>
>>> Sorry, I'm still not following why the empty check doesn't suffice.
>>> It's true that mpscq elements can be removed from the head one at a
>>> time, but mpscq_push() will continue to return false until the
>>> consumer pops all the elements and successfully sets tail back to
>>> &stub. mpscq_push() will return true once when tail transitions away
>>> from &stub, and then not again until the task work runs and sets tail
>>> back to &stub.
>>
>> Let's say the task_work is currently running, a producer is adding more.
>> It finds queue empty, re-adds the task_work. That part is fine, we can
>> add the task_work while it's running as it has been detached already.
>> The task_work keeps running and also prunes this new item. Producer adds
>> another one, finds the queue empty, re-adds task_work. This one is not
>> OK, the task_work was already re-added when it previously found it
>> empty. Boom.
> 
> Ah right, I forgot that mpscq_pop() can both return a popped node and
> set the tail back to &stub. Maybe it would make sense for it to return
> whether the queue has been marked empty and break out of
> tctx_task_work_run() in that case instead of relying on a separate
> call to mpscq_empty()? The atomic RMW for tw_pending every time the
> queue transitions between empty and non-empty seems like it could be
> quite expensive.

We could tweak it like that. I didn't look too closely as this is the
!DEFER case and hence a lot less interesting, but if you want to send a
patch my way I'd be happy to stage it on top.

-- 
Jens Axboe