Skip to content

Commit

Permalink
locking/mutex: Rework mutex::owner
Browse files Browse the repository at this point in the history
The current mutex implementation has an atomic lock word and a
non-atomic owner field.

This disparity leads to a number of issues with the current mutex code
as it means that we can have a locked mutex without an explicit owner
(because the owner field has not been set, or already cleared).

This leads to a number of weird corner cases, esp. between the
optimistic spinning and debug code. Where the optimistic spinning
code needs the owner field updated inside the lock region, the debug
code is more relaxed because the whole lock is serialized by the
wait_lock.

Also, the spinning code itself has a few corner cases where we need to
deal with a held lock without an owner field.

Furthermore, it becomes even more of a problem when trying to fix
starvation cases in the current code. We end up stacking special case
on special case.

To solve this rework the basic mutex implementation to be a single
atomic word that contains the owner and uses the low bits for extra
state.

This matches how PI futexes and rt_mutex already work. By having the
owner an integral part of the lock state a lot of the problems
dissapear and we get a better option to deal with starvation cases,
direct owner handoff.

Changing the basic mutex does however invalidate all the arch specific
mutex code; this patch leaves that unused in-place, a later patch will
remove that.

Tested-by: Jason Low <jason.low2@hpe.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Will Deacon <will.deacon@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
  • Loading branch information
Peter Zijlstra authored and Ingo Molnar committed Oct 25, 2016
1 parent 3ab7c08 commit 3ca0ff5
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 305 deletions.
24 changes: 0 additions & 24 deletions include/linux/mutex-debug.h

This file was deleted.

46 changes: 30 additions & 16 deletions include/linux/mutex.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <linux/atomic.h>
#include <asm/processor.h>
#include <linux/osq_lock.h>
#include <linux/debug_locks.h>

/*
* Simple, straightforward mutexes with strict semantics:
Expand Down Expand Up @@ -48,16 +49,12 @@
* locks and tasks (and only those tasks)
*/
struct mutex {
/* 1: unlocked, 0: locked, negative: locked, possible waiters */
atomic_t count;
atomic_long_t owner;
spinlock_t wait_lock;
struct list_head wait_list;
#if defined(CONFIG_DEBUG_MUTEXES) || defined(CONFIG_MUTEX_SPIN_ON_OWNER)
struct task_struct *owner;
#endif
#ifdef CONFIG_MUTEX_SPIN_ON_OWNER
struct optimistic_spin_queue osq; /* Spinner MCS lock */
#endif
struct list_head wait_list;
#ifdef CONFIG_DEBUG_MUTEXES
void *magic;
#endif
Expand All @@ -66,6 +63,11 @@ struct mutex {
#endif
};

static inline struct task_struct *__mutex_owner(struct mutex *lock)
{
return (struct task_struct *)(atomic_long_read(&lock->owner) & ~0x03);
}

/*
* This is the control structure for tasks blocked on mutex,
* which resides on the blocked task's kernel stack:
Expand All @@ -79,9 +81,20 @@ struct mutex_waiter {
};

#ifdef CONFIG_DEBUG_MUTEXES
# include <linux/mutex-debug.h>

#define __DEBUG_MUTEX_INITIALIZER(lockname) \
, .magic = &lockname

extern void mutex_destroy(struct mutex *lock);

#else

# define __DEBUG_MUTEX_INITIALIZER(lockname)

static inline void mutex_destroy(struct mutex *lock) {}

#endif

/**
* mutex_init - initialize the mutex
* @mutex: the mutex to be initialized
Expand All @@ -90,14 +103,12 @@ struct mutex_waiter {
*
* It is not allowed to initialize an already locked mutex.
*/
# define mutex_init(mutex) \
do { \
static struct lock_class_key __key; \
\
__mutex_init((mutex), #mutex, &__key); \
#define mutex_init(mutex) \
do { \
static struct lock_class_key __key; \
\
__mutex_init((mutex), #mutex, &__key); \
} while (0)
static inline void mutex_destroy(struct mutex *lock) {}
#endif

#ifdef CONFIG_DEBUG_LOCK_ALLOC
# define __DEP_MAP_MUTEX_INITIALIZER(lockname) \
Expand All @@ -107,7 +118,7 @@ static inline void mutex_destroy(struct mutex *lock) {}
#endif

#define __MUTEX_INITIALIZER(lockname) \
{ .count = ATOMIC_INIT(1) \
{ .owner = ATOMIC_LONG_INIT(0) \
, .wait_lock = __SPIN_LOCK_UNLOCKED(lockname.wait_lock) \
, .wait_list = LIST_HEAD_INIT(lockname.wait_list) \
__DEBUG_MUTEX_INITIALIZER(lockname) \
Expand All @@ -127,7 +138,10 @@ extern void __mutex_init(struct mutex *lock, const char *name,
*/
static inline int mutex_is_locked(struct mutex *lock)
{
return atomic_read(&lock->count) != 1;
/*
* XXX think about spin_is_locked
*/
return __mutex_owner(lock) != NULL;
}

/*
Expand Down
13 changes: 0 additions & 13 deletions kernel/locking/mutex-debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,8 @@ void debug_mutex_unlock(struct mutex *lock)
{
if (likely(debug_locks)) {
DEBUG_LOCKS_WARN_ON(lock->magic != lock);

if (!lock->owner)
DEBUG_LOCKS_WARN_ON(!lock->owner);
else
DEBUG_LOCKS_WARN_ON(lock->owner != current);

DEBUG_LOCKS_WARN_ON(!lock->wait_list.prev && !lock->wait_list.next);
}

/*
* __mutex_slowpath_needs_to_unlock() is explicitly 0 for debug
* mutexes so that we can do it here after we've verified state.
*/
mutex_clear_owner(lock);
atomic_set(&lock->count, 1);
}

void debug_mutex_init(struct mutex *lock, const char *name,
Expand Down
10 changes: 0 additions & 10 deletions kernel/locking/mutex-debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ extern void debug_mutex_unlock(struct mutex *lock);
extern void debug_mutex_init(struct mutex *lock, const char *name,
struct lock_class_key *key);

static inline void mutex_set_owner(struct mutex *lock)
{
WRITE_ONCE(lock->owner, current);
}

static inline void mutex_clear_owner(struct mutex *lock)
{
WRITE_ONCE(lock->owner, NULL);
}

#define spin_lock_mutex(lock, flags) \
do { \
struct mutex *l = container_of(lock, struct mutex, wait_lock); \
Expand Down
Loading

0 comments on commit 3ca0ff5

Please sign in to comment.