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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 13 May 2016 02:27:01 +0100
From:	Mark Rutland <mark.rutland@....com>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	catalin.marinas@....com, dennis.chen@....com,
	jiangshanlai@...il.com, josh@...htriplett.org,
	linux-kernel@...r.kernel.org, mathieu.desnoyers@...icios.com,
	rostedt@...dmis.org, steve.capper@....com, will.deacon@....com
Subject: Re: [PATCHv2] rcu: tree: correctly handle sparse possible CPUs

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.