* [PATCH v2] libperf tests: Fix test_stat_cpu @ 2021-10-06 9:48 Shunsuke Nakamura 2021-10-07 17:05 ` Jiri Olsa 0 siblings, 1 reply; 4+ messages in thread From: Shunsuke Nakamura @ 2021-10-06 9:48 UTC (permalink / raw) To: peterz, mingo, acme, mark.rutland, alexander.shishkin, jolsa, namhyung Cc: linux-kernel, linux-perf-users `cpu` of perf_evsel__read() must be specified the cpu index. perf_cpu_map__for_each_cpu is for iterating the cpu number (not index) and is not appropriate. So, if there is an offline CPU, the cpu number specified in the argument may point out of range because the cpu number and the cpu index are different. Fix test_stat_cpu. Committer testing: # make tests -C tools/lib/perf/ make: Entering directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' running static: - running tests/test-cpumap.c...OK - running tests/test-threadmap.c...OK - running tests/test-evlist.c...OK - running tests/test-evsel.c...OK running dynamic: - running tests/test-cpumap.c...OK - running tests/test-threadmap.c...OK - running tests/test-evlist.c...OK - running tests/test-evsel.c...OK make: Leaving directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com> --- Previous version at: https://lore.kernel.org/lkml/20211006080456.474273-1-nakamura.shun@fujitsu.com/ Changes in v2: - Remove "2/2" from Patch Subject tools/lib/perf/tests/test-evlist.c | 6 +++--- tools/lib/perf/tests/test-evsel.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c index c67c83399170..47badd7eabf2 100644 --- a/tools/lib/perf/tests/test-evlist.c +++ b/tools/lib/perf/tests/test-evlist.c @@ -40,7 +40,7 @@ static int test_stat_cpu(void) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_TASK_CLOCK, }; - int err, cpu, tmp; + int err, idx; cpus = perf_cpu_map__new(NULL); __T("failed to create cpus", cpus); @@ -70,10 +70,10 @@ static int test_stat_cpu(void) perf_evlist__for_each_evsel(evlist, evsel) { cpus = perf_evsel__cpus(evsel); - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { + for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { struct perf_counts_values counts = { .val = 0 }; - perf_evsel__read(evsel, cpu, 0, &counts); + perf_evsel__read(evsel, idx, 0, &counts); __T("failed to read value for evsel", counts.val != 0); } } diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c index 9abd4c0bf6db..33ae9334861a 100644 --- a/tools/lib/perf/tests/test-evsel.c +++ b/tools/lib/perf/tests/test-evsel.c @@ -22,7 +22,7 @@ static int test_stat_cpu(void) .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_CLOCK, }; - int err, cpu, tmp; + int err, idx; cpus = perf_cpu_map__new(NULL); __T("failed to create cpus", cpus); @@ -33,10 +33,10 @@ static int test_stat_cpu(void) err = perf_evsel__open(evsel, cpus, NULL); __T("failed to open evsel", err == 0); - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { + for (idx = 0; idx < perf_cpu_map__nr(cpus); idx++) { struct perf_counts_values counts = { .val = 0 }; - perf_evsel__read(evsel, cpu, 0, &counts); + perf_evsel__read(evsel, idx, 0, &counts); __T("failed to read value for evsel", counts.val != 0); } -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libperf tests: Fix test_stat_cpu 2021-10-06 9:48 [PATCH v2] libperf tests: Fix test_stat_cpu Shunsuke Nakamura @ 2021-10-07 17:05 ` Jiri Olsa 2021-10-08 18:53 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 4+ messages in thread From: Jiri Olsa @ 2021-10-07 17:05 UTC (permalink / raw) To: Shunsuke Nakamura Cc: peterz, mingo, acme, mark.rutland, alexander.shishkin, namhyung, linux-kernel, linux-perf-users On Wed, Oct 06, 2021 at 06:48:17PM +0900, Shunsuke Nakamura wrote: > `cpu` of perf_evsel__read() must be specified the cpu index. > perf_cpu_map__for_each_cpu is for iterating the cpu number (not index) > and is not appropriate. > So, if there is an offline CPU, the cpu number specified in the argument > may point out of range because the cpu number and the cpu index are > different. nice catch > > Fix test_stat_cpu. > > Committer testing: > > # make tests -C tools/lib/perf/ > make: Entering directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' > running static: > - running tests/test-cpumap.c...OK > - running tests/test-threadmap.c...OK > - running tests/test-evlist.c...OK > - running tests/test-evsel.c...OK > running dynamic: > - running tests/test-cpumap.c...OK > - running tests/test-threadmap.c...OK > - running tests/test-evlist.c...OK > - running tests/test-evsel.c...OK > make: Leaving directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' > > > Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com> > --- > Previous version at: > https://lore.kernel.org/lkml/20211006080456.474273-1-nakamura.shun@fujitsu.com/ > > Changes in v2: > - Remove "2/2" from Patch Subject > > tools/lib/perf/tests/test-evlist.c | 6 +++--- > tools/lib/perf/tests/test-evsel.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c > index c67c83399170..47badd7eabf2 100644 > --- a/tools/lib/perf/tests/test-evlist.c > +++ b/tools/lib/perf/tests/test-evlist.c > @@ -40,7 +40,7 @@ static int test_stat_cpu(void) > .type = PERF_TYPE_SOFTWARE, > .config = PERF_COUNT_SW_TASK_CLOCK, > }; > - int err, cpu, tmp; > + int err, idx; > > cpus = perf_cpu_map__new(NULL); > __T("failed to create cpus", cpus); > @@ -70,10 +70,10 @@ static int test_stat_cpu(void) > perf_evlist__for_each_evsel(evlist, evsel) { > cpus = perf_evsel__cpus(evsel); > > - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { > + for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { s/,/;/ ^ tests/test-evlist.c: In function ‘test_stat_cpu’: tests/test-evlist.c:73:52: error: expected ‘;’ before ‘)’ token 73 | for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { | ^ | ; perf_cpu_map__for_each_cpu also returns the cpu index (tmp), maybe we could use that instead? thanks, jirka > struct perf_counts_values counts = { .val = 0 }; > > - perf_evsel__read(evsel, cpu, 0, &counts); > + perf_evsel__read(evsel, idx, 0, &counts); > __T("failed to read value for evsel", counts.val != 0); > } > } > diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c > index 9abd4c0bf6db..33ae9334861a 100644 > --- a/tools/lib/perf/tests/test-evsel.c > +++ b/tools/lib/perf/tests/test-evsel.c > @@ -22,7 +22,7 @@ static int test_stat_cpu(void) > .type = PERF_TYPE_SOFTWARE, > .config = PERF_COUNT_SW_CPU_CLOCK, > }; > - int err, cpu, tmp; > + int err, idx; > > cpus = perf_cpu_map__new(NULL); > __T("failed to create cpus", cpus); > @@ -33,10 +33,10 @@ static int test_stat_cpu(void) > err = perf_evsel__open(evsel, cpus, NULL); > __T("failed to open evsel", err == 0); > > - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { > + for (idx = 0; idx < perf_cpu_map__nr(cpus); idx++) { > struct perf_counts_values counts = { .val = 0 }; > > - perf_evsel__read(evsel, cpu, 0, &counts); > + perf_evsel__read(evsel, idx, 0, &counts); > __T("failed to read value for evsel", counts.val != 0); > } > > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libperf tests: Fix test_stat_cpu 2021-10-07 17:05 ` Jiri Olsa @ 2021-10-08 18:53 ` Arnaldo Carvalho de Melo 2021-10-11 8:11 ` nakamura.shun 0 siblings, 1 reply; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-10-08 18:53 UTC (permalink / raw) To: Jiri Olsa Cc: Shunsuke Nakamura, peterz, mingo, mark.rutland, alexander.shishkin, namhyung, linux-kernel, linux-perf-users Em Thu, Oct 07, 2021 at 07:05:58PM +0200, Jiri Olsa escreveu: > On Wed, Oct 06, 2021 at 06:48:17PM +0900, Shunsuke Nakamura wrote: > > `cpu` of perf_evsel__read() must be specified the cpu index. > > perf_cpu_map__for_each_cpu is for iterating the cpu number (not index) > > and is not appropriate. > > So, if there is an offline CPU, the cpu number specified in the argument > > may point out of range because the cpu number and the cpu index are > > different. > > nice catch Indeed, Nakamura-san, please address Jiri's comment and resubmit, Thanks, - Arnaldo > > > > Fix test_stat_cpu. > > > > Committer testing: > > > > # make tests -C tools/lib/perf/ > > make: Entering directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' > > running static: > > - running tests/test-cpumap.c...OK > > - running tests/test-threadmap.c...OK > > - running tests/test-evlist.c...OK > > - running tests/test-evsel.c...OK > > running dynamic: > > - running tests/test-cpumap.c...OK > > - running tests/test-threadmap.c...OK > > - running tests/test-evlist.c...OK > > - running tests/test-evsel.c...OK > > make: Leaving directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' > > > > > > Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com> > > --- > > Previous version at: > > https://lore.kernel.org/lkml/20211006080456.474273-1-nakamura.shun@fujitsu.com/ > > > > Changes in v2: > > - Remove "2/2" from Patch Subject > > > > tools/lib/perf/tests/test-evlist.c | 6 +++--- > > tools/lib/perf/tests/test-evsel.c | 6 +++--- > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c > > index c67c83399170..47badd7eabf2 100644 > > --- a/tools/lib/perf/tests/test-evlist.c > > +++ b/tools/lib/perf/tests/test-evlist.c > > @@ -40,7 +40,7 @@ static int test_stat_cpu(void) > > .type = PERF_TYPE_SOFTWARE, > > .config = PERF_COUNT_SW_TASK_CLOCK, > > }; > > - int err, cpu, tmp; > > + int err, idx; > > > > cpus = perf_cpu_map__new(NULL); > > __T("failed to create cpus", cpus); > > @@ -70,10 +70,10 @@ static int test_stat_cpu(void) > > perf_evlist__for_each_evsel(evlist, evsel) { > > cpus = perf_evsel__cpus(evsel); > > > > - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { > > + for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { > > s/,/;/ ^ > > tests/test-evlist.c: In function ‘test_stat_cpu’: > tests/test-evlist.c:73:52: error: expected ‘;’ before ‘)’ token > 73 | for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { > | ^ > | ; > > > perf_cpu_map__for_each_cpu also returns the cpu index (tmp), > maybe we could use that instead? > > thanks, > jirka > > > struct perf_counts_values counts = { .val = 0 }; > > > > - perf_evsel__read(evsel, cpu, 0, &counts); > > + perf_evsel__read(evsel, idx, 0, &counts); > > __T("failed to read value for evsel", counts.val != 0); > > } > > } > > diff --git a/tools/lib/perf/tests/test-evsel.c b/tools/lib/perf/tests/test-evsel.c > > index 9abd4c0bf6db..33ae9334861a 100644 > > --- a/tools/lib/perf/tests/test-evsel.c > > +++ b/tools/lib/perf/tests/test-evsel.c > > @@ -22,7 +22,7 @@ static int test_stat_cpu(void) > > .type = PERF_TYPE_SOFTWARE, > > .config = PERF_COUNT_SW_CPU_CLOCK, > > }; > > - int err, cpu, tmp; > > + int err, idx; > > > > cpus = perf_cpu_map__new(NULL); > > __T("failed to create cpus", cpus); > > @@ -33,10 +33,10 @@ static int test_stat_cpu(void) > > err = perf_evsel__open(evsel, cpus, NULL); > > __T("failed to open evsel", err == 0); > > > > - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { > > + for (idx = 0; idx < perf_cpu_map__nr(cpus); idx++) { > > struct perf_counts_values counts = { .val = 0 }; > > > > - perf_evsel__read(evsel, cpu, 0, &counts); > > + perf_evsel__read(evsel, idx, 0, &counts); > > __T("failed to read value for evsel", counts.val != 0); > > } > > > > -- > > 2.25.1 > > -- - Arnaldo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] libperf tests: Fix test_stat_cpu 2021-10-08 18:53 ` Arnaldo Carvalho de Melo @ 2021-10-11 8:11 ` nakamura.shun 0 siblings, 0 replies; 4+ messages in thread From: nakamura.shun @ 2021-10-11 8:11 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Jiri Olsa Cc: peterz, mingo, mark.rutland, alexander.shishkin, namhyung, linux-kernel, linux-perf-users Hi Arnaldo, jirka > Em Thu, Oct 07, 2021 at 07:05:58PM +0200, Jiri Olsa escreveu: > > On Wed, Oct 06, 2021 at 06:48:17PM +0900, Shunsuke Nakamura wrote: > > > `cpu` of perf_evsel__read() must be specified the cpu index. > > > perf_cpu_map__for_each_cpu is for iterating the cpu number (not index) > > > and is not appropriate. > > > So, if there is an offline CPU, the cpu number specified in the argument > > > may point out of range because the cpu number and the cpu index are > > > different. > > > > nice catch > > Indeed, Nakamura-san, please address Jiri's comment and resubmit, I'll send it right away. > > Thanks, > > - Arnaldo > > > > > > > Fix test_stat_cpu. > > > > > > Committer testing: > > > > > > # make tests -C tools/lib/perf/ > > > make: Entering directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' > > > running static: > > > - running tests/test-cpumap.c...OK > > > - running tests/test-threadmap.c...OK > > > - running tests/test-evlist.c...OK > > > - running tests/test-evsel.c...OK > > > running dynamic: > > > - running tests/test-cpumap.c...OK > > > - running tests/test-threadmap.c...OK > > > - running tests/test-evlist.c...OK > > > - running tests/test-evsel.c...OK > > > make: Leaving directory '/home/nakamura/kernel_src/linux-5.15-rc4_fix/tools/lib/perf' > > > > > > > > > Signed-off-by: Shunsuke Nakamura <nakamura.shun@fujitsu.com> > > > --- > > > Previous version at: > > > https://lore.kernel.org/lkml/20211006080456.474273-1-nakamura.shun@fujitsu.com/ > > > > > > Changes in v2: > > > - Remove "2/2" from Patch Subject > > > > > > tools/lib/perf/tests/test-evlist.c | 6 +++--- > > > tools/lib/perf/tests/test-evsel.c | 6 +++--- > > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c > > > index c67c83399170..47badd7eabf2 100644 > > > --- a/tools/lib/perf/tests/test-evlist.c > > > +++ b/tools/lib/perf/tests/test-evlist.c > > > @@ -40,7 +40,7 @@ static int test_stat_cpu(void) > > > .type = PERF_TYPE_SOFTWARE, > > > .config = PERF_COUNT_SW_TASK_CLOCK, > > > }; > > > - int err, cpu, tmp; > > > + int err, idx; > > > > > > cpus = perf_cpu_map__new(NULL); > > > __T("failed to create cpus", cpus); > > > @@ -70,10 +70,10 @@ static int test_stat_cpu(void) > > > perf_evlist__for_each_evsel(evlist, evsel) { > > > cpus = perf_evsel__cpus(evsel); > > > > > > - perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { > > > + for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { > > > > s/,/;/ ^ > > > > tests/test-evlist.c: In function 'test_stat_cpu': > > tests/test-evlist.c:73:52: error: expected ';' before ')' token > > 73 | for (idx = 0, idx < perf_cpu_map__nr(cpus); idx++) { > > | ^ > > | ; Thanks for review. I will fix it. > > > > > > perf_cpu_map__for_each_cpu also returns the cpu index (tmp), > > maybe we could use that instead? Using only cpu index results in a build error. $ git diff diff --git a/tools/lib/perf/tests/test-evlist.c b/tools/lib/perf/tests/test-evlist.c index c67c83399170..b36aa8afe95c 100644 --- a/tools/lib/perf/tests/test-evlist.c +++ b/tools/lib/perf/tests/test-evlist.c @@ -73,7 +73,7 @@ static int test_stat_cpu(void) perf_cpu_map__for_each_cpu(cpu, tmp, cpus) { struct perf_counts_values counts = { .val = 0 }; - perf_evsel__read(evsel, cpu, 0, &counts); + perf_evsel__read(evsel, tmp, 0, &counts); __T("failed to read value for evsel", counts.val != 0); } } $ make tests -C tools/lib/perf/ make: Entering directory '/ubinux-dev/common/home/nakamura/kernel_Build/linux/tools/lib/perf' CC tests/test-evlist.o tests/test-evlist.c: In function 'test_stat_cpu': tests/test-evlist.c:43:11: error: variable 'cpu' set but not used [-Werror=unused-but-set-variable] 43 | int err, cpu, tmp; | Best Regards Shunsuke ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-11 8:19 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-06 9:48 [PATCH v2] libperf tests: Fix test_stat_cpu Shunsuke Nakamura 2021-10-07 17:05 ` Jiri Olsa 2021-10-08 18:53 ` Arnaldo Carvalho de Melo 2021-10-11 8:11 ` nakamura.shun
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.