All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Omar Sandoval <osandov@osandov.com>
Cc: Ingo Molnar <mingo@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Jens Axboe <axboe@fb.com>
Subject: Re: add_wait_queue() (unintentional?) behavior change in v4.13
Date: Wed, 29 Nov 2017 18:37:40 -0800	[thread overview]
Message-ID: <CA+55aFwhq-ZyDW2SYhYDg1ytKybwHywiH9jW3UmMu=YAgk694Q@mail.gmail.com> (raw)
In-Reply-To: <20171130005828.GA15628@vader>

On Wed, Nov 29, 2017 at 4:58 PM, Omar Sandoval <osandov@osandov.com> wrote:
>
> Note the change from __add_wait_queue() to
> __add_wait_queue_entry_tail(). I'm assuming this was a typo since the
> commit message doesn't mention any functional changes. This patch
> restores the old behavior:
>   [...]
> I didn't go through and audit callers of add_wait_queue(), but from a
> quick code read this makes it so that non-exclusive waiters will not be
> woken up if they are behind enough exclusive waiters, and I bet that'll
> cause some bugs.

This sounds right to me.

Ingo?

The "add to head of wait-queue" is nasty and causes unfair waiter
behavior, but it does have that exclusive waiter reason going for it.

In the page bit-wait queues, we actually did this change
_intentionally_ a few months ago (see commits

  3510ca20ece0 Minor page waitqueue cleanups
  9c3a815f471a page waitqueue: always add new entries at the end

but there it was intentional: an exclusive waiter on the bit
wait-queues is going to acquire the bit lock, which in turn means that
they'll eventually release the bit lock and then wake up any
subsequent non-exclusive waiters, so the non-exclusive ones _will_ get
woken up eventually (and in a fair order).

Sadly, when it comes to wait-queues in general, we don't have those
kinds of guarantees. An exclusive waiter is going to use the resource,
but there's no fundamental reason to believe that non-exclusive
waiters will be woken up again (although in practice it's probably
very rare that they wouldn't).

                Linus

  reply	other threads:[~2017-11-30  2:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30  0:58 add_wait_queue() (unintentional?) behavior change in v4.13 Omar Sandoval
2017-11-30  2:37 ` Linus Torvalds [this message]
2017-12-06  7:15   ` [PATCH] sched/wait: fix add_wait_queue() behavior change Omar Sandoval
2017-12-06 16:11     ` Jens Axboe
2017-12-06 16:46       ` Ingo Molnar
2017-12-06 20:28     ` [tip:sched/core] sched/wait: Fix add_wait_queue() behavioral change tip-bot for Omar Sandoval
2017-12-06 16:43   ` add_wait_queue() (unintentional?) behavior change in v4.13 Ingo Molnar
2017-12-06  2:35 ` Fubo Chen

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='CA+55aFwhq-ZyDW2SYhYDg1ytKybwHywiH9jW3UmMu=YAgk694Q@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=axboe@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=osandov@osandov.com \
    /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.