Message ID | 20200608144212.985144-1-sandipan@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | selftests: powerpc: Fix online CPU selection | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (ec7b8eb9bc7a519047485c95f7292b48f5b73fe6) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 55 lines checked |
snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
On 6/8/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> LGTM, Reviewed-by: Kamalesh Babulal <kamalesh@linux.vnet.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
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 @@ > [...] > > 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; Missed the fact that the for loop before this would anyway make 'cpu' count down to -1 if no online CPU is found. Please ignore the previous message. > + > +done: > + CPU_FREE(mask); > + return cpu; > } > > bool is_ppc64le(void) > - Sandipan
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 @@ #include <string.h> #include <sys/ioctl.h> #include <sys/stat.h> +#include <sys/sysinfo.h> #include <sys/types.h> #include <sys/utsname.h> #include <unistd.h> @@ -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; + +done: + CPU_FREE(mask); + return cpu; } bool is_ppc64le(void)
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(-)