Re: [kvm-unit-tests PATCH 1/4] arm/arm64: Avoid calling cpumask_test_cpu for CPUs above nr_cpu

[Nikos Nikoleris <nikos.nikoleris@arm.com>]

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@arm.com>
>> ---
>>   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.

> Can you please add a commit message explaining how you found
> this bug?
> 

Yes I'll do that.

Thanks,

Nikos

> Thanks,
> drew
>