Re: [PATCH 3/4] cpumask: Convert cpumask_any_but() to the new random function

[Qais Yousef]

On 04/14/20 12:28, Steven Rostedt wrote:
> On Tue, 14 Apr 2020 16:05:55 +0100
> Qais Yousef <qais.yousef@xxxxxxx> wrote:
> 
> > +int cpumask_any_but(const struct cpumask *srcp, unsigned int cpu)
> > +{
> > +	unsigned int i;
> > +
> > +	cpumask_check(cpu);
> > +
> > +	for_each_cpu(i, srcp) {
> > +		i = cpumask_any(srcp);
> 
> Hmm, if the current CPU is the last CPU in the mask, and cpumask_any()
> happens to return it, what happens?

cpumask_any() will wrap.

> 
> > +		if (i != cpu)
> > +			return i;
> 
> We loop again, and wouldn't i being the last CPU in the mask cause this
> loop to exit, and return nr_cpu_ids?

No, because if we happen to start from the last cpu, on the next call, we'll
wrap again to the beginning.

But this implementation is crap indeed. No matter how unlikely, there's no
guarantee that for all the iters cpumask_any() will return a different cpu.
So if there are 3 cpus in the mask, cpumask_any() could potentially always
return 'cpu' if the planets aligned correctly.

I'll open code it instead to guarantee robustness.

Thanks!

--
Qais Yousef