Re: [PATCH] selftests: powerpc: Fix online CPU selection

[Sandipan Das <sandipan@linux.ibm.com>]

On 08/06/20 8:12 pm, Sandipan Das wrote:
> The size of the cpu set must be large enough for systems
> with a very large number of CPUs. Otherwise, tests which
> try to determine the first online CPU by calling
> sched_getaffinity() will fail. This makes sure that the
> size of the allocated cpu set is dependent on the number
> of CPUs as reported by get_nprocs().
> 
> Fixes: 3752e453f6ba ("selftests/powerpc: Add tests of PMU EBBs")
> Reported-by: Shirisha Ganta <shiganta@in.ibm.com>
> Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
> ---
>  tools/testing/selftests/powerpc/utils.c | 33 ++++++++++++++++---------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/powerpc/utils.c b/tools/testing/selftests/powerpc/utils.c
> index 933678f1ed0a..bb8e402752c0 100644
> --- a/tools/testing/selftests/powerpc/utils.c
> +++ b/tools/testing/selftests/powerpc/utils.c
> @@ -16,6 +16,7 @@
> @@ -88,28 +89,36 @@ void *get_auxv_entry(int type)
> [...] 
>  int pick_online_cpu(void)
>  {
> -	cpu_set_t mask;
> -	int cpu;
> +	int ncpus, cpu = -1;
> +	cpu_set_t *mask;
> +	size_t size;
>  
> -	CPU_ZERO(&mask);
> +	ncpus = get_nprocs();
> +	size = CPU_ALLOC_SIZE(ncpus);
> +	mask = CPU_ALLOC(ncpus);
>  
> -	if (sched_getaffinity(0, sizeof(mask), &mask)) {
> +	CPU_ZERO_S(size, mask);
> +
> +	if (sched_getaffinity(0, size, mask)) {
>  		perror("sched_getaffinity");
> -		return -1;
> +		goto done;
>  	}
>  
>  	/* We prefer a primary thread, but skip 0 */
> -	for (cpu = 8; cpu < CPU_SETSIZE; cpu += 8)
> -		if (CPU_ISSET(cpu, &mask))
> -			return cpu;
> +	for (cpu = 8; cpu < ncpus; cpu += 8)
> +		if (CPU_ISSET_S(cpu, size, mask))
> +			goto done;
>  
>  	/* Search for anything, but in reverse */
> -	for (cpu = CPU_SETSIZE - 1; cpu >= 0; cpu--)
> -		if (CPU_ISSET(cpu, &mask))
> -			return cpu;
> +	for (cpu = ncpus - 1; cpu >= 0; cpu--)
> +		if (CPU_ISSET_S(cpu, size, mask))
> +			goto done;
>  
>  	printf("No cpus in affinity mask?!\n");
> -	return -1;

There's a bug here as cpu should have been set to -1.
Will send v2 with this fix.

> +
> +done:
> +	CPU_FREE(mask);
> +	return cpu;
>  }
>  
>  bool is_ppc64le(void)
> 

- Sandipan