[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160518184416.GC3528@linux.vnet.ibm.com>
Date: Wed, 18 May 2016 11:44:16 -0700
From: "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To: Mark Rutland <mark.rutland@....com>
Cc: Andrey Ryabinin <ryabinin.a.a@...il.com>,
Catalin Marinas <catalin.marinas@....com>, dennis.chen@....com,
jiangshanlai@...il.com, josh@...htriplett.org,
LKML <linux-kernel@...r.kernel.org>,
mathieu.desnoyers@...icios.com,
Steven Rostedt <rostedt@...dmis.org>, steve.capper@....com,
Will Deacon <will.deacon@....com>
Subject: Re: [PATCH] rcu: tree: correctly handle sparse possible CPUs
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@....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
Powered by blists - more mailing lists