From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753094AbcBAKIk (ORCPT ); Mon, 1 Feb 2016 05:08:40 -0500 Received: from bombadil.infradead.org ([198.137.202.9]:49700 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751780AbcBAKIi (ORCPT ); Mon, 1 Feb 2016 05:08:38 -0500 Date: Mon, 1 Feb 2016 11:08:24 +0100 From: Peter Zijlstra To: Ding Tianhong Cc: Waiman Long , Jason Low , Ingo Molnar , "linux-kernel@vger.kernel.org" , Davidlohr Bueso , Linus Torvalds , "Paul E. McKenney" , Thomas Gleixner , Will Deacon , Tim Chen , Waiman Long Subject: [PATCH] locking/mutex: Avoid spinner vs waiter starvation Message-ID: <20160201100824.GO6357@twins.programming.kicks-ass.net> References: <56A1638A.7050202@hpe.com> <20160122085422.GO6357@twins.programming.kicks-ass.net> <1453458019.9727.8.camel@j-VirtualBox> <20160122105312.GQ6357@twins.programming.kicks-ass.net> <20160122105652.GE6375@twins.programming.kicks-ass.net> <20160122110653.GF6375@twins.programming.kicks-ass.net> <56A235DC.2060900@hpe.com> <56A48566.9040206@huawei.com> <20160129095347.GA6356@twins.programming.kicks-ass.net> <56AC0F74.2040808@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56AC0F74.2040808@huawei.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Jan 30, 2016 at 09:18:44AM +0800, Ding Tianhong wrote: > On 2016/1/29 17:53, Peter Zijlstra wrote: > > On Sun, Jan 24, 2016 at 04:03:50PM +0800, Ding Tianhong wrote: > > > >> looks good to me, I will try this solution and report the result, thanks everyone. > > > > Did you get a change to run with this? > > > > . > > > > I backport this patch to 3.10 lts kernel, and didn't change any logic, > Till now, the patch works fine to me, and no need to change anything, > So I think this patch is no problem, could you formal release this > patch to the latest kernel? :) Thanks for testing, I've queued the below patch. --- Subject: locking/mutex: Avoid spinner vs waiter starvation From: Peter Zijlstra Date: Fri, 22 Jan 2016 12:06:53 +0100 Ding Tianhong reported that under his load the optimistic spinners would totally starve a task that ended up on the wait list. Fix this by ensuring the top waiter also partakes in the optimistic spin queue. There are a few subtle differences between the assumed state of regular optimistic spinners and those already on the wait list, which result in the @acquired complication of the acquire path. Most notable are: - waiters are on the wait list and need to be taken off - mutex_optimistic_spin() sets the lock->count to 0 on acquire even though there might be more tasks on the wait list. Cc: Jason Low Cc: Ingo Molnar Cc: Tim Chen Cc: Linus Torvalds Cc: Waiman Long Cc: Thomas Gleixner Cc: "Paul E. McKenney" Cc: Davidlohr Bueso Cc: Will Deacon Reported-by: Ding Tianhong Tested-by: Ding Tianhong Tested-by: "Huang, Ying" Suggested-by: Waiman Long Signed-off-by: Peter Zijlstra (Intel) Link: http://lkml.kernel.org/r/20160122110653.GF6375@twins.programming.kicks-ass.net --- kernel/locking/mutex.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) --- a/kernel/locking/mutex.c +++ b/kernel/locking/mutex.c @@ -512,6 +512,7 @@ __mutex_lock_common(struct mutex *lock, struct task_struct *task = current; struct mutex_waiter waiter; unsigned long flags; + bool acquired; int ret; preempt_disable(); @@ -543,6 +544,7 @@ __mutex_lock_common(struct mutex *lock, lock_contended(&lock->dep_map, ip); for (;;) { + acquired = false; /* * Lets try to take the lock again - this is needed even if * we get here for the first time (shortly after failing to @@ -577,7 +579,16 @@ __mutex_lock_common(struct mutex *lock, /* didn't get the lock, go to sleep: */ spin_unlock_mutex(&lock->wait_lock, flags); schedule_preempt_disabled(); + + if (mutex_is_locked(lock)) + acquired = mutex_optimistic_spin(lock, ww_ctx, use_ww_ctx); + spin_lock_mutex(&lock->wait_lock, flags); + + if (acquired) { + atomic_set(&lock->count, -1); + break; + } } __set_task_state(task, TASK_RUNNING); @@ -587,6 +598,9 @@ __mutex_lock_common(struct mutex *lock, atomic_set(&lock->count, 0); debug_mutex_free_waiter(&waiter); + if (acquired) + goto unlock; + skip_wait: /* got the lock - cleanup and rejoice! */ lock_acquired(&lock->dep_map, ip); @@ -597,6 +611,7 @@ __mutex_lock_common(struct mutex *lock, ww_mutex_set_context_slowpath(ww, ww_ctx); } +unlock: spin_unlock_mutex(&lock->wait_lock, flags); preempt_enable(); return 0;