Re: [PATCH v2 2/2] perf tests: Test for PMU alias

[Jin, Yao]

Hi Riccardo,

On 7/29/2021 9:15 PM, Riccardo Mancini wrote:
> Hi,
>
> On Thu, 2021-07-29 at 15:06 +0800, Jin Yao wrote:
> > A perf uncore PMU may have two PMU names, a real name and an alias
> > name. Add one test case to verify the real name and alias name having
> > the same effect.
> >
> > Iterate the sysfs to get one event which has an alias and create
> > evlist by adding two evsels. Evsel1 is created by event and evsel2
> > is created by alias.
> >
> > Test asserts:
> > evsel1->core.attr.type == evsel2->core.attr.type
> > evsel1->core.attr.config == evsel2->core.attr.config
> >
> > Signed-off-by: Jin Yao <yao.jin@xxxxxxxxxxxxxxx>
> > ---
> > v2:
> >   - New in v2.
> >
> >   tools/perf/tests/parse-events.c | 79 +++++++++++++++++++++++++++++++++
> >   1 file changed, 79 insertions(+)
> >
> > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> > index 56a7b6a14195..b416851e4074 100644
> > --- a/tools/perf/tests/parse-events.c
> > +++ b/tools/perf/tests/parse-events.c
> > @@ -6,6 +6,7 @@
> >   #include "tests.h"
> >   #include "debug.h"
> >   #include "pmu.h"
> > +#include "fncache.h"
> >   #include <dirent.h>
> >   #include <errno.h>
> >   #include <sys/types.h>
> > @@ -2190,9 +2191,79 @@ static int test_pmu_events(void)
> >          return ret;
> >   }
> >   
> > +static bool test_alias(char **event, char **alias)
> > +{
> > +       char path[PATH_MAX];
> > +       DIR *dir;
> > +       struct dirent *dent;
> > +       const char *sysfs = sysfs__mountpoint();
> > +       char buf[128];
> > +       FILE *file;
> > +
> > +       if (!sysfs)
> > +               return false;
> > +
> > +       snprintf(path, PATH_MAX, "%s/bus/event_source/devices/", sysfs);
> > +       dir = opendir(path);
> > +       if (!dir)
> > +               return false;
> > +
> > +       while ((dent = readdir(dir))) {
> > +               if (!strcmp(dent->d_name, ".") ||
> > +                   !strcmp(dent->d_name, ".."))
> > +                       continue;
> > +
> > +               snprintf(path, PATH_MAX,
> > "%s/bus/event_source/devices/%s/alias",
> > +                        sysfs, dent->d_name);
> > +
> > +               if (!file_available(path))
> > +                       continue;
> > +
> > +               file = fopen(path, "r");
> > +               if (!file)
> > +                       continue;
> > +
> > +               if (fscanf(file, "%s", buf) != 1) {
> > +                       fclose(file);
> > +                       continue;
> > +               }
>
> ditto as in the first patch.

Got it, thanks!

> > +
> > +               fclose(file);
> > +               *event = strdup(dent->d_name);
> > +               *alias = strdup(buf);
> > +               return true;
> > +       }
>
> dir is never closed.

Oh yes, should add "closedir(dir);" at the end of function.

Thanks
Jin Yao

> Thanks,
> Riccardo
>
> > +
> > +       return false;
> > +}
> > +
> > +static int test__checkevent_pmu_events_alias(struct evlist *evlist)
> > +{
> > +       struct evsel *evsel1 = evlist__first(evlist);
> > +       struct evsel *evsel2 = evlist__last(evlist);
> > +
> > +       TEST_ASSERT_VAL("wrong type", evsel1->core.attr.type == evsel2-
> > > core.attr.type);
> > +       TEST_ASSERT_VAL("wrong config", evsel1->core.attr.config == evsel2-
> > > core.attr.config);
> > +       return 0;
> > +}
> > +
> > +static int test_pmu_events_alias(char *event, char *alias)
> > +{
> > +       struct evlist_test e = { .id = 0, };
> > +       char name[2 * NAME_MAX + 20];
> > +
> > +       snprintf(name, sizeof(name), "%s/event=1/,%s/event=1/",
> > +                event, alias);
> > +
> > +       e.name  = name;
> > +       e.check = test__checkevent_pmu_events_alias;
> > +       return test_event(&e);
> > +}
> > +
> >   int test__parse_events(struct test *test __maybe_unused, int subtest
> > __maybe_unused)
> >   {
> >          int ret1, ret2 = 0;
> > +       char *event, *alias;
> >   
> >   #define TEST_EVENTS(tests)                             \
> >   do {                                                   \
> > @@ -2217,6 +2288,14 @@ do
> > {                                                     \
> >                          return ret;
> >          }
> >   
> > +       if (test_alias(&event, &alias)) {
> > +               int ret = test_pmu_events_alias(event, alias);
> > +               free(event);
> > +               free(alias);
> > +               if (ret)
> > +                       return ret;
> > +       }
> > +
> >          ret1 = test_terms(test__terms, ARRAY_SIZE(test__terms));
> >          if (!ret2)
> >                  ret2 = ret1;