Em Tue, May 19, 2020 at 01:15:41PM -0700, Ian Rogers escreveu: > On Tue, May 19, 2020 at 12:06 PM Arnaldo Carvalho de Melo > <arnaldo.melo@xxxxxxxxx> wrote: > > > > Em Fri, May 15, 2020 at 03:17:31PM -0700, Ian Rogers escreveu: > > > Break pmu-events test into 2 and add a test to verify that all pmu > > > metric expressions simply parse. Try to parse all metric ids/events, > > > skip/warn if metrics for the current architecture fail to parse. To > > > support warning for a skip, and an ability for a subtest to describe why > > > it skips. > > > > > > Tested on power9, skylakex, haswell, broadwell, westmere, sandybridge and > > > ivybridge. > > > > > > May skip/warn on other architectures if metrics are invalid. In > > > particular s390 is untested, but its expressions are trivial. The > > > untested architectures with expressions are power8, cascadelakex, > > > tremontx, skylake, jaketown, ivytown and variants of haswell and > > > broadwell. > > > > > > v3. addresses review comments from John Garry <john.garry@xxxxxxxxxx>, > > > Jiri Olsa <jolsa@xxxxxxxxxx> and Arnaldo Carvalho de Melo > > > <acme@xxxxxxxxxx>. > > > v2. changes the commit message as event parsing errors no longer cause > > > the test to fail. > > > > > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx> > > > Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx> > > > Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx> > > > Cc: Andi Kleen <ak@xxxxxxxxxxxxxxx> > > > Cc: Jin Yao <yao.jin@xxxxxxxxxxxxxxx> > > > Cc: Jiri Olsa <jolsa@xxxxxxxxxx> > > > Cc: John Garry <john.garry@xxxxxxxxxx> > > > Cc: Kajol Jain <kjain@xxxxxxxxxxxxx> > > > Cc: Kan Liang <kan.liang@xxxxxxxxxxxxxxx> > > > Cc: Leo Yan <leo.yan@xxxxxxxxxx> > > > Cc: Mark Rutland <mark.rutland@xxxxxxx> > > > Cc: Namhyung Kim <namhyung@xxxxxxxxxx> > > > Cc: Paul Clarke <pc@xxxxxxxxxx> > > > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > > > Cc: Stephane Eranian <eranian@xxxxxxxxxx> > > > Link: http://lore.kernel.org/lkml/20200513212933.41273-1-irogers@xxxxxxxxxx > > > [ split from a larger patch ] > > > Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> > > > --- > > > tools/perf/tests/builtin-test.c | 7 ++ > > > tools/perf/tests/pmu-events.c | 168 ++++++++++++++++++++++++++++++-- > > > tools/perf/tests/tests.h | 3 + > > > 3 files changed, 172 insertions(+), 6 deletions(-) > > > > > > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c > > > index baee735e6aa5..9553f8061772 100644 > > > --- a/tools/perf/tests/builtin-test.c > > > +++ b/tools/perf/tests/builtin-test.c > > > @@ -75,6 +75,13 @@ static struct test generic_tests[] = { > > > { > > > .desc = "PMU events", > > > .func = test__pmu_events, > > > + .subtest = { > > > + .skip_if_fail = false, > > > + .get_nr = test__pmu_events_subtest_get_nr, > > > + .get_desc = test__pmu_events_subtest_get_desc, > > > + .skip_reason = test__pmu_events_subtest_skip_reason, > > > + }, > > > + > > > }, > > > { > > > .desc = "DSO data read", > > > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c > > > index d64261da8bf7..e21f0addcfbb 100644 > > > --- a/tools/perf/tests/pmu-events.c > > > +++ b/tools/perf/tests/pmu-events.c > > > @@ -8,6 +8,9 @@ > > > #include <linux/zalloc.h> > > > #include "debug.h" > > > #include "../pmu-events/pmu-events.h" > > > +#include "util/evlist.h" > > > +#include "util/expr.h" > > > +#include "util/parse-events.h" > > > > > > struct perf_pmu_test_event { > > > struct pmu_event event; > > > @@ -144,7 +147,7 @@ static struct pmu_events_map *__test_pmu_get_events_map(void) > > > } > > > > > > /* Verify generated events from pmu-events.c is as expected */ > > > -static int __test_pmu_event_table(void) > > > +static int test_pmu_event_table(void) > > > { > > > struct pmu_events_map *map = __test_pmu_get_events_map(); > > > struct pmu_event *table; > > > @@ -347,14 +350,11 @@ static int __test__pmu_event_aliases(char *pmu_name, int *count) > > > return res; > > > } > > > > > > -int test__pmu_events(struct test *test __maybe_unused, > > > - int subtest __maybe_unused) > > > + > > > +static int test_aliases(void) > > > { > > > struct perf_pmu *pmu = NULL; > > > > > > - if (__test_pmu_event_table()) > > > - return -1; > > > - > > > while ((pmu = perf_pmu__scan(pmu)) != NULL) { > > > int count = 0; > > > > > > @@ -377,3 +377,159 @@ int test__pmu_events(struct test *test __maybe_unused, > > > > > > return 0; > > > } > > > + > > > +static bool is_number(const char *str) > > > +{ > > > + char *end_ptr; > > > + > > > + strtod(str, &end_ptr); > > > + return end_ptr != str; > > > +} > > > > So, this breaks in some systems: > > > > cc1: warnings being treated as errors > > tests/pmu-events.c: In function 'is_number': > > tests/pmu-events.c:385: error: ignoring return value of 'strtod', declared with attribute warn_unused_result > > mv: cannot stat `/tmp/build/perf/tests/.pmu-events.o.tmp': No such file or director > > > > So I'm changing it to verify the result of strtod() which is, humm, > > interesting, please check: > > Thanks Arnaldo and sorry for the difficulty. This looks like a good fix. > > > diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c > > index 3de59564deb0..6c58c3a89e6b 100644 > > --- a/tools/perf/tests/pmu-events.c > > +++ b/tools/perf/tests/pmu-events.c > > @@ -1,4 +1,5 @@ > > // SPDX-License-Identifier: GPL-2.0 > > +#include "math.h" > > #include "parse-events.h" > > #include "pmu.h" > > #include "tests.h" > > @@ -381,8 +382,12 @@ static int test_aliases(void) > > static bool is_number(const char *str) > > { > > char *end_ptr; > > + double v; > > > > - strtod(str, &end_ptr); > > + errno = 0; > > + v = strtod(str, &end_ptr); > > + if ((errno == ERANGE && (v == HUGE_VAL || v == -HUGE_VAL)) || (errno != 0 && v == 0.0)) > > errno can either be 0 or ERANGE here, but we test both. Perhaps use > errno != 0 for both cases as the man page notes suggest doing this. > The tests using v are necessary to avoid the unused result, but > presumably any errno case should return false here? I guess testing > that is redundant as the return below will catch it. Perhaps this > should be: > > errno = 0; > v = strtod(str, &end_ptr); > (void)v; /* We don't care for the value of the double, just that it > converts. Avoid unused result warnings. */ > return errno == 0 && end_ptr != str; Ok, I'll try that one. - Arnaldo > Thanks, > Ian > > > + return false; > > return end_ptr != str; > > } > > -- - Arnaldo
