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

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