Re: [PATCH v2] selftests: powerpc: Fix CPU affinity for child process
On 6/9/20 9:10 AM, Harish wrote: > On systems with large number of cpus, test fails trying to set > affinity for child process by calling sched_setaffinity() with > smaller size for cpuset. This patch fixes it by making sure that > the size of allocated cpu set is dependent on the number of CPUs > as reported by get_nprocs(). > > Fixes: 00b7ec5c9cf3 ("selftests/powerpc: Import Anton's context_switch2 > benchmark") > Reported-by: Shirisha Ganta > Signed-off-by: Harish > Signed-off-by: Sandipan Das > --- > .../powerpc/benchmarks/context_switch.c| 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/powerpc/benchmarks/context_switch.c > b/tools/testing/selftests/powerpc/benchmarks/context_switch.c > index a2e8c9da7fa5..de6c49d6f88f 100644 > --- a/tools/testing/selftests/powerpc/benchmarks/context_switch.c > +++ b/tools/testing/selftests/powerpc/benchmarks/context_switch.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -104,8 +105,9 @@ static void start_thread_on(void *(*fn)(void *), void > *arg, unsigned long cpu) > > static void start_process_on(void *(*fn)(void *), void *arg, unsigned long > cpu) > { > - int pid; > - cpu_set_t cpuset; > + int pid, ncpus; > + cpu_set_t *cpuset; > + size_t size; > > pid = fork(); > if (pid == -1) { > @@ -116,12 +118,16 @@ static void start_process_on(void *(*fn)(void *), void > *arg, unsigned long cpu) > if (pid) > return; > > - CPU_ZERO(); > - CPU_SET(cpu, ); > + size = CPU_ALLOC_SIZE(ncpus); > + ncpus = get_nprocs(); > + cpuset = CPU_ALLOC(ncpus); CPU_ALLOC() allocation failure needs to be checked, like malloc() allocations. > + CPU_ZERO_S(size, cpuset); > + CPU_SET_S(cpu, size, cpuset); > > - if (sched_setaffinity(0, sizeof(cpuset), )) { > + if (sched_setaffinity(0, size, cpuset)) { > perror("sched_setaffinity"); > - exit(1); > + CPU_FREE(cpuset); > + exit(-1); > } once the cpu affinity is set, you probably want to free the cpuset mask. > > fn(arg); > -- Kamalesh
Re: [PATCH v2] selftests: powerpc: Fix CPU affinity for child process
On Tue, Jun 09, 2020 at 09:10:05AM +0530, Harish wrote: > On systems with large number of cpus, test fails trying to set > affinity for child process by calling sched_setaffinity() with > smaller size for cpuset. This patch fixes it by making sure that > the size of allocated cpu set is dependent on the number of CPUs > as reported by get_nprocs(). > > Fixes: 00b7ec5c9cf3 ("selftests/powerpc: Import Anton's context_switch2 > benchmark") > Reported-by: Shirisha Ganta > Signed-off-by: Harish > Signed-off-by: Sandipan Das > --- > .../powerpc/benchmarks/context_switch.c| 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/tools/testing/selftests/powerpc/benchmarks/context_switch.c > b/tools/testing/selftests/powerpc/benchmarks/context_switch.c > index a2e8c9da7fa5..de6c49d6f88f 100644 > --- a/tools/testing/selftests/powerpc/benchmarks/context_switch.c > +++ b/tools/testing/selftests/powerpc/benchmarks/context_switch.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -104,8 +105,9 @@ static void start_thread_on(void *(*fn)(void *), void > *arg, unsigned long cpu) > > static void start_process_on(void *(*fn)(void *), void *arg, unsigned long > cpu) > { > - int pid; > - cpu_set_t cpuset; > + int pid, ncpus; > + cpu_set_t *cpuset; > + size_t size; > > pid = fork(); > if (pid == -1) { > @@ -116,12 +118,16 @@ static void start_process_on(void *(*fn)(void *), void > *arg, unsigned long cpu) > if (pid) > return; > > - CPU_ZERO(); > - CPU_SET(cpu, ); > + size = CPU_ALLOC_SIZE(ncpus); > + ncpus = get_nprocs(); above two lines should be interchanged, ncpus not assigned while getting used to get size. > + cpuset = CPU_ALLOC(ncpus); > + CPU_ZERO_S(size, cpuset); > + CPU_SET_S(cpu, size, cpuset); > > - if (sched_setaffinity(0, sizeof(cpuset), )) { > + if (sched_setaffinity(0, size, cpuset)) { > perror("sched_setaffinity"); > - exit(1); > + CPU_FREE(cpuset); > + exit(-1); do we need to change the return value here? probably other framework might rely on previous value? Regards, -Satheesh. > } > > fn(arg); > -- > 2.24.1 >
[PATCH v2] selftests: powerpc: Fix CPU affinity for child process
On systems with large number of cpus, test fails trying to set affinity for child process by calling sched_setaffinity() with smaller size for cpuset. This patch fixes it by making sure that the size of allocated cpu set is dependent on the number of CPUs as reported by get_nprocs(). Fixes: 00b7ec5c9cf3 ("selftests/powerpc: Import Anton's context_switch2 benchmark") Reported-by: Shirisha Ganta Signed-off-by: Harish Signed-off-by: Sandipan Das --- .../powerpc/benchmarks/context_switch.c| 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/powerpc/benchmarks/context_switch.c b/tools/testing/selftests/powerpc/benchmarks/context_switch.c index a2e8c9da7fa5..de6c49d6f88f 100644 --- a/tools/testing/selftests/powerpc/benchmarks/context_switch.c +++ b/tools/testing/selftests/powerpc/benchmarks/context_switch.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -104,8 +105,9 @@ static void start_thread_on(void *(*fn)(void *), void *arg, unsigned long cpu) static void start_process_on(void *(*fn)(void *), void *arg, unsigned long cpu) { - int pid; - cpu_set_t cpuset; + int pid, ncpus; + cpu_set_t *cpuset; + size_t size; pid = fork(); if (pid == -1) { @@ -116,12 +118,16 @@ static void start_process_on(void *(*fn)(void *), void *arg, unsigned long cpu) if (pid) return; - CPU_ZERO(); - CPU_SET(cpu, ); + size = CPU_ALLOC_SIZE(ncpus); + ncpus = get_nprocs(); + cpuset = CPU_ALLOC(ncpus); + CPU_ZERO_S(size, cpuset); + CPU_SET_S(cpu, size, cpuset); - if (sched_setaffinity(0, sizeof(cpuset), )) { + if (sched_setaffinity(0, size, cpuset)) { perror("sched_setaffinity"); - exit(1); + CPU_FREE(cpuset); + exit(-1); } fn(arg); -- 2.24.1