Re: [PATCH] io_uring: fix memory ordering when SQPOLL thread goes to sleep

[Jens Axboe <axboe@kernel.dk>]

On 3/6/22 6:32 AM, Almog Khaikin wrote:
> On 3/6/22 12:35, Almog Khaikin wrote:
>> Without a full memory barrier between the store to the flags and the
>> load of the SQ tail the two operations can be reordered and this can
>> lead to a situation where the SQPOLL thread goes to sleep while the
>> application writes to the SQ tail and doesn't see the wakeup flag.
>> This memory barrier pairs with a full memory barrier in the application
>> between its store to the SQ tail and its load of the flags.
> 
> The IOPOLL list is internal to the kernel, userspace doesn't interact
> with it. AFAICT it can't cause any races with userspace so the check if
> the list is empty seems unnecessary. The flags and the SQ tail are the
> only things that are shared that can cause any problems when the kernel
> thread goes to sleep so I think it's safe to remove that check.
> 
> The race here can result in a situation where the kernel thread goes to
> sleep while the application updates the SQ tail and doesn't see the
> NEED_WAKEUP flag. Checking the SQ tail after setting the wakeup flag
> along with the full barrier would ensure that either we see the tail
> update or the application sees the wakeup flag. The IOPOLL list doesn't
> tie into any of this.

I think you're mixing up two different things, and even if not, the
IOPOLL change should be a separate change.

The iopoll list check isn't about synchronizing with userspace, it's
about not going to sleep if we have entries to reap. If we're running
with IOPOLL|SQPOLL, then it's the sq poll thread that does the polling
and reaping.

-- 
Jens Axboe