Re: [PATCH 2/4] io_uring: switch deferred task_work to an io_wq_work_list

[Jens Axboe <axboe@kernel.dk>]

On 3/27/24 11:28 AM, Pavel Begunkov wrote:
> On 3/27/24 16:37, Jens Axboe wrote:
>> On 3/27/24 9:45 AM, Jens Axboe wrote:
>>>> smp_mb(), see the comment below, and fwiw "_after_atomic" would not
>>>> work.
>>>
>>> For this one, I think all we need to do is have the wq_list_empty()
>>> check be fully stable. If we read:
>>>
>>> nr_wait = atomic_read(&ctx->cq_wait_nr);
>>>
>>> right before a waiter does:
>>>
>>> atomic_set(&ctx->cq_wait_nr, foo);
>>> set_current_state(TASK_INTERRUPTIBLE);
>>>
>>> then we need to ensure that the "I have work" check in
>>> io_cqring_wait_schedule() sees the work. The spin_unlock() has release
>>> semantics, and the current READ_ONCE() for work check sbould be enough,
>>> no?
>>
>> To answer my own question - no, it's not enough. Let me think about this
>> a bit.
> 
> Right, to my knowledge release does nothing for write; read;
> ordering, and all ops after can leak before the barrier.

Yeah, it needs an smp_mb() before that atomic_read() on the task work
add side.

-- 
Jens Axboe