Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932966AbbFEN7Y (ORCPT ); Fri, 5 Jun 2015 09:59:24 -0400 Received: from www.linutronix.de ([62.245.132.108]:35525 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932928AbbFEN7S (ORCPT ); Fri, 5 Jun 2015 09:59:18 -0400 Date: Fri, 5 Jun 2015 15:59:10 +0200 (CEST) From: Thomas Gleixner To: Davidlohr Bueso cc: Peter Zijlstra , Ingo Molnar , Steven Rostedt , Mike Galbraith , "Paul E. McKenney" , Sebastian Andrzej Siewior , linux-kernel@vger.kernel.org, Davidlohr Bueso Subject: Re: [PATCH -rfc 4/4] locking/rtmutex: Support spin on owner (osq) In-Reply-To: <1432056298-18738-5-git-send-email-dave@stgolabs.net> Message-ID: References: <1432056298-18738-1-git-send-email-dave@stgolabs.net> <1432056298-18738-5-git-send-email-dave@stgolabs.net> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2228 Lines: 72 On Tue, 19 May 2015, Davidlohr Bueso wrote: > > +/* > + * Lockless alternative to rt_mutex_has_waiters() as we do not need the > + * wait_lock to check if we are in, for instance, a transitional state > + * after calling mark_rt_mutex_waiters(). Before I get into a state of brain melt, could you please explain that in an understandable way? rt_mutex_has_waiters() looks at the root pointer of the rbtree head whether that's empty. You can do a lockless check of that as well, right? So what's the FAST part of that function and how is that related to a point after we called mark_rt_mutex_waiters()? > + */ > +static inline bool rt_mutex_has_waiters_fast(struct rt_mutex *lock) > +{ > + unsigned long val = (unsigned long)lock->owner; > + > + if (!val) > + return false; > + return val & RT_MUTEX_HAS_WAITERS; > +} > + > +/* > + * Initial check for entering the mutex spinning loop > + */ > +static inline bool rt_mutex_can_spin_on_owner(struct rt_mutex *lock) > +{ > + struct task_struct *owner; > + /* default return to spin: if no owner, the lock is free */ Rather than having a comment in the middle of the variable declaration section, I'd prefer a comment explaing the whole logic of this function. > + int ret = true; > +static bool rt_mutex_optimistic_spin(struct rt_mutex *lock) > +{ > + bool taken = false; > + > + preempt_disable(); > + > + if (!rt_mutex_can_spin_on_owner(lock)) > + goto done; > + /* > + * In order to avoid a stampede of mutex spinners trying to > + * acquire the mutex all at once, the spinners need to take a > + * MCS (queued) lock first before spinning on the owner field. > + */ > + if (!osq_lock(&lock->osq)) > + goto done; Hmm. The queue lock is serializing potential spinners, right? So that's going to lead to a potential priority ordering problem because if a lower prio task wins the racing to the ocq_lock queue, then the higher prio waiter will be queued behind and blocked from taking the lock first. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/