[locking/ww_mutex] 659cf9f582: WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:305 __ww_mutex_wakeup_for_backoff
10:20 p.m.
Greetings,
0day kernel testing robot got the below dmesg and the first bad commit is
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
commit 659cf9f5824a12e6b50785e4e9cf1adf8a3adbd0
Author: Nicolai Hähnle <Nicolai.Haehnle(a)amd.com>
AuthorDate: Wed Dec 21 19:46:36 2016 +0100
Commit: Ingo Molnar <mingo(a)kernel.org>
CommitDate: Sat Jan 14 11:14:44 2017 +0100
locking/ww_mutex: Optimize ww-mutexes by waking at most one waiter for backoff when
acquiring the lock
The wait list is sorted by stamp order, and the only waiting task that may
have to back off is the first waiter with a context.
The regular slow path does not have to wake any other tasks at all, since
all other waiters that would have to back off were either woken up when
the waiter was added to the list, or detected the condition before they
added themselves.
Median timings taken of a contention-heavy GPU workload:
Without this series:
real 0m59.900s
user 0m7.516s
sys 2m16.076s
With changes up to and including this patch:
real 0m52.946s
user 0m7.272s
sys 1m55.964s
Signed-off-by: Nicolai Hähnle <Nicolai.Haehnle(a)amd.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz(a)infradead.org>
Cc: Andrew Morton <akpm(a)linux-foundation.org>
Cc: Chris Wilson <chris(a)chris-wilson.co.uk>
Cc: Daniel Vetter <daniel(a)ffwll.ch>
Cc: Linus Torvalds <torvalds(a)linux-foundation.org>
Cc: Maarten Lankhorst <dev(a)mblankhorst.nl>
Cc: Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz(a)infradead.org>
Cc: Thomas Gleixner <tglx(a)linutronix.de>
Cc: dri-devel(a)lists.freedesktop.org
Link: http://lkml.kernel.org/r/1482346000-9927-9-git-send-email-nhaehnle@gmail.com
Signed-off-by: Ingo Molnar <mingo(a)kernel.org>
+------------------------------------------------------------------+------------+------------+------------+
| | 200b187440 |
659cf9f582 | 40d4867672 |
+------------------------------------------------------------------+------------+------------+------------+
| boot_successes | 87 | 0
| 0 |
| boot_failures | 1 | 22
| 13 |
| BUG:kernel_reboot-without-warning_in_test_stage | 1 |
| |
| WARNING:at_kernel/locking/mutex.c:#__ww_mutex_wakeup_for_backoff | 0 | 22
| 13 |
| WARNING:at_include/linux/pci.h:#vp_synchronize_vectors | 0 | 0
| 13 |
+------------------------------------------------------------------+------------+------------+------------+
[ 0.000000] locking mismatches: ok | ok | ok |
[ 0.000000] EDEADLK handling:
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:305
__ww_mutex_wakeup_for_backoff+0x62/0xf4
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc3-00143-g659cf9f #1
[ 0.000000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.9.3-20161025_171302-gandalf 04/01/2014
[ 0.000000] Call Trace:
[ 0.000000] dump_stack+0x19/0x27
[ 0.000000] __warn+0xbf/0xe1
[ 0.000000] warn_slowpath_null+0x1d/0x2a
git bisect start 40d48676729952466298b37798c5cd2ff634d541
7a308bb3016f57e5be11a677d15b821536419d36 --
git bisect bad 1300c8e774bff908e6b16ce6eca5cc1ac60039dc # 07:21 0- 4 Merge
'mediatek/travis-test' into devel-spot-201701260520
git bisect bad dbf2f144ccf6813b1ac651a985fdc5e1c9eeace2 # 07:33 0- 18 Merge
'scottwood/next' into devel-spot-201701260520
git bisect bad eabce5f693cbb5b6a393a562f7414c6831154903 # 07:42 0- 10 Merge
'linux-review/Rafa-Mi-ecki/net-next-Broadcom-PHY-driver-cleanup/20170126-040712'
into devel-spot-201701260520
git bisect good c343523d3409fb840bcd0fa641d3c22081902e1a # 07:54 22+ 0 Merge
'luto/vfs/suid' into devel-spot-201701260520
git bisect good 8baced835e021ee8739a73d397df4164090d1982 # 08:10 22+ 0 Merge
'linux-review/Chris-Brandt/mmc-sh_mobile_sdhi-fix-missing-r7s72100-clocks/20170126-043717'
into devel-spot-201701260520
git bisect bad e546c6ba62a562ca0053ad89ffa57c546ae54c33 # 08:23 0- 13 Merge
'linux-review/Ricardo-Neri/x86-Enable-User-Mode-Instruction-Prevention/20170126-043345'
into devel-spot-201701260520
git bisect good 18e97e7ab2898629407d61beeedb3cbe1e10c511 # 08:44 20+ 0 Merge
'linux-review/Jiri-Kosina/netfilter-nf_ct_helper-warn-when-not-applying-default-helper-assignment/20170126-044835'
into devel-spot-201701260520
git bisect good 200b1874401555e9b36010aa318e75cf57005f36 # 08:59 21+ 1
locking/ww_mutex: Notify waiters that have to back off while adding tasks to wait list
git bisect bad 0874170baf55007bc8baa3fdf3c6c5f1a2188f9b # 09:20 0- 1 Merge
branch 'x86/platform'
git bisect bad 297d66bfa7e6a04115847181601912c7de14a0cc # 09:33 0- 1 Merge
branch 'perf/core'
git bisect bad 736d67c8db47d2f780f5ed9b2233a56bea865f8c # 09:50 0- 12 Merge
branch 'locking/core'
git bisect bad 427b18207a87f6306bd53a74e03dbe17392b0045 # 10:10 0- 14
locking/mutex: Improve inlining
git bisect bad 659cf9f5824a12e6b50785e4e9cf1adf8a3adbd0 # 10:29 0- 22
locking/ww_mutex: Optimize ww-mutexes by waking at most one waiter for backoff when
acquiring the lock
# first bad commit: [659cf9f5824a12e6b50785e4e9cf1adf8a3adbd0] locking/ww_mutex: Optimize
ww-mutexes by waking at most one waiter for backoff when acquiring the lock
git bisect good 200b1874401555e9b36010aa318e75cf57005f36 # 10:35 66+ 1
locking/ww_mutex: Notify waiters that have to back off while adding tasks to wait list
# extra tests with CONFIG_DEBUG_INFO_REDUCED
# extra tests on HEAD of linux-devel/devel-spot-201701260520
git bisect bad 40d48676729952466298b37798c5cd2ff634d541 # 10:42 0- 13 0day
head guard for 'devel-spot-201701260520'
# extra tests on tree/branch linux-next/master
git bisect bad f37208bc3c9c2f811460ef264909dfbc7f605a60 # 10:54 0- 8 Add
linux-next specific files for 20170125
# extra tests with first bad commit reverted
# extra tests on tree/branch linus/master
git bisect good 49e555a932de57611eb27edf2d1ad03d9a267bdd # 11:20 66+ 0 Merge
tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost
# extra tests on tree/branch linux-next/master
git bisect bad f37208bc3c9c2f811460ef264909dfbc7f605a60 # 11:20 0- 66 Add
linux-next specific files for 20170125
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/lkp Intel Corporation
5:19 a.m.
On Thu, Jan 26, 2017 at 11:20:55AM +0800, kernel test robot wrote:
Greetings,
0day kernel testing robot got the below dmesg and the first bad commit is
[ 0.000000] locking mismatches: ok | ok | ok |
[ 0.000000] EDEADLK handling:
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:305
__ww_mutex_wakeup_for_backoff+0x62/0xf4
[ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc3-00143-g659cf9f #1
[ 0.000000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.9.3-20161025_171302-gandalf 04/01/2014
[ 0.000000] Call Trace:
[ 0.000000] dump_stack+0x19/0x27
[ 0.000000] __warn+0xbf/0xe1
[ 0.000000] warn_slowpath_null+0x1d/0x2a
(Just to remind you, truncated WARNs are very very annoying, this one
for example doesn't show anything useful in the call trace).
I have the following patch to address this.
---
Subject: locking/mutex: Fix lockdep_assert_held() fail
From: Peter Zijlstra <peterz(a)infradead.org>
Date: Tue, 17 Jan 2017 16:06:09 +0100
In commit 659cf9f5824a ("locking/ww_mutex: Optimize ww-mutexes by
waking at most one waiter for backoff when acquiring the lock") I
replaced a comment with a lockdep_assert_held(). However it turns out
we hide that lock from lockdep for hysterical raisins, which results
in the assertion always firing.
Remove the old debug code as lockdep will easily spot the abuse it was
meant to catch, which will make the lock visible to lockdep and make
the assertion work as intended.
Reported-by: Mike Galbraith <efault(a)gmx.de>
Cc: Ingo Molnar <mingo(a)elte.hu>
Cc: Thomas Gleixner <tglx(a)linutronix.de>
Cc: Nicolai Haehnle <Nicolai.Haehnle(a)amd.com>
Fixes: 659cf9f5824a ("locking/ww_mutex: Optimize ww-mutexes by waking at most one
waiter for backoff when acquiring the lock")
Signed-off-by: Peter Zijlstra (Intel) <peterz(a)infradead.org>
Link: http://lkml.kernel.org/r/20170117150609.GB32474@worktop
---
kernel/locking/mutex-debug.h | 17 -----------------
kernel/locking/mutex.c | 25 +++++++++++--------------
kernel/locking/mutex.h | 4 ----
3 files changed, 11 insertions(+), 35 deletions(-)
--- a/kernel/locking/mutex-debug.h
+++ b/kernel/locking/mutex-debug.h
@@ -26,20 +26,3 @@ extern void mutex_remove_waiter(struct m
extern void debug_mutex_unlock(struct mutex *lock);
extern void debug_mutex_init(struct mutex *lock, const char *name,
struct lock_class_key *key);
-
-#define spin_lock_mutex(lock, flags) \
- do { \
- struct mutex *l = container_of(lock, struct mutex, wait_lock); \
- \
- DEBUG_LOCKS_WARN_ON(in_interrupt()); \
- local_irq_save(flags); \
- arch_spin_lock(&(lock)->rlock.raw_lock);\
- DEBUG_LOCKS_WARN_ON(l->magic != l); \
- } while (0)
-
-#define spin_unlock_mutex(lock, flags) \
- do { \
- arch_spin_unlock(&(lock)->rlock.raw_lock); \
- local_irq_restore(flags); \
- preempt_check_resched(); \
- } while (0)
--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -325,8 +325,6 @@ __ww_mutex_wakeup_for_backoff(struct mut
static __always_inline void
ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
{
- unsigned long flags;
-
ww_mutex_lock_acquired(lock, ctx);
lock->ctx = ctx;
@@ -350,9 +348,9 @@ ww_mutex_set_context_fastpath(struct ww_
* Uh oh, we raced in fastpath, wake up everyone in this case,
* so they can see the new lock->ctx.
*/
- spin_lock_mutex(&lock->base.wait_lock, flags);
+ spin_lock(&lock->base.wait_lock);
__ww_mutex_wakeup_for_backoff(&lock->base, ctx);
- spin_unlock_mutex(&lock->base.wait_lock, flags);
+ spin_unlock(&lock->base.wait_lock);
}
/*
@@ -740,7 +738,6 @@ __mutex_lock_common(struct mutex *lock,
struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
{
struct mutex_waiter waiter;
- unsigned long flags;
bool first = false;
struct ww_mutex *ww;
int ret;
@@ -766,7 +763,7 @@ __mutex_lock_common(struct mutex *lock,
return 0;
}
- spin_lock_mutex(&lock->wait_lock, flags);
+ spin_lock(&lock->wait_lock);
/*
* After waiting to acquire the wait_lock, try again.
*/
@@ -830,7 +827,7 @@ __mutex_lock_common(struct mutex *lock,
goto err;
}
- spin_unlock_mutex(&lock->wait_lock, flags);
+ spin_unlock(&lock->wait_lock);
schedule_preempt_disabled();
/*
@@ -853,9 +850,9 @@ __mutex_lock_common(struct mutex *lock,
(first && mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, &waiter)))
break;
- spin_lock_mutex(&lock->wait_lock, flags);
+ spin_lock(&lock->wait_lock);
}
- spin_lock_mutex(&lock->wait_lock, flags);
+ spin_lock(&lock->wait_lock);
acquired:
__set_current_state(TASK_RUNNING);
@@ -872,7 +869,7 @@ __mutex_lock_common(struct mutex *lock,
if (use_ww_ctx && ww_ctx)
ww_mutex_set_context_slowpath(ww, ww_ctx);
- spin_unlock_mutex(&lock->wait_lock, flags);
+ spin_unlock(&lock->wait_lock);
preempt_enable();
return 0;
@@ -880,7 +877,7 @@ __mutex_lock_common(struct mutex *lock,
__set_current_state(TASK_RUNNING);
mutex_remove_waiter(lock, &waiter, current);
err_early_backoff:
- spin_unlock_mutex(&lock->wait_lock, flags);
+ spin_unlock(&lock->wait_lock);
debug_mutex_free_waiter(&waiter);
mutex_release(&lock->dep_map, 1, ip);
preempt_enable();
@@ -1013,8 +1010,8 @@ EXPORT_SYMBOL_GPL(ww_mutex_lock_interrup
static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigned long
ip)
{
struct task_struct *next = NULL;
- unsigned long owner, flags;
DEFINE_WAKE_Q(wake_q);
+ unsigned long owner;
mutex_release(&lock->dep_map, 1, ip);
@@ -1049,7 +1046,7 @@ static noinline void __sched __mutex_unl
owner = old;
}
- spin_lock_mutex(&lock->wait_lock, flags);
+ spin_lock(&lock->wait_lock);
debug_mutex_unlock(lock);
if (!list_empty(&lock->wait_list)) {
/* get the first entry from the wait-list: */
@@ -1066,7 +1063,7 @@ static noinline void __sched __mutex_unl
if (owner & MUTEX_FLAG_HANDOFF)
__mutex_handoff(lock, next);
- spin_unlock_mutex(&lock->wait_lock, flags);
+ spin_unlock(&lock->wait_lock);
wake_up_q(&wake_q);
}
--- a/kernel/locking/mutex.h
+++ b/kernel/locking/mutex.h
@@ -9,10 +9,6 @@
* !CONFIG_DEBUG_MUTEXES case. Most of them are NOPs:
*/
-#define spin_lock_mutex(lock, flags) \
- do { spin_lock(lock); (void)(flags); } while (0)
-#define spin_unlock_mutex(lock, flags) \
- do { spin_unlock(lock); (void)(flags); } while (0)
#define mutex_remove_waiter(lock, waiter, task) \
__list_del((waiter)->list.prev, (waiter)->list.next)
11:47 p.m.
Hi Peter,
On Thu, Jan 26, 2017 at 11:19:37AM +0100, Peter Zijlstra wrote:
On Thu, Jan 26, 2017 at 11:20:55AM +0800, kernel test robot wrote:
> Greetings,
>
> 0day kernel testing robot got the below dmesg and the first bad commit is
>
> [ 0.000000] locking mismatches: ok | ok | ok |
> [ 0.000000] EDEADLK handling:
> [ 0.000000] ------------[ cut here ]------------
> [ 0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/mutex.c:305
__ww_mutex_wakeup_for_backoff+0x62/0xf4
> [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc3-00143-g659cf9f #1
> [ 0.000000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.9.3-20161025_171302-gandalf 04/01/2014
> [ 0.000000] Call Trace:
> [ 0.000000] dump_stack+0x19/0x27
> [ 0.000000] __warn+0xbf/0xe1
> [ 0.000000] warn_slowpath_null+0x1d/0x2a
>
(Just to remind you, truncated WARNs are very very annoying, this one
for example doesn't show anything useful in the call trace).
I just fixed the incomplete dmesg, thank you for pointing it out!
Regards,
Fengguang
1471
days inactive
1493
days old
2 comments
3 participants
participants (3)
-
Fengguang Wu -
kernel test robot
-
Peter Zijlstra