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

From: Qais Yousef <qais.yousef@arm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Juri Lelli <juri.lelli@redhat.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Yury Norov <yury.norov@gmail.com>, Paul Turner <pjt@google.com>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Josh Don <joshdon@google.com>,
	Pavan Kondeti <pkondeti@codeaurora.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] cpumask: Convert cpumask_any_but() to the new random function
Date: Mon, 20 Apr 2020 16:49:29 +0100	[thread overview]
Message-ID: <20200420154929.taacfhjyeku4e5bx@e107158-lin.cambridge.arm.com> (raw)
In-Reply-To: <20200414122810.4b83ddd2@gandalf.local.home>

On 04/14/20 12:28, Steven Rostedt wrote:
> On Tue, 14 Apr 2020 16:05:55 +0100
> Qais Yousef <qais.yousef@arm.com> 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