Message ID | 20200609034005.520137-1-harish@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v2] selftests: powerpc: Fix CPU affinity for child process | 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, 38 lines checked |
snowpatch_ozlabs/needsstable | warning | Please consider tagging this patch for stable! |
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 <shiganta@in.ibm.com> > Signed-off-by: Harish <harish@linux.ibm.com> > Signed-off-by: Sandipan Das <sandipan@linux.ibm.com> > --- > .../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 <limits.h> > #include <sys/time.h> > #include <sys/syscall.h> > +#include <sys/sysinfo.h> > #include <sys/types.h> > #include <sys/shm.h> > #include <linux/futex.h> > @@ -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(&cpuset); > - CPU_SET(cpu, &cpuset); > + 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), &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 >
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 <shiganta@in.ibm.com> > Signed-off-by: Harish <harish@linux.ibm.com> > Signed-off-by: Sandipan Das <sandipan@linux.ibm.com> > --- > .../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 <limits.h> > #include <sys/time.h> > #include <sys/syscall.h> > +#include <sys/sysinfo.h> > #include <sys/types.h> > #include <sys/shm.h> > #include <linux/futex.h> > @@ -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(&cpuset); > - CPU_SET(cpu, &cpuset); > + 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), &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); >
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 <limits.h> #include <sys/time.h> #include <sys/syscall.h> +#include <sys/sysinfo.h> #include <sys/types.h> #include <sys/shm.h> #include <linux/futex.h> @@ -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(&cpuset); - CPU_SET(cpu, &cpuset); + 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), &cpuset)) { + if (sched_setaffinity(0, size, cpuset)) { perror("sched_setaffinity"); - exit(1); + CPU_FREE(cpuset); + exit(-1); } fn(arg);