Skip to content
This repository has been archived by the owner on Sep 24, 2020. It is now read-only.

Commit

Permalink
locking/mutex: Fix lockdep_assert_held() fail
Browse files Browse the repository at this point in the history
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 <efault@gmx.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Nicolai Haehnle <Nicolai.Haehnle@amd.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
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 <mingo@kernel.org>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Jan 30, 2017
1 parent 4009f4b commit b9c16a0
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 35 deletions.
17 changes: 0 additions & 17 deletions kernel/locking/mutex-debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
25 changes: 11 additions & 14 deletions kernel/locking/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

/*
Expand Down Expand Up @@ -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;
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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();

/*
Expand All @@ -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);

Expand All @@ -872,15 +869,15 @@ __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;

err:
__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();
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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: */
Expand All @@ -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);
}
Expand Down
4 changes: 0 additions & 4 deletions kernel/locking/mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit b9c16a0

Please sign in to comment.