lkml.org 
[lkml]   [2021]   [Oct]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] libperf tests: Fix test_stat_cpu
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

\
 
 \ /
  Last update: 2021-10-08 20:55    [W:0.099 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site