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 S933429AbbDITnz (ORCPT <rfc822;w@1wt.eu>);
	Thu, 9 Apr 2015 15:43:55 -0400
Received: from g4t3425.houston.hp.com ([15.201.208.53]:50285 "EHLO
	g4t3425.houston.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S933249AbbDITnw (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 9 Apr 2015 15:43:52 -0400
Message-ID: <1428608618.12911.9.camel@j-VirtualBox>
Subject: Re: [PATCH 2/2] locking/rwsem: Use a return variable in
 rwsem_spin_on_owner()
From: Jason Low <jason.low2@hp.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>, Ingo Molnar <mingo@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        Aswin Chandramouleeswaran <aswin@hp.com>,
        LKML <linux-kernel@vger.kernel.org>, jason.low2@hp.com
Date: Thu, 09 Apr 2015 12:43:38 -0700
In-Reply-To: <CA+55aFwZWi6ecDmVsMBQJTrgrW3GD2DaRtpiOspe=5amR1=dNg@mail.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>
Content-Type: text/plain; charset="UTF-8"
X-Mailer: Evolution 3.2.3-0ubuntu6 
Content-Transfer-Encoding: 7bit
Mime-Version: 1.0
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, 2015-04-09 at 11:16 -0700, Linus Torvalds wrote:
> On Thu, Apr 9, 2015 at 11:08 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > The pointer is a known-safe kernel pointer - it's just that it was
> > "known safe" a few instructions ago, and might be rcu-free'd at any
> > time.
> 
> Actually, we could even do something like this:
> 
>  static inline int sem_owner_on_cpu(struct semaphore *sem, struct
> task_struct *owner)
>  {
>         int on_cpu;
> 
>     #ifdef CONFIG_DEBUG_PAGEALLOC
>         rcu_read_lock();
>     #endif
>         on_cpu = sem->owner == owner && owner->on_cpu;
>     #ifdef CONFIG_DEBUG_PAGEALLOC
>         rcu_read_unlock();
>     #endif
>         return on_cpu;
>     }
> 
> because we really don't need to hold the RCU lock over the whole loop,
> we just need to validate that the semaphore owner still matches, and
> if so, check that it's on_cpu.
> 
> And if CONFIG_DEBUG_PAGEALLOC is set, we don't care about performance
> *at*all*. We will have worse performance problems than doing some RCU
> read-locking inside the loop.
> 
> And if CONFIG_DEBUG_PAGEALLOC isn't set, we don't really care about
> locking, since at worst we just access stale memory for one iteration.
> 
> Hmm. It's not pretty, but neither is the current "let's just take a
> rcu lock that we don't really need over a loop that doesn't have very
> strict bounding".
> 
> Comments?

So that looks more similar to how the original code was where the
rcu_read_lock() and rcu_read_unlock() was done inside the owner_running
helper function (though without the CONFIG_DEBUG_PAGEALLOC), before
commit 307bf9803f25 ("sched: Simplify mutex_spin_on_owner()") modified
it to be done outside the loop.