From: Omar Sandoval <[email protected]>
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. That commit was a cleanup and didn't mention any
functional changes so it was likely unintentional. This change in
behavior theoretically breaks wait queues which mix exclusive and
non-exclusive waiters, as non-exclusive waiters will not be woken up if
they are queued behind enough exclusive waiters.
Signed-off-by: Omar Sandoval <[email protected]>
---
Since Ingo hasn't replied, here's a proper patch.
kernel/sched/wait.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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);
--
2.15.1
On 12/06/2017 12:15 AM, Omar Sandoval wrote:
> From: Omar Sandoval <[email protected]>
>
> 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. That commit was a cleanup and didn't mention any
> functional changes so it was likely unintentional. This change in
> behavior theoretically breaks wait queues which mix exclusive and
> non-exclusive waiters, as non-exclusive waiters will not be woken up if
> they are queued behind enough exclusive waiters.
Ingo? You've been silent on this issue, which is somewhat odd since
your cosmetic renaming patch introduced this behavioral change.
Omar, should probably turn that into a proper Fixes: line as
well.
> Signed-off-by: Omar Sandoval <[email protected]>
Reviewed-by: Jens Axboe <[email protected]>
--
Jens Axboe
* Jens Axboe <[email protected]> wrote:
> On 12/06/2017 12:15 AM, Omar Sandoval wrote:
> > From: Omar Sandoval <[email protected]>
> >
> > 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. That commit was a cleanup and didn't mention any
> > functional changes so it was likely unintentional. This change in
> > behavior theoretically breaks wait queues which mix exclusive and
> > non-exclusive waiters, as non-exclusive waiters will not be woken up if
> > they are queued behind enough exclusive waiters.
>
> Ingo? You've been silent on this issue, which is somewhat odd since
> your cosmetic renaming patch introduced this behavioral change.
>
> Omar, should probably turn that into a proper Fixes: line as
> well.
>
> > Signed-off-by: Omar Sandoval <[email protected]>
>
> Reviewed-by: Jens Axboe <[email protected]>
Thanks!
I've applied the fix and will send it to Linus later today.
Thanks,
Ingo
Commit-ID: c6b9d9a33029014446bd9ed84c1688f6d3d4eab9
Gitweb: https://git.kernel.org/tip/c6b9d9a33029014446bd9ed84c1688f6d3d4eab9
Author: Omar Sandoval <[email protected]>
AuthorDate: Tue, 5 Dec 2017 23:15:31 -0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 6 Dec 2017 19:30:34 +0100
sched/wait: Fix add_wait_queue() behavioral change
The following cleanup commit:
50816c48997a ("sched/wait: Standardize internal naming of wait-queue entries")
... unintentionally 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.
Beyond a negative performance impact this change in behavior
theoretically also breaks wait queues which mix exclusive and
non-exclusive waiters, as non-exclusive waiters will not be
woken up if they are queued behind enough exclusive waiters.
Signed-off-by: Omar Sandoval <[email protected]>
Reviewed-by: Jens Axboe <[email protected]>
Acked-by: Peter Zijlstra <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: [email protected]
Fixes: ("sched/wait: Standardize internal naming of wait-queue entries")
Link: http://lkml.kernel.org/r/a16c8ccffd39bd08fdaa45a5192294c784b803a7.1512544324.git.osandov@fb.com
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/sched/wait.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/wait.c b/kernel/sched/wait.c
index 98feab7..929ecb7 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);