Re: [PATCH 1/2 v2] io_uring: Fix race condition when sqp thread goes to sleep

From: Olivier Langlois
Date: Wed Jun 23 2021 - 09:52:59 EST


On Wed, 2021-06-23 at 00:03 +0100, Pavel Begunkov wrote:
> On 6/22/21 11:42 PM, Olivier Langlois wrote:
> > On Tue, 2021-06-22 at 18:37 -0400, Olivier Langlois wrote:
> > > On Tue, 2021-06-22 at 21:45 +0100, Pavel Begunkov wrote:
> > >
> > >
> > > I can do that if you want but considering that the function is
> > > inline
> > > and the race condition is a relatively rare occurence, is the
> > > cost
> > > coming with inline expansion really worth it in this case?
> > > >
> > On hand, there is the inline expansion concern.
> >
> > OTOH, the benefit of going with your suggestion is that completions
> > generally precedes new submissions so yes, it might be better that
> > way.
> >
> > I'm really unsure about this. I'm just raising the concern and I'll
> > let
> > you make the final decision...
>
> It seems it may actually loop infinitely until it gets a signal,
> so yes. And even if not, rare stalls are nasty, they will ruin
> some 9s of latency and hard to catch.
>
> That part is quite cold anyway, would generate some extra cold
> instructions, meh
>
I'm not 100% sure to see the infinite loop possibility but I guess that
with some badly placed preemptions, it could take few iterations before
entering the block:

if (sqt_spin || !time_after(jiffies, timeout)) {

So I will go ahead with your suggestion.

I'll retest the new patch version (it should be a formality) and I'll
resend an update once done.

Greetings,