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
