All of lore.kernel.org
 help / color / mirror / Atom feed
From: Omar Sandoval <osandov@osandov.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Jens Axboe <axboe@fb.com>
Subject: add_wait_queue() (unintentional?) behavior change in v4.13
Date: Wed, 29 Nov 2017 16:58:28 -0800	[thread overview]
Message-ID: <20171130005828.GA15628@vader> (raw)

Hi, Ingo,

Commit 50816c48997a ("sched/wait: Standardize internal naming of
wait-queue entries") changed the behavior of add_wait_queue() from
inserting the wait entry at the head of the wait queue to the tail of
the wait queue. This is the relevant hunk:

-void add_wait_queue(wait_queue_head_t *q, wait_queue_entry_t *wait)
+void add_wait_queue(wait_queue_head_t *q, struct wait_queue_entry *wq_entry)
 {
        unsigned long flags;

-       wait->flags &= ~WQ_FLAG_EXCLUSIVE;
+       wq_entry->flags &= ~WQ_FLAG_EXCLUSIVE;
        spin_lock_irqsave(&q->lock, flags);
-       __add_wait_queue(q, wait);
+       __add_wait_queue_entry_tail(q, wq_entry);
        spin_unlock_irqrestore(&q->lock, flags);
 }
 EXPORT_SYMBOL(add_wait_queue);

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:

diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 98feab7933c7..929ecb7d6b78 100644
--- a/kernel/sched/wait.c
+++ b/kernel/sched/wait.c
@@ -27,7 +27,7 @@ void add_wait_queue(struct wait_queue_head *wq_head, struct wait_queue_entry *wq
 
 	wq_entry->flags &= ~WQ_FLAG_EXCLUSIVE;
 	spin_lock_irqsave(&wq_head->lock, flags);
-	__add_wait_queue_entry_tail(wq_head, wq_entry);
+	__add_wait_queue(wq_head, wq_entry);
 	spin_unlock_irqrestore(&wq_head->lock, flags);
 }
 EXPORT_SYMBOL(add_wait_queue);

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.

             reply	other threads:[~2017-11-30  0:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30  0:58 Omar Sandoval [this message]
2017-11-30  2:37 ` add_wait_queue() (unintentional?) behavior change in v4.13 Linus Torvalds
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=20171130005828.GA15628@vader \
    --to=osandov@osandov.com \
    --cc=axboe@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=torvalds@linux-foundation.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.