On Mon, Mar 22, 2021 at 09:45:09AM +0000, Nikos Nikoleris wrote:
> Hi Drew,
>
> On 22/03/2021 09:31, Andrew Jones wrote:
> > On Fri, Mar 19, 2021 at 12:24:11PM +0000, Nikos Nikoleris wrote:
> > > Signed-off-by: Nikos Nikoleris <nikos.nikoleris@xxxxxxx>
> > > ---
> > > lib/arm/asm/cpumask.h | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/lib/arm/asm/cpumask.h b/lib/arm/asm/cpumask.h
> > > index 6683bb6..02124de 100644
> > > --- a/lib/arm/asm/cpumask.h
> > > +++ b/lib/arm/asm/cpumask.h
> > > @@ -105,7 +105,7 @@ static inline void cpumask_copy(cpumask_t *dst, const cpumask_t *src)
> > > static inline int cpumask_next(int cpu, const cpumask_t *mask)
> > > {
> > > - while (cpu < nr_cpus && !cpumask_test_cpu(++cpu, mask))
> > > + while (++cpu < nr_cpus && !cpumask_test_cpu(cpu, mask))
> > > ;
> > > return cpu;
> >
>
> Thanks for reviewing this!
>
>
> > This looks like the right thing to do, but I'm surprised that
> > I've never seen an assert in cpumask_test_cpu, even though
> > it looks like we call cpumask_next with cpu == nr_cpus - 1
> > in several places.
> >
>
> cpumask_next() would trigger one of the assertions in the 4th patch in this
> series without this fix. The 4th patch is a way to demonstrate (if we apply
> it without the rest) the problem of using cpu0's thread_info->cpu
> uninitialized.
Ah, I see my error. I had already applied your 4th patch but hadn't
reviewed it yet, so I didn't realize it was new code. Now it makes
sense that we didn't hit that assert before (it didn't exist
before :-)
>
> > Can you please add a commit message explaining how you found
> > this bug?
> >
>
> Yes I'll do that.
If you just write one here then I'll add it while applying. The rest of
the patches look good to me. So no need to respin.
Thanks,
drew
