| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 1 Mar 2017 23:55:12 +0800
From: Fengguang Wu <fengguang.wu@...el.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Boqun Feng <boqun.feng@...il.com>,
Nicolai Hähnle <Nicolai.Haehnle@....com>,
Chris Wilson <chris@...is-wilson.co.uk>,
Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
LKP <lkp@...org>
Subject: Re: [locking/ww_mutex] 2a0c112828 WARNING: CPU: 0 PID: 18 at
kernel/locking/mutex.c:305 __ww_mutex_wakeup_for_backoff
On Wed, Mar 01, 2017 at 04:51:36PM +0100, Peter Zijlstra wrote:
>On Wed, Mar 01, 2017 at 11:40:43PM +0800, Fengguang Wu wrote:
>> Thanks for the patch! I applied the patch on top of "locking/ww_mutex:
>> Add kselftests for ww_mutex stress", and find no "bad unlock balance
>> detected" but this warning. Attached is the new dmesg which is a bit
>> large due to lots of repeated errors.
>>
>> [ 9.105427] Freeing initrd memory: 24852K
>> [ 9.121306] The force parameter has not been set to 1. The Iris poweroff handler will not be installed.
>> [ 9.141216] NatSemi SCx200 Driver
>> [ 9.724519] ------------[ cut here ]------------
>> [ 9.726795] WARNING: CPU: 0 PID: 22 at kernel/locking/mutex.c:305 __ww_mutex_wakeup_for_backoff+0x31/0x7a
>> [ 9.738281] CPU: 0 PID: 22 Comm: kworker/u2:1 Not tainted 4.10.0-rc3-00156-g7d81c50 #1
>> [ 9.741977] Workqueue: test-ww_mutex test_cycle_work
>> [ 9.745524] Call Trace:
>> [ 9.747610] dump_stack+0x16/0x18
>> [ 9.754619] __warn+0xa0/0xb7
>> [ 9.757553] ? __ww_mutex_wakeup_for_backoff+0x31/0x7a
>> [ 9.760881] warn_slowpath_null+0x11/0x16
>> [ 9.765222] __ww_mutex_wakeup_for_backoff+0x31/0x7a
>> [ 9.768028] __ww_mutex_lock+0x2f3/0xb63
>> [ 9.770979] ? wake_up_q+0x25/0x40
>> [ 9.773044] ? __might_sleep+0x6c/0x73
>> [ 9.774890] ww_mutex_lock+0x34/0x3b
>> [ 9.776001] ? test_cycle_work+0xf7/0x170
>> [ 9.777751] test_cycle_work+0xf7/0x170
>> [ 9.779036] process_one_work+0x1c0/0x33a
>> [ 9.780664] ? process_one_work+0x168/0x33a
>> [ 9.788924] worker_thread+0x22f/0x315
>> [ 9.791016] kthread+0xed/0xf2
>> [ 9.793255] ? process_scheduled_works+0x24/0x24
>> [ 9.795475] ? __kthread_create_on_node+0x11f/0x11f
>> [ 9.798741] ? __kthread_create_on_node+0x11f/0x11f
>> [ 9.802371] ret_from_fork+0x19/0x30
>> [ 9.804430] ---[ end trace 9036bbb174aed804 ]---
>
>Do you have the below patch in?
Nope. I'll re-test with it added.
Regards,
Fengguang
>---
>commit b9c16a0e1f733c97e48798b2a9362c485bb3b731
>Author: Peter Zijlstra <peterz@...radead.org>
>Date: Tue Jan 17 16:06:09 2017 +0100
>
> locking/mutex: Fix lockdep_assert_held() fail
>
> 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@....de>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> Cc: Nicolai Haehnle <Nicolai.Haehnle@....com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Fixes: 659cf9f5824a ("locking/ww_mutex: Optimize ww-mutexes by waking at most one waiter for backoff when acquiring the lock")
> Link: http://lkml.kernel.org/r/20170117150609.GB32474@worktop
> Signed-off-by: Ingo Molnar <mingo@...nel.org>
>
>diff --git a/kernel/locking/mutex-debug.h b/kernel/locking/mutex-debug.h
>index a459faa48987..4174417d5309 100644
>--- a/kernel/locking/mutex-debug.h
>+++ b/kernel/locking/mutex-debug.h
>@@ -26,20 +26,3 @@ extern void mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
> 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)
>diff --git a/kernel/locking/mutex.c b/kernel/locking/mutex.c
>index 935116723a3d..705e06fe5e6c 100644
>--- a/kernel/locking/mutex.c
>+++ b/kernel/locking/mutex.c
>@@ -325,8 +325,6 @@ __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
> 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_mutex *lock, struct ww_acquire_ctx *ctx)
> * 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, long state, unsigned int subclass,
> 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, long state, unsigned int subclass,
> 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, long state, unsigned int subclass,
> 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, long state, unsigned int subclass,
> (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, long state, unsigned int subclass,
> 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, long state, unsigned int subclass,
> __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();
>@@ -999,8 +996,8 @@ EXPORT_SYMBOL_GPL(ww_mutex_lock_interruptible);
> 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);
>
>@@ -1035,7 +1032,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
> 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: */
>@@ -1052,7 +1049,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
> 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);
> }
>diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h
>index 4410a4af42a3..6ebc1902f779 100644
>--- 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)
>
Powered by blists - more mailing lists