From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1754604AbcHSMOT (ORCPT <rfc822;w@1wt.eu>);
        Fri, 19 Aug 2016 08:14:19 -0400
Received: from bombadil.infradead.org ([198.137.202.9]:51679 "EHLO
        bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750889AbcHSMOR (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 19 Aug 2016 08:14:17 -0400
Date: Fri, 19 Aug 2016 14:13:28 +0200
From: Peter Zijlstra <peterz@infradead.org>
To: Jason Low <jason.low2@hpe.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Ding Tianhong <dingtianhong@huawei.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Will Deacon <Will.Deacon@arm.com>, Ingo Molnar <mingo@redhat.com>,
        imre.deak@intel.com, linux-kernel@vger.kernel.org,
        Waiman Long <Waiman.Long@hpe.com>, Davidlohr Bueso <dave@stgolabs.net>,
        Tim Chen <tim.c.chen@linux.intel.com>, terry.rudd@hpe.com,
        "Paul E. McKenney" <paulmck@us.ibm.com>, jason.low2@hp.com
Subject: Re: [PATCH v4] locking/mutex: Prevent lock starvation when spinning
 is disabled
Message-ID: <20160819121328.GD10153@twins.programming.kicks-ass.net>
References: <1471567197.4991.41.camel@j-VirtualBox>
 <1471579876.4991.51.camel@j-VirtualBox>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1471579876.4991.51.camel@j-VirtualBox>
User-Agent: Mutt/1.5.23.1 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Aug 18, 2016 at 09:11:16PM -0700, Jason Low wrote:

> 3. Only clear yield_to_waiter if the thread is the top waiter and not if it
>    is a non-top waiter that received a signal.

Ah, and that is an equivalent condition to the singular one I use to
test if we need to set pending, since if we just add waiter and its the
top waiter, it must be the only waiter.

How about something like so on top?

--- a/kernel/locking/mutex.c
+++ b/kernel/locking/mutex.c
@@ -74,7 +74,6 @@ EXPORT_SYMBOL(__mutex_init);
  */
 __visible void __sched __mutex_lock_slowpath(atomic_t *lock_count);
 
-
 static inline bool need_yield_to_waiter(struct mutex *lock);
 
 /**
@@ -457,6 +456,12 @@ static bool mutex_optimistic_spin(struct
 }
 #endif
 
+static inline bool
+__mutex_waiter_is_head(struct mutex *lock, struct mutex_waiter *waiter)
+{
+	return list_first_entry(&lock->wait_list, struct mutex_waiter, list) == waiter;
+}
+
 #if !defined(CONFIG_MUTEX_SPIN_ON_OWNER) && defined(CONFIG_SMP)
 
 #define MUTEX_WAKEUP_THRESHOLD 16
@@ -471,7 +476,7 @@ static inline void clear_yield_to_waiter
 					 struct mutex_waiter *waiter)
 {
 	/*  Only clear yield_to_waiter if we are the top waiter. */
-	if (lock->wait_list.next == &waiter->list && lock->yield_to_waiter)
+	if (lock->yield_to_waiter && __mutex_waiter_is_head(lock, waiter))
 		lock->yield_to_waiter = false;
 }
 
@@ -648,7 +653,7 @@ __mutex_lock_common(struct mutex *lock,
 	 * If this is the first waiter, mark the lock as having pending
 	 * waiters, if we happen to acquire it while doing so, yay!
 	 */
-	if (list_is_singular(&lock->wait_list) &&
+	if (__mutex_waiter_is_head(lock, &waiter) &&
 	    __mutex_trylock_pending(lock))
 		goto remove_waiter;