Re: [PATCH 4/4] perf test: Add expand cgroup event test
On Fri, Sep 18, 2020 at 10:52 PM Jiri Olsa wrote: > > On Wed, Sep 16, 2020 at 03:31:29PM +0900, Namhyung Kim wrote: > > SNIP > > > +++ b/tools/perf/tests/tests.h > > @@ -123,6 +123,7 @@ const char *test__pfm_subtest_get_desc(int subtest); > > int test__pfm_subtest_get_nr(void); > > int test__parse_metric(struct test *test, int subtest); > > int test__pe_file_parsing(struct test *test, int subtest); > > +int test__expand_cgroup_events(struct test *test, int subtest); > > > > bool test__bp_signal_is_supported(void); > > bool test__bp_account_is_supported(void); > > diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c > > index a1bf345a770b..eeffa08251b5 100644 > > --- a/tools/perf/util/cgroup.c > > +++ b/tools/perf/util/cgroup.c > > @@ -52,7 +52,7 @@ static struct cgroup *evlist__find_cgroup(struct evlist > > *evlist, const char *str > > return NULL; > > } > > > could you please put the do_open change below into separate patch? Ok, will do. Thanks Namhyung > > > > -static struct cgroup *cgroup__new(const char *name) > > +static struct cgroup *cgroup__new(const char *name, bool do_open) > > { > > struct cgroup *cgroup = zalloc(sizeof(*cgroup)); > > > > @@ -62,9 +62,14 @@ static struct cgroup *cgroup__new(const char *name) > > cgroup->name = strdup(name); > > if (!cgroup->name) > > goto out_err; > > - cgroup->fd = open_cgroup(name); > > - if (cgroup->fd == -1) > > - goto out_free_name; > > + > > + if (do_open) { > > + cgroup->fd = open_cgroup(name); > > + if (cgroup->fd == -1) > > + goto out_free_name; > > + } else { > > + cgroup->fd = -1; > > + } > > } > > > > return cgroup; > > @@ -80,7 +85,7 @@ struct cgroup *evlist__findnew_cgroup(struct evlist > > *evlist, const char *name) > > { > > struct cgroup *cgroup = evlist__find_cgroup(evlist, name); > > > > - return cgroup ?: cgroup__new(name); > > + return cgroup ?: cgroup__new(name, true); > > } > > > > static int add_cgroup(struct evlist *evlist, const char *str) > > @@ -202,7 +207,7 @@ int parse_cgroups(const struct option *opt, const char > > *str, > > } > > > > int evlist__expand_cgroup(struct evlist *evlist, const char *str, > > - struct rblist *metric_events) > > + struct rblist *metric_events, bool open_cgroup) > > { > > struct evlist *orig_list, *tmp_list; > > struct evsel *pos, *evsel, *leader; > > @@ -240,7 +245,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const > > char *str, > > if (!name) > > break; > > > > - cgrp = cgroup__new(name); > > + cgrp = cgroup__new(name, open_cgroup); > > free(name); > > if (cgrp == NULL) > > break; > > diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h > > index eea6df8ee373..162906f3412a 100644 > > --- a/tools/perf/util/cgroup.h > > +++ b/tools/perf/util/cgroup.h > > @@ -26,7 +26,7 @@ struct rblist; > > > > struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char > > *name); > > int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups, > > - struct rblist *metric_events); > > + struct rblist *metric_events, bool open_cgroup); > > > > void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup > > *cgroup); > > > > -- > > 2.28.0.618.gf4bc123cb7-goog > > >
Re: [PATCH 4/4] perf test: Add expand cgroup event test
On Wed, Sep 16, 2020 at 03:31:29PM +0900, Namhyung Kim wrote: SNIP > +++ b/tools/perf/tests/tests.h > @@ -123,6 +123,7 @@ const char *test__pfm_subtest_get_desc(int subtest); > int test__pfm_subtest_get_nr(void); > int test__parse_metric(struct test *test, int subtest); > int test__pe_file_parsing(struct test *test, int subtest); > +int test__expand_cgroup_events(struct test *test, int subtest); > > bool test__bp_signal_is_supported(void); > bool test__bp_account_is_supported(void); > diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c > index a1bf345a770b..eeffa08251b5 100644 > --- a/tools/perf/util/cgroup.c > +++ b/tools/perf/util/cgroup.c > @@ -52,7 +52,7 @@ static struct cgroup *evlist__find_cgroup(struct evlist > *evlist, const char *str > return NULL; > } could you please put the do_open change below into separate patch? thanks, jirka > > -static struct cgroup *cgroup__new(const char *name) > +static struct cgroup *cgroup__new(const char *name, bool do_open) > { > struct cgroup *cgroup = zalloc(sizeof(*cgroup)); > > @@ -62,9 +62,14 @@ static struct cgroup *cgroup__new(const char *name) > cgroup->name = strdup(name); > if (!cgroup->name) > goto out_err; > - cgroup->fd = open_cgroup(name); > - if (cgroup->fd == -1) > - goto out_free_name; > + > + if (do_open) { > + cgroup->fd = open_cgroup(name); > + if (cgroup->fd == -1) > + goto out_free_name; > + } else { > + cgroup->fd = -1; > + } > } > > return cgroup; > @@ -80,7 +85,7 @@ struct cgroup *evlist__findnew_cgroup(struct evlist > *evlist, const char *name) > { > struct cgroup *cgroup = evlist__find_cgroup(evlist, name); > > - return cgroup ?: cgroup__new(name); > + return cgroup ?: cgroup__new(name, true); > } > > static int add_cgroup(struct evlist *evlist, const char *str) > @@ -202,7 +207,7 @@ int parse_cgroups(const struct option *opt, const char > *str, > } > > int evlist__expand_cgroup(struct evlist *evlist, const char *str, > - struct rblist *metric_events) > + struct rblist *metric_events, bool open_cgroup) > { > struct evlist *orig_list, *tmp_list; > struct evsel *pos, *evsel, *leader; > @@ -240,7 +245,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const > char *str, > if (!name) > break; > > - cgrp = cgroup__new(name); > + cgrp = cgroup__new(name, open_cgroup); > free(name); > if (cgrp == NULL) > break; > diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h > index eea6df8ee373..162906f3412a 100644 > --- a/tools/perf/util/cgroup.h > +++ b/tools/perf/util/cgroup.h > @@ -26,7 +26,7 @@ struct rblist; > > struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char > *name); > int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups, > - struct rblist *metric_events); > + struct rblist *metric_events, bool open_cgroup); > > void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup > *cgroup); > > -- > 2.28.0.618.gf4bc123cb7-goog >
[PATCH 4/4] perf test: Add expand cgroup event test
It'll expand given events for cgroups A, B and C. $ ./perf test -v expansion 69: Event expansion for cgroups : --- start --- test child forked, pid 983140 metric expr 1 / IPC for CPI metric expr instructions / cycles for IPC found event instructions found event cycles adding {instructions,cycles}:W copying metric event for cgroup 'A': instructions (idx=0) copying metric event for cgroup 'B': instructions (idx=0) copying metric event for cgroup 'C': instructions (idx=0) test child finished with 0 end Event expansion for cgroups: Ok Cc: John Garry Signed-off-by: Namhyung Kim --- tools/perf/builtin-stat.c| 2 +- tools/perf/tests/Build | 1 + tools/perf/tests/builtin-test.c | 4 + tools/perf/tests/expand-cgroup.c | 241 +++ tools/perf/tests/tests.h | 1 + tools/perf/util/cgroup.c | 19 ++- tools/perf/util/cgroup.h | 2 +- 7 files changed, 261 insertions(+), 9 deletions(-) create mode 100644 tools/perf/tests/expand-cgroup.c diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index 8b81d62ab18b..f00600d9903e 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -2249,7 +2249,7 @@ int cmd_stat(int argc, const char **argv) if (stat_config.cgroup_list) { if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list, - _config.metric_events) < 0) + _config.metric_events, true) < 0) goto out; } diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build index 69bea7996f18..4d15bf6041fb 100644 --- a/tools/perf/tests/Build +++ b/tools/perf/tests/Build @@ -61,6 +61,7 @@ perf-y += demangle-java-test.o perf-y += pfm.o perf-y += parse-metric.o perf-y += pe-file-parsing.o +perf-y += expand-cgroup.o $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build $(call rule_mkdir) diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c index 651b8ea3354a..132bdb3e6c31 100644 --- a/tools/perf/tests/builtin-test.c +++ b/tools/perf/tests/builtin-test.c @@ -345,6 +345,10 @@ static struct test generic_tests[] = { .desc = "PE file support", .func = test__pe_file_parsing, }, + { + .desc = "Event expansion for cgroups", + .func = test__expand_cgroup_events, + }, { .func = NULL, }, diff --git a/tools/perf/tests/expand-cgroup.c b/tools/perf/tests/expand-cgroup.c new file mode 100644 index ..d5771e4d094f --- /dev/null +++ b/tools/perf/tests/expand-cgroup.c @@ -0,0 +1,241 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "tests.h" +#include "debug.h" +#include "evlist.h" +#include "cgroup.h" +#include "rblist.h" +#include "metricgroup.h" +#include "parse-events.h" +#include "pmu-events/pmu-events.h" +#include "pfm.h" +#include +#include +#include +#include + +static int test_expand_events(struct evlist *evlist, + struct rblist *metric_events) +{ + int i, ret = TEST_FAIL; + int nr_events; + bool was_group_event; + int nr_members; /* for the first evsel only */ + const char cgrp_str[] = "A,B,C"; + const char *cgrp_name[] = { "A", "B", "C" }; + int nr_cgrps = ARRAY_SIZE(cgrp_name); + char **ev_name; + struct evsel *evsel; + + TEST_ASSERT_VAL("evlist is empty", !perf_evlist__empty(evlist)); + + nr_events = evlist->core.nr_entries; + ev_name = calloc(nr_events, sizeof(*ev_name)); + if (ev_name == NULL) { + pr_debug("memory allocation failure\n"); + return TEST_FAIL; + } + i = 0; + evlist__for_each_entry(evlist, evsel) { + ev_name[i] = strdup(evsel->name); + if (ev_name[i] == NULL) { + pr_debug("memory allocation failure\n"); + goto out; + } + i++; + } + /* remember grouping info */ + was_group_event = evsel__is_group_event(evlist__first(evlist)); + nr_members = evlist__first(evlist)->core.nr_members; + + ret = evlist__expand_cgroup(evlist, cgrp_str, metric_events, false); + if (ret < 0) { + pr_debug("failed to expand events for cgroups\n"); + goto out; + } + + ret = TEST_FAIL; + if (evlist->core.nr_entries != nr_events * nr_cgrps) { + pr_debug("event count doesn't match\n"); + goto out; + } + + i = 0; + evlist__for_each_entry(evlist, evsel) { + if (strcmp(evsel->name, ev_name[i % nr_events])) { + pr_debug("event name doesn't match:\n"); + pr_debug(" evsel[%d]: %s\n expected: %s\n", +i,