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 S1758143AbbA3Hvp (ORCPT <rfc822;w@1wt.eu>);
	Fri, 30 Jan 2015 02:51:45 -0500
Received: from casper.infradead.org ([85.118.1.10]:60856 "EHLO
	casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753873AbbA3Hvo (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 30 Jan 2015 02:51:44 -0500
Date: Fri, 30 Jan 2015 08:51:40 +0100
From: Peter Zijlstra <peterz@infradead.org>
To: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jason Low <jason.low2@hp.com>, Ingo Molnar <mingo@kernel.org>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        Michel Lespinasse <walken@google.com>,
        Tim Chen <tim.c.chen@linux.intel.com>, linux-kernel@vger.kernel.org
Subject: Re: Refactoring mutex spin on owner code
Message-ID: <20150130075140.GS2896@worktop.programming.kicks-ass.net>
References: <1422257769-14083-5-git-send-email-dave@stgolabs.net>
 <1422379430.6710.6.camel@j-VirtualBox>
 <1422417294.4604.15.camel@stgolabs.net>
 <1422479028.4111.34.camel@j-VirtualBox>
 <1422493812.4604.29.camel@stgolabs.net>
 <1422562401.2418.13.camel@j-VirtualBox>
 <1422562731.2418.16.camel@j-VirtualBox>
 <1422573326.2005.7.camel@stgolabs.net>
 <1422582768.2418.31.camel@j-VirtualBox>
 <1422602080.2005.9.camel@stgolabs.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1422602080.2005.9.camel@stgolabs.net>
User-Agent: Mutt/1.5.22.1 (2013-10-16)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Jan 29, 2015 at 11:14:40PM -0800, Davidlohr Bueso wrote:
> > +bool mutex_spin_on_owner(struct mutex *lock, struct task_struct *owner)
> >  {
> > +	bool ret;
> > +
> >  	rcu_read_lock();
> > -	while (owner_running(lock, owner)) {
> > -		if (need_resched())
> > +	while (true) {
> > +		/* Return success when the lock owner changed */
> > +		if (lock->owner != owner) {
> 
> Shouldn't this be a READ_ONCE(lock->owner)? We're in a loop and need to
> avoid gcc giving us stale data if the owner is updated after a few
> iterations, no?

There's a barrier() in that loop, and cpu_relax() also implies
barrier(). I'm pretty sure that's more than sufficient to make GCC emit
loads.