Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

[Mark Rutland <mark.rutland@arm.com>]

On Tue, May 17, 2016 at 12:01:06PM -0700, Paul E. McKenney wrote:
> On Tue, May 17, 2016 at 11:22:10AM +0100, Mark Rutland wrote:
> >  /*
> > + * 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))
> 
> What if the rnp->grplo corresponds to a non-existent CPU?

Good point, I had evidently not considered that.

> Would something like this handle that possibility?
> 
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +	for ((cpu) = cpumask_next(rnp->grplo - 1, cpu_possible_mask); \
> +	     cpu <= rnp->grphi; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask))
> 
> Or maybe like this, with less duplicated code but very strange style:
> 
> +#define for_each_leaf_node_possible_cpu(rnp, cpu) \
> +	for ((cpu) = rnp->grplo - 1; \
> +	     cpu = cpumask_next((cpu), cpu_possible_mask), cpu <= rnp->grphi; 1)
> 
> The first one is probably far better, assuming that it works, but I could
> not resist inflicting the second one on you.  ;-)

:)

Those both look like they should work, I'll fold the former in.

> > +/*
> > + * 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))
> 
> Same question here.

Likewise.

I'll also see about fixing the build issue you spotted in the other reply; that
appears to be a typo (missing 'possible_' in the macro invocation).

I'm away from my development machine at the moment, so that may not appear
until next week.

Thanks,
Mark.