public inbox for [email protected]
 help / color / mirror / Atom feed
From: Olivier Langlois <[email protected]>
To: Pavel Begunkov <[email protected]>,
	Jens Axboe <[email protected]>,
	[email protected], [email protected]
Subject: Re: [PATCH 1/2 v2] io_uring: Fix race condition when sqp thread goes to sleep
Date: Tue, 22 Jun 2021 18:42:20 -0400	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On Tue, 2021-06-22 at 18:37 -0400, Olivier Langlois wrote:
> On Tue, 2021-06-22 at 21:45 +0100, Pavel Begunkov wrote:
> > On 6/22/21 7:55 PM, Olivier Langlois wrote:
> > > If an asynchronous completion happens before the task is
> > > preparing
> > > itself to wait and set its state to TASK_INTERRUPTIBLE, the
> > > completion
> > > will not wake up the sqp thread.
> > > 
> > > Signed-off-by: Olivier Langlois <[email protected]>
> > > ---
> > >  fs/io_uring.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/fs/io_uring.c b/fs/io_uring.c
> > > index fc8637f591a6..02f789e07d4c 100644
> > > --- a/fs/io_uring.c
> > > +++ b/fs/io_uring.c
> > > @@ -6902,7 +6902,7 @@ static int io_sq_thread(void *data)
> > >                 }
> > >  
> > >                 prepare_to_wait(&sqd->wait, &wait,
> > > TASK_INTERRUPTIBLE);
> > > -               if (!io_sqd_events_pending(sqd)) {
> > > +               if (!io_sqd_events_pending(sqd) && !current-
> > > > task_works) {
> > 
> > Agree that it should be here, but we also lack a good enough
> > task_work_run() around, and that may send the task burn CPU
> > for a while in some cases. Let's do
> > 
> > if (!io_sqd_events_pending(sqd) && !io_run_task_work())
> >    ...
> 
> 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...



  reply	other threads:[~2021-06-22 22:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-22 18:45 [PATCH 0/2] Minor SQPOLL thread fix and improvement Olivier Langlois, Olivier Langlois
2021-06-22 18:45 ` [PATCH 1/2] io_uring: Fix race condition when sqp thread goes to sleep Olivier Langlois
2021-06-22 18:53   ` Olivier Langlois
2021-06-22 18:55     ` Pavel Begunkov
2021-06-22 19:07       ` Olivier Langlois
     [not found]   ` <[email protected]>
2021-06-22 20:45     ` [PATCH 1/2 v2] " Pavel Begunkov
2021-06-22 22:37       ` Olivier Langlois
2021-06-22 22:42         ` Olivier Langlois [this message]
2021-06-22 23:03           ` Pavel Begunkov
2021-06-23 13:52             ` Olivier Langlois
2021-06-23 15:58               ` Pavel Begunkov
2021-06-22 18:45 ` [PATCH 2/2] io_uring: Create define to modify a SQPOLL parameter Olivier Langlois
2021-06-22 20:58   ` Pavel Begunkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=169899caad96c3214d6e380ac7686d054eed3b12.camel@trillion01.com \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox