Skip to content

Commit

Permalink
mutex: Do not unnecessarily deal with waiters
Browse files Browse the repository at this point in the history
Upon entering the slowpath, we immediately attempt to acquire
the lock by checking if it is already unlocked. If we are lucky
enough that this is the case, then we don't need to deal with
any waiter related logic.

Furthermore any checks for an empty wait_list are unnecessary as
we already know that count is non-negative and hence no one is
waiting for the lock.

Move the count check and xchg calls to be done before any
waiters are setup - including waiter debugging. Upon failure to
acquire the lock, the xchg sets the counter to 0, instead of -1
as it was originally. This can be done here since we set it back
to -1 right at the beginning of the loop so other waiters are
woken up when the lock is released.

When tested on a 8-socket (80 core) system against a vanilla
3.10-rc1 kernel, this patch provides some small performance
benefits (+2-6%). While it could be considered in the noise
level, the average percentages were stable across multiple runs
and no performance regressions were seen. Two big winners, for
small amounts of users (10-100), were the short and compute
workloads had a +19.36% and +%15.76% in jobs per minute.

Also change some break statements to 'goto slowpath', which IMO
makes a little more intuitive to read.

Signed-off-by: Davidlohr Bueso <davidlohr.bueso@hp.com>
Acked-by: Rik van Riel <riel@redhat.com>
Acked-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1372450398.2106.1.camel@buesod1.americas.hpqcorp.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
  • Loading branch information
Davidlohr Bueso authored and Ingo Molnar committed Jul 23, 2013
1 parent b59f2b4 commit ec83f42
Showing 1 changed file with 18 additions and 23 deletions.
41 changes: 18 additions & 23 deletions kernel/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
* performed the optimistic spinning cannot be done.
*/
if (ACCESS_ONCE(ww->ctx))
break;
goto slowpath;
}

/*
Expand All @@ -474,7 +474,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
owner = ACCESS_ONCE(lock->owner);
if (owner && !mutex_spin_on_owner(lock, owner)) {
mspin_unlock(MLOCK(lock), &node);
break;
goto slowpath;
}

if ((atomic_read(&lock->count) == 1) &&
Expand All @@ -489,8 +489,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,

mutex_set_owner(lock);
mspin_unlock(MLOCK(lock), &node);
preempt_enable();
return 0;
goto done;
}
mspin_unlock(MLOCK(lock), &node);

Expand All @@ -501,7 +500,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
* the owner complete.
*/
if (!owner && (need_resched() || rt_task(task)))
break;
goto slowpath;

/*
* The cpu_relax() call is a compiler barrier which forces
Expand All @@ -515,16 +514,17 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
#endif
spin_lock_mutex(&lock->wait_lock, flags);

/* once more, can we acquire the lock? */
if (MUTEX_SHOW_NO_WAITER(lock) && (atomic_xchg(&lock->count, 0) == 1))
goto skip_wait;

debug_mutex_lock_common(lock, &waiter);
debug_mutex_add_waiter(lock, &waiter, task_thread_info(task));

/* add waiting tasks to the end of the waitqueue (FIFO): */
list_add_tail(&waiter.list, &lock->wait_list);
waiter.task = task;

if (MUTEX_SHOW_NO_WAITER(lock) && (atomic_xchg(&lock->count, -1) == 1))
goto done;

lock_contended(&lock->dep_map, ip);

for (;;) {
Expand All @@ -538,7 +538,7 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
* other waiters:
*/
if (MUTEX_SHOW_NO_WAITER(lock) &&
(atomic_xchg(&lock->count, -1) == 1))
(atomic_xchg(&lock->count, -1) == 1))
break;

/*
Expand All @@ -563,24 +563,25 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
schedule_preempt_disabled();
spin_lock_mutex(&lock->wait_lock, flags);
}
mutex_remove_waiter(lock, &waiter, current_thread_info());
/* set it to 0 if there are no waiters left: */
if (likely(list_empty(&lock->wait_list)))
atomic_set(&lock->count, 0);
debug_mutex_free_waiter(&waiter);

done:
skip_wait:
/* got the lock - cleanup and rejoice! */
lock_acquired(&lock->dep_map, ip);
/* got the lock - rejoice! */
mutex_remove_waiter(lock, &waiter, current_thread_info());
mutex_set_owner(lock);

if (!__builtin_constant_p(ww_ctx == NULL)) {
struct ww_mutex *ww = container_of(lock,
struct ww_mutex,
base);
struct ww_mutex *ww = container_of(lock, struct ww_mutex, base);
struct mutex_waiter *cur;

/*
* This branch gets optimized out for the common case,
* and is only important for ww_mutex_lock.
*/

ww_mutex_lock_acquired(ww, ww_ctx);
ww->ctx = ww_ctx;

Expand All @@ -594,15 +595,9 @@ __mutex_lock_common(struct mutex *lock, long state, unsigned int subclass,
}
}

/* set it to 0 if there are no waiters left: */
if (likely(list_empty(&lock->wait_list)))
atomic_set(&lock->count, 0);

spin_unlock_mutex(&lock->wait_lock, flags);

debug_mutex_free_waiter(&waiter);
done:
preempt_enable();

return 0;

err:
Expand Down

0 comments on commit ec83f42

Please sign in to comment.