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
