Re: [PATCH] x86: Fix /proc/cpuinfo cpumask warning

[Andrew Jones]

On Tue, Oct 11, 2022 at 08:01:03PM +0200, Borislav Petkov wrote:
> On Tue, Oct 11, 2022 at 07:50:31PM +0200, Andrew Jones wrote:
> > Upcoming cpumask changes will start issuing warnings[*] when cpu
> 
> What upcoming changes?
> 
> This needs a concrete pointer to a commit or so.

Hi Boris,

Sorry, I should have pointed this out. The upcoming change is

linux-next/master commit a314123c8bdb ("cpumask: fix checking valid cpu
range")

And also an ongoing discussion here
https://lore.kernel.org/lkml/20221011170949.upxk3tcfcwnkytwm@kamzik/

I'm hoping that Yury will pick these patches up and integrate
them at the front of his series when introducing the warnings.
I wasn't sure how to call that out other than with the generic
"upcoming change".

> 
> > indices equal to nr_cpu_ids are passed to cpumask_next* functions.
> 
> How do those indices get passed here? I think you need to explain how
> exactly this happens.

I took a stab at explaining it in the discussion thread[1] just now and I
can bring that explanation into the commit message for v2.

[1] https://lore.kernel.org/lkml/20221011180442.cwjtcvjioias3qt6@kamzik/

> 
> > Ensure we don't generate a warning when reading /proc/cpuinfo by
> 
> Please use passive voice in your commit message: no "we" or "I", etc,
> and describe your changes in imperative mood.

I'll change to "Ensure no warning is generated ..."

> 
> > validating the cpu index before calling cpumask_next().
> 
> s/cpu/CPU/g
> 
> > [*] Warnings will only appear with DEBUG_PER_CPU_MAPS enabled.
> > 
> > Signed-off-by: Andrew Jones <ajones@xxxxxxxxxxxxxxxx>
> > Cc: Yury Norov <yury.norov@xxxxxxxxx>
> > ---
> >  arch/x86/kernel/cpu/proc.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> > index 099b6f0d96bd..584ae6cb5b87 100644
> > --- a/arch/x86/kernel/cpu/proc.c
> > +++ b/arch/x86/kernel/cpu/proc.c
> > @@ -153,9 +153,12 @@ static int show_cpuinfo(struct seq_file *m, void *v)
> >  
> >  static void *c_start(struct seq_file *m, loff_t *pos)
> >  {
> > -	*pos = cpumask_next(*pos - 1, cpu_online_mask);
> > -	if ((*pos) < nr_cpu_ids)
> > -		return &cpu_data(*pos);
> > +	if (*pos < nr_cpu_ids) {
> > +		*pos = cpumask_next(*pos - 1, cpu_online_mask);
> > +		if (*pos < nr_cpu_ids)
> > +			return &cpu_data(*pos);
> > +	}
> 
> Simpler: on function entry:
> 
> 	if (*pos >= nr_cpu_ids)
> 		return NULL;
> 
> 	 /* the rest remains unchanged */

Will do for v2.

Thanks,
drew