Skip to content

Commit b9c16a0

Browse files
Peter ZijlstraIngo Molnar
authored andcommitted
locking/mutex: Fix lockdep_assert_held() fail
In commit: 659cf9f ("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 <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: Nicolai Haehnle <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Fixes: 659cf9f ("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 <[email protected]>
1 parent 4009f4b commit b9c16a0

File tree

3 files changed

+11
-35
lines changed

3 files changed

+11
-35
lines changed

kernel/locking/mutex-debug.h

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,3 @@ extern void mutex_remove_waiter(struct mutex *lock, struct mutex_waiter *waiter,
2626
extern void debug_mutex_unlock(struct mutex *lock);
2727
extern void debug_mutex_init(struct mutex *lock, const char *name,
2828
struct lock_class_key *key);
29-
30-
#define spin_lock_mutex(lock, flags) \
31-
do { \
32-
struct mutex *l = container_of(lock, struct mutex, wait_lock); \
33-
\
34-
DEBUG_LOCKS_WARN_ON(in_interrupt()); \
35-
local_irq_save(flags); \
36-
arch_spin_lock(&(lock)->rlock.raw_lock);\
37-
DEBUG_LOCKS_WARN_ON(l->magic != l); \
38-
} while (0)
39-
40-
#define spin_unlock_mutex(lock, flags) \
41-
do { \
42-
arch_spin_unlock(&(lock)->rlock.raw_lock); \
43-
local_irq_restore(flags); \
44-
preempt_check_resched(); \
45-
} while (0)

kernel/locking/mutex.c

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,6 @@ __ww_mutex_wakeup_for_backoff(struct mutex *lock, struct ww_acquire_ctx *ww_ctx)
325325
static __always_inline void
326326
ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
327327
{
328-
unsigned long flags;
329-
330328
ww_mutex_lock_acquired(lock, ctx);
331329

332330
lock->ctx = ctx;
@@ -350,9 +348,9 @@ ww_mutex_set_context_fastpath(struct ww_mutex *lock, struct ww_acquire_ctx *ctx)
350348
* Uh oh, we raced in fastpath, wake up everyone in this case,
351349
* so they can see the new lock->ctx.
352350
*/
353-
spin_lock_mutex(&lock->base.wait_lock, flags);
351+
spin_lock(&lock->base.wait_lock);
354352
__ww_mutex_wakeup_for_backoff(&lock->base, ctx);
355-
spin_unlock_mutex(&lock->base.wait_lock, flags);
353+
spin_unlock(&lock->base.wait_lock);
356354
}
357355

358356
/*
@@ -740,7 +738,6 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
740738
struct ww_acquire_ctx *ww_ctx, const bool use_ww_ctx)
741739
{
742740
struct mutex_waiter waiter;
743-
unsigned long flags;
744741
bool first = false;
745742
struct ww_mutex *ww;
746743
int ret;
@@ -766,7 +763,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
766763
return 0;
767764
}
768765

769-
spin_lock_mutex(&lock->wait_lock, flags);
766+
spin_lock(&lock->wait_lock);
770767
/*
771768
* After waiting to acquire the wait_lock, try again.
772769
*/
@@ -830,7 +827,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
830827
goto err;
831828
}
832829

833-
spin_unlock_mutex(&lock->wait_lock, flags);
830+
spin_unlock(&lock->wait_lock);
834831
schedule_preempt_disabled();
835832

836833
/*
@@ -853,9 +850,9 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
853850
(first && mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx, &waiter)))
854851
break;
855852

856-
spin_lock_mutex(&lock->wait_lock, flags);
853+
spin_lock(&lock->wait_lock);
857854
}
858-
spin_lock_mutex(&lock->wait_lock, flags);
855+
spin_lock(&lock->wait_lock);
859856
acquired:
860857
__set_current_state(TASK_RUNNING);
861858

@@ -872,15 +869,15 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
872869
if (use_ww_ctx && ww_ctx)
873870
ww_mutex_set_context_slowpath(ww, ww_ctx);
874871

875-
spin_unlock_mutex(&lock->wait_lock, flags);
872+
spin_unlock(&lock->wait_lock);
876873
preempt_enable();
877874
return 0;
878875

879876
err:
880877
__set_current_state(TASK_RUNNING);
881878
mutex_remove_waiter(lock, &waiter, current);
882879
err_early_backoff:
883-
spin_unlock_mutex(&lock->wait_lock, flags);
880+
spin_unlock(&lock->wait_lock);
884881
debug_mutex_free_waiter(&waiter);
885882
mutex_release(&lock->dep_map, 1, ip);
886883
preempt_enable();
@@ -999,8 +996,8 @@ EXPORT_SYMBOL_GPL(ww_mutex_lock_interruptible);
999996
static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigned long ip)
1000997
{
1001998
struct task_struct *next = NULL;
1002-
unsigned long owner, flags;
1003999
DEFINE_WAKE_Q(wake_q);
1000+
unsigned long owner;
10041001

10051002
mutex_release(&lock->dep_map, 1, ip);
10061003

@@ -1035,7 +1032,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
10351032
owner = old;
10361033
}
10371034

1038-
spin_lock_mutex(&lock->wait_lock, flags);
1035+
spin_lock(&lock->wait_lock);
10391036
debug_mutex_unlock(lock);
10401037
if (!list_empty(&lock->wait_list)) {
10411038
/* get the first entry from the wait-list: */
@@ -1052,7 +1049,7 @@ static noinline void __sched __mutex_unlock_slowpath(struct mutex *lock, unsigne
10521049
if (owner & MUTEX_FLAG_HANDOFF)
10531050
__mutex_handoff(lock, next);
10541051

1055-
spin_unlock_mutex(&lock->wait_lock, flags);
1052+
spin_unlock(&lock->wait_lock);
10561053

10571054
wake_up_q(&wake_q);
10581055
}

kernel/locking/mutex.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,6 @@
99
* !CONFIG_DEBUG_MUTEXES case. Most of them are NOPs:
1010
*/
1111

12-
#define spin_lock_mutex(lock, flags) \
13-
do { spin_lock(lock); (void)(flags); } while (0)
14-
#define spin_unlock_mutex(lock, flags) \
15-
do { spin_unlock(lock); (void)(flags); } while (0)
1612
#define mutex_remove_waiter(lock, waiter, task) \
1713
__list_del((waiter)->list.prev, (waiter)->list.next)
1814

0 commit comments

Comments
 (0)