From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757290AbcJGPZm (ORCPT ); Fri, 7 Oct 2016 11:25:42 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:44718 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756420AbcJGPX6 (ORCPT ); Fri, 7 Oct 2016 11:23:58 -0400 Message-Id: <20161007150211.271490994@infradead.org> User-Agent: quilt/0.63-1 Date: Fri, 07 Oct 2016 16:52:49 +0200 From: Peter Zijlstra To: Linus Torvalds , Waiman Long , Jason Low , Ding Tianhong , Thomas Gleixner , Will Deacon , Ingo Molnar , Imre Deak , Linux Kernel Mailing List , Davidlohr Bueso , Tim Chen , Terry Rudd , "Paul E. McKenney" , Jason Low , Peter Zijlstra Cc: Chris Wilson , Daniel Vetter Subject: [PATCH -v4 6/8] locking/mutex: Restructure wait loop References: <20161007145243.361481786@infradead.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Disposition: inline; filename=peterz-locking-mutex-wait_lock-opt.patch Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Doesn't really matter yet, but pull the HANDOFF and trylock out from under the wait_lock. The intention is to add an optimistic spin loop here, which requires we do not hold the wait_lock, so shuffle code around in preparation. Also clarify the purpose of taking the wait_lock in the wait loop, its tempting to want to avoid it altogether, but the cancellation cases need to to avoid losing wakeups. Suggested-by: Waiman Long Signed-off-by: Peter Zijlstra (Intel) --- kernel/locking/mutex.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -631,13 +631,21 @@ __mutex_lock_common(struct mutex *lock, lock_contended(&lock->dep_map, ip); + set_task_state(task, state); for (;;) { + /* + * Once we hold wait_lock, we're serialized against + * mutex_unlock() handing the lock off to us, do a trylock + * before testing the error conditions to make sure we pick up + * the handoff. + */ if (__mutex_trylock(lock, first)) - break; + goto acquired; /* - * got a signal? (This code gets eliminated in the - * TASK_UNINTERRUPTIBLE case.) + * Check for signals and wound conditions while holding + * wait_lock. This ensures the lock cancellation is ordered + * against mutex_unlock() and wake-ups do not go missing. */ if (unlikely(signal_pending_state(state, task))) { ret = -EINTR; @@ -650,16 +658,27 @@ __mutex_lock_common(struct mutex *lock, goto err; } - __set_task_state(task, state); spin_unlock_mutex(&lock->wait_lock, flags); schedule_preempt_disabled(); - spin_lock_mutex(&lock->wait_lock, flags); if (!first && __mutex_waiter_is_first(lock, &waiter)) { first = true; __mutex_set_flag(lock, MUTEX_FLAG_HANDOFF); } + + set_task_state(task, state); + /* + * Here we order against unlock; we must either see it change + * state back to RUNNING and fall through the next schedule(), + * or we must see its unlock and acquire. + */ + if (__mutex_trylock(lock, first)) + break; + + spin_lock_mutex(&lock->wait_lock, flags); } + spin_lock_mutex(&lock->wait_lock, flags); +acquired: __set_task_state(task, TASK_RUNNING); mutex_remove_waiter(lock, &waiter, task); @@ -682,6 +701,7 @@ __mutex_lock_common(struct mutex *lock, return 0; err: + __set_task_state(task, TASK_RUNNING); mutex_remove_waiter(lock, &waiter, task); spin_unlock_mutex(&lock->wait_lock, flags); debug_mutex_free_waiter(&waiter);