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 S1753782AbcERSoU (ORCPT <rfc822;w@1wt.eu>);
	Wed, 18 May 2016 14:44:20 -0400
Received: from e36.co.us.ibm.com ([32.97.110.154]:53223 "EHLO
	e36.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752276AbcERSoS (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Wed, 18 May 2016 14:44:18 -0400
X-IBM-Helo: d03dlp02.boulder.ibm.com
X-IBM-MailFrom: paulmck@linux.vnet.ibm.com
X-IBM-RcptTo: linux-kernel@vger.kernel.org
Date: Wed, 18 May 2016 11:44:16 -0700
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        Catalin Marinas <catalin.marinas@arm.com>, dennis.chen@arm.com,
        jiangshanlai@gmail.com, josh@joshtriplett.org,
        LKML <linux-kernel@vger.kernel.org>, mathieu.desnoyers@efficios.com,
        Steven Rostedt <rostedt@goodmis.org>, steve.capper@arm.com,
        Will Deacon <will.deacon@arm.com>
Subject: Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs
Message-ID: <20160518184416.GC3528@linux.vnet.ibm.com>
Reply-To: paulmck@linux.vnet.ibm.com
References: <1463417306-3826-1-git-send-email-mark.rutland@arm.com>
 <CAPAsAGzYxfEsc8EMVHr0NFP1x1CjtdHbi_GApfbu18of98mBvw@mail.gmail.com>
 <20160518180153.GA3528@linux.vnet.ibm.com>
 <20160518183041.GB8664@svinekod>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160518183041.GB8664@svinekod>
User-Agent: Mutt/1.5.21 (2010-09-15)
X-TM-AS-MML: disable
X-Content-Scanned: Fidelis XPS MAILER
x-cbid: 16051818-0021-0000-0000-000051E6613A
X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Wed, May 18, 2016 at 07:30:41PM +0100, Mark Rutland wrote:
> On Wed, May 18, 2016 at 11:01:53AM -0700, Paul E. McKenney wrote:
> > On Wed, May 18, 2016 at 06:15:23PM +0300, Andrey Ryabinin wrote:
> > > 2016-05-16 19:48 GMT+03:00 Mark Rutland <mark.rutland@arm.com>:
> > > 
> > > >  /*
> > > > + * Iterate over all possible CPUs in a leaf RCU node.
> > > > + */
> > > > +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> > > > +       for ((cpu) = rnp->grplo; \
> > > > +            cpu <= rnp->grphi; \
> > > > +            cpu = cpumask_next((cpu), cpu_possible_mask))
> > > > +
> > > > +/*
> > > > + * Iterate over all possible CPUs in a leaf RCU node, at each step providing a
> > > > + * bit for comparison against rcu_node bitmasks.
> > > > + */
> > > > +#define for_each_leaf_node_possible_cpu_bit(rnp, cpu, bit) \
> > > > +       for ((cpu) = rnp->grplo, (bit) = 1; \
> > > > +            cpu <= rnp->grphi; \
> > > > +            cpu = cpumask_next((cpu), cpu_possible_mask), \
> > > > +                  (bit) = 1UL << (cpu - rnp->grplo))
> > > > +
> > > 
> > > [    0.163652] UBSAN: Undefined behaviour in ../kernel/rcu/tree.c:2912:3
> > > [    0.164000] shift exponent 64 is too large for 64-bit type 'long
> > > unsigned int'
> > 
> > Ah, dead value, but can happen nevertheless.  One fix is to prevent the
> > assignment to bit when cpu > rnp->grphi.
> > 
> > Any ideas for a better fix?  And isn't there some combination of
> > signedness that makes shifting all the bits out of the value defined
> > to zero?  Or is that only for right shifts?
> 
> We could add a (leaf/rcu)_node_cpu_mask(rnp, cpu) macro, and only use that in
> the body of the loop. That would avoid the stale value and would be useful in a
> couple of additional places.
> 
> If that makes sense to you, I can respin the patch with that.

Please try it and then let's see what it looks like.

							Thanx, Paul