All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Felipe Balbi <balbi@kernel.org>
Subject: Re: Fundamental race condition in wait_event_interruptible_exclusive() ?
Date: Mon, 9 Dec 2019 18:38:20 +0100	[thread overview]
Message-ID: <20191209173820.GA11415@redhat.com> (raw)
In-Reply-To: <CAHk-=whiKy63tpFVUUS1sH07ce692rKcoo0ztnHw5UaPaMg8Ng@mail.gmail.com>

I have alredy replied to Ingo, but if I was not clear...

On 12/08, Linus Torvalds wrote:
>
> The reason it is buggy is that wait_event_interruptible_exclusive()
> does this (inside the __wait_event() macro that it expands to):
>
>                 long __int = prepare_to_wait_event(&wq_head,
> &__wq_entry, state);\
>
>          \
>                 if (condition)
>          \
>                         break;
>          \
>
>          \
>                 if (___wait_is_interruptible(state) && __int) {
>          \
>                         __ret = __int;
>          \
>                         goto __out;
>          \
>
> and the thing is, if does that "__ret = __int" case and returns
> -ERESTARTSYS, it's possible that the wakeup event has already been
> consumed,

Afaics, no.

> because we've added ourselves as an exclusive writer to the
> queue. So it _says_ it was interrupted, not woken up, and the wait got
> cancelled, but because we were an exclusive waiter, we might be the
> _only_ thing that got woken up,

And that is why ___wait_event() always checks the condition after
prepare_to_wait_event(), whatever it returns.

And. If it actually does "__ret = __int" and returns -ERESTARTSYS, then
this task was already removed from the list, so we should not worry about
the case when wake_up() comes after prepare_to_wait_event().

> And the basic point is that the return value
> from wait_event_interruptible_exclusive() seems to not really be
> reliable. You can't really use it -

see above ...

> even if it says you got
> interrupted, you still have to go back and check the condition and do
> the work, and only do interruptability handling after that.

This is exactly what ___wait_event() does. Even if prepare_to_wait_event()
says you got interrupted, it still checks the condition and returns success
if it is true.

Oleg.


  parent reply	other threads:[~2019-12-09 17:38 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-08 21:12 Fundamental race condition in wait_event_interruptible_exclusive() ? Linus Torvalds
2019-12-09  9:18 ` [RFC PATCH] sched/wait: Make interruptible exclusive waitqueue wakeups reliable Ingo Molnar
2019-12-09 10:27   ` Ingo Molnar
2019-12-09 13:00     ` Oleg Nesterov
2019-12-10  7:21       ` Ingo Molnar
2019-12-10 19:19         ` [PATCH] sched/wait: fix ___wait_var_event(exclusive) Oleg Nesterov
2019-12-17 12:39           ` [tip: sched/core] " tip-bot2 for Oleg Nesterov
2019-12-09 18:06     ` [RFC PATCH] sched/wait: Make interruptible exclusive waitqueue wakeups reliable Linus Torvalds
2019-12-09 12:08   ` Oleg Nesterov
2019-12-10  7:29     ` Ingo Molnar
2019-12-10 17:30       ` Oleg Nesterov
2019-12-09 17:38 ` Oleg Nesterov [this message]
2019-12-09 18:03   ` Fundamental race condition in wait_event_interruptible_exclusive() ? Linus Torvalds

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=20191209173820.GA11415@redhat.com \
    --to=oleg@redhat.com \
    --cc=balbi@kernel.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.guittot@linaro.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.