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 S1755555AbbDJJEs (ORCPT <rfc822;w@1wt.eu>);
	Fri, 10 Apr 2015 05:04:48 -0400
Received: from mail-wg0-f48.google.com ([74.125.82.48]:33081 "EHLO
	mail-wg0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1754404AbbDJJEo (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Fri, 10 Apr 2015 05:04:44 -0400
Date: Fri, 10 Apr 2015 11:04:39 +0200
From: Ingo Molnar <mingo@kernel.org>
To: Jason Low <jason.low2@hp.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Paul McKenney <paulmck@linux.vnet.ibm.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Aswin Chandramouleeswaran <aswin@hp.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/2] locking/rwsem: Use a return variable in
 rwsem_spin_on_owner()
Message-ID: <20150410090439.GB28549@gmail.com>
References: <1428521960-5268-1-git-send-email-jason.low2@hp.com>
 <1428521960-5268-3-git-send-email-jason.low2@hp.com>
 <20150409053725.GB13871@gmail.com>
 <1428561611.3506.78.camel@j-VirtualBox>
 <20150409075311.GA4645@gmail.com>
 <CA+55aFz6KKxGVxPAbsmw9GsKJfy85P2C0EmYBrGpn+aJDjZJWw@mail.gmail.com>
 <20150409175652.GI6464@linux.vnet.ibm.com>
 <CA+55aFzXMDjQQ7jTjsPdh1RikXfgV7OCd-+13cz06MOmDBA33w@mail.gmail.com>
 <CA+55aFwZWi6ecDmVsMBQJTrgrW3GD2DaRtpiOspe=5amR1=dNg@mail.gmail.com>
 <1428611797.12911.18.camel@j-VirtualBox>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1428611797.12911.18.camel@j-VirtualBox>
User-Agent: Mutt/1.5.23 (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


* Jason Low <jason.low2@hp.com> wrote:

> +static inline bool owner_running(struct rw_semaphore *sem, struct task_struct *owner)
> +{
> +	bool ret;
> +
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> +	rcu_read_lock();
> +#endif
> +	if (READ_ONCE(sem->owner) == owner) {
> +		/*
> +		 * Ensure we emit the owner->on_cpu dereference
> +		 * after checking sem->owner still matches owner.
> +		 */
> +		barrier();
> +		ret = owner->on_cpu;
> +	} else {
> +		/* Owner changed. */
> +		ret = false;
> +	}
> +
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> +	rcu_read_unlock();
> +#endif
> +	return ret;
> +}

So I really don't like this due to the assymetric RCU pattern. (Also, 
the fact that we might read from already freed kernel memory here 
needs big honking comments.)

But, I think we can do this differently, see the patch I just sent:

  [PATCH] mutex: Speed up mutex_spin_on_owner() by not taking the RCU lock

that should I think work just as well, without having to introduce 
owner_running() as the whole mutex_spin_on_owner() logic is kept 
pretty simple.

Thanks,

	Ingo