* [PATCH 1/5] perf evsel: Add evsel__clone() function
2020-09-21 9:46 [PATCHSET v3 0/5] perf stat: Expand events for each cgroup Namhyung Kim
@ 2020-09-21 9:46 ` Namhyung Kim
2020-09-21 9:46 ` [PATCH 2/5] perf stat: Add --for-each-cgroup option Namhyung Kim
` (3 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-21 9:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers
The evsel__clone() is to create an exactly same evsel from same
attributes. The function assumes the given evsel is not configured
yet so it cares fields set during event parsing. Those fields are now
moved together as Jiri suggested. Note that metric events will be
handled by later patch.
It will be used by perf stat to generate separate events for each
cgroup.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/evsel.c | 104 ++++++++++++++++++++++++++++++++++++++++
tools/perf/util/evsel.h | 93 ++++++++++++++++++++---------------
2 files changed, 158 insertions(+), 39 deletions(-)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index fd865002cbbd..c63dd9f7e9fe 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -331,6 +331,110 @@ struct evsel *evsel__new_cycles(bool precise)
goto out;
}
+static int evsel__copy_config_terms(struct evsel *dst, struct evsel *src)
+{
+ struct evsel_config_term *pos, *tmp;
+
+ list_for_each_entry(pos, &src->config_terms, list) {
+ tmp = malloc(sizeof(*tmp));
+ if (tmp == NULL)
+ return -ENOMEM;
+
+ *tmp = *pos;
+ if (tmp->free_str) {
+ tmp->val.str = strdup(pos->val.str);
+ if (tmp->val.str == NULL) {
+ free(tmp);
+ return -ENOMEM;
+ }
+ }
+ list_add_tail(&tmp->list, &dst->config_terms);
+ }
+ return 0;
+}
+
+/**
+ * evsel__clone - create a new evsel copied from @orig
+ * @orig: original evsel
+ *
+ * The assumption is that @orig is not configured nor opened yet.
+ * So we only care about the attributes that can be set while it's parsed.
+ */
+struct evsel *evsel__clone(struct evsel *orig)
+{
+ struct evsel *evsel;
+
+ BUG_ON(orig->core.fd);
+ BUG_ON(orig->counts);
+ BUG_ON(orig->priv);
+ BUG_ON(orig->per_pkg_mask);
+
+ /* cannot handle BPF objects for now */
+ if (orig->bpf_obj)
+ return NULL;
+
+ evsel = evsel__new(&orig->core.attr);
+ if (evsel == NULL)
+ return NULL;
+
+ evsel->core.cpus = perf_cpu_map__get(orig->core.cpus);
+ evsel->core.own_cpus = perf_cpu_map__get(orig->core.own_cpus);
+ evsel->core.threads = perf_thread_map__get(orig->core.threads);
+ evsel->core.nr_members = orig->core.nr_members;
+ evsel->core.system_wide = orig->core.system_wide;
+
+ if (orig->name) {
+ evsel->name = strdup(orig->name);
+ if (evsel->name == NULL)
+ goto out_err;
+ }
+ if (orig->group_name) {
+ evsel->group_name = strdup(orig->group_name);
+ if (evsel->group_name == NULL)
+ goto out_err;
+ }
+ if (orig->pmu_name) {
+ evsel->pmu_name = strdup(orig->pmu_name);
+ if (evsel->pmu_name == NULL)
+ goto out_err;
+ }
+ if (orig->filter) {
+ evsel->filter = strdup(orig->filter);
+ if (evsel->filter == NULL)
+ goto out_err;
+ }
+ evsel->cgrp = cgroup__get(orig->cgrp);
+ evsel->tp_format = orig->tp_format;
+ evsel->handler = orig->handler;
+ evsel->leader = orig->leader;
+
+ evsel->max_events = orig->max_events;
+ evsel->tool_event = orig->tool_event;
+ evsel->unit = orig->unit;
+ evsel->scale = orig->scale;
+ evsel->snapshot = orig->snapshot;
+ evsel->per_pkg = orig->per_pkg;
+ evsel->percore = orig->percore;
+ evsel->precise_max = orig->precise_max;
+ evsel->use_uncore_alias = orig->use_uncore_alias;
+ evsel->is_libpfm_event = orig->is_libpfm_event;
+
+ evsel->exclude_GH = orig->exclude_GH;
+ evsel->sample_read = orig->sample_read;
+ evsel->auto_merge_stats = orig->auto_merge_stats;
+ evsel->collect_stat = orig->collect_stat;
+ evsel->weak_group = orig->weak_group;
+
+ if (evsel__copy_config_terms(evsel, orig) < 0)
+ goto out_err;
+
+ return evsel;
+
+out_err:
+ evsel__delete(evsel);
+ return NULL;
+}
+
/*
* Returns pointer with encoded error via <linux/err.h> interface.
*/
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 35e3f6d66085..79a860d8e3ee 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -42,65 +42,79 @@ enum perf_tool_event {
*/
struct evsel {
struct perf_evsel core;
- struct evlist *evlist;
- char *filter;
+ struct evlist *evlist;
+ off_t id_offset;
+ int idx;
+ int id_pos;
+ int is_pos;
+ unsigned int sample_size;
+
+ /*
+ * These fields can be set in the parse-events code or similar.
+ * Please check evsel__clone() to copy them properly so that
+ * they can be released properly.
+ */
+ struct {
+ char *name;
+ char *group_name;
+ const char *pmu_name;
+ struct tep_event *tp_format;
+ char *filter;
+ unsigned long max_events;
+ double scale;
+ const char *unit;
+ struct cgroup *cgrp;
+ enum perf_tool_event tool_event;
+ /* parse modifier helper */
+ int exclude_GH;
+ int sample_read;
+ bool snapshot;
+ bool per_pkg;
+ bool percore;
+ bool precise_max;
+ bool use_uncore_alias;
+ bool is_libpfm_event;
+ bool auto_merge_stats;
+ bool collect_stat;
+ bool weak_group;
+ int bpf_fd;
+ struct bpf_object *bpf_obj;
+ };
+
+ /*
+ * metric fields are similar, but needs more care as they can have
+ * references to other metric (evsel).
+ */
+ const char * metric_expr;
+ const char * metric_name;
+ struct evsel **metric_events;
+ struct evsel *metric_leader;
+
+ void *handler;
struct perf_counts *counts;
struct perf_counts *prev_raw_counts;
- int idx;
- unsigned long max_events;
unsigned long nr_events_printed;
- char *name;
- double scale;
- const char *unit;
- struct tep_event *tp_format;
- off_t id_offset;
struct perf_stat_evsel *stats;
void *priv;
u64 db_id;
- struct cgroup *cgrp;
- void *handler;
- unsigned int sample_size;
- int id_pos;
- int is_pos;
- enum perf_tool_event tool_event;
bool uniquified_name;
- bool snapshot;
bool supported;
bool needs_swap;
bool disabled;
bool no_aux_samples;
bool immediate;
bool tracking;
- bool per_pkg;
- bool precise_max;
bool ignore_missing_thread;
bool forced_leader;
- bool use_uncore_alias;
- bool is_libpfm_event;
- /* parse modifier helper */
- int exclude_GH;
- int sample_read;
- unsigned long *per_pkg_mask;
- struct evsel *leader;
- char *group_name;
bool cmdline_group_boundary;
- struct list_head config_terms;
- struct bpf_object *bpf_obj;
- int bpf_fd;
- int err;
- bool auto_merge_stats;
bool merged_stat;
- const char * metric_expr;
- const char * metric_name;
- struct evsel **metric_events;
- struct evsel *metric_leader;
- bool collect_stat;
- bool weak_group;
bool reset_group;
bool errored;
- bool percore;
+ unsigned long *per_pkg_mask;
+ struct evsel *leader;
+ struct list_head config_terms;
+ int err;
int cpu_iter;
- const char *pmu_name;
struct {
evsel__sb_cb_t *cb;
void *data;
@@ -169,6 +183,7 @@ static inline struct evsel *evsel__new(struct perf_event_attr *attr)
return evsel__new_idx(attr, 0);
}
+struct evsel *evsel__clone(struct evsel *orig);
struct evsel *evsel__newtp_idx(const char *sys, const char *name, int idx);
/*
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-21 9:46 [PATCHSET v3 0/5] perf stat: Expand events for each cgroup Namhyung Kim
2020-09-21 9:46 ` [PATCH 1/5] perf evsel: Add evsel__clone() function Namhyung Kim
@ 2020-09-21 9:46 ` Namhyung Kim
2020-09-22 21:40 ` Jiri Olsa
` (2 more replies)
2020-09-21 9:46 ` [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups Namhyung Kim
` (2 subsequent siblings)
4 siblings, 3 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-21 9:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers
The --for-each-cgroup option is a syntax sugar to monitor large number
of cgroups easily. Current command line requires to list all the
events and cgroups even if users want to monitor same events for each
cgroup. This patch addresses that usage by copying given events for
each cgroup on user's behalf.
For instance, if they want to monitor 6 events for 200 cgroups each
they should write 1200 event names (with -e) AND 1200 cgroup names
(with -G) on the command line. But with this change, they can just
specify 6 events and 200 cgroups with a new option.
A simpler example below: It wants to measure 3 events for 2 cgroups
('A' and 'B'). The result is that total 6 events are counted like
below.
$ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup A,B sleep 1
Performance counter stats for 'system wide':
988.18 msec cpu-clock A # 0.987 CPUs utilized
3,153,761,702 cycles A # 3.200 GHz (100.00%)
8,067,769,847 instructions A # 2.57 insn per cycle (100.00%)
982.71 msec cpu-clock B # 0.982 CPUs utilized
3,136,093,298 cycles B # 3.182 GHz (99.99%)
8,109,619,327 instructions B # 2.58 insn per cycle (99.99%)
1.001228054 seconds time elapsed
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 20 +++++++++-
tools/perf/util/cgroup.c | 84 +++++++++++++++++++++++++++++++++++++++
tools/perf/util/cgroup.h | 1 +
tools/perf/util/stat.h | 1 +
4 files changed, 105 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7f8d756d9408..a43e58e0a088 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1051,6 +1051,17 @@ static int parse_control_option(const struct option *opt,
return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack, &config->ctl_fd_close);
}
+static int parse_stat_cgroups(const struct option *opt,
+ const char *str, int unset)
+{
+ if (stat_config.cgroup_list) {
+ pr_err("--cgroup and --for-each-cgroup cannot be used together\n");
+ return -1;
+ }
+
+ return parse_cgroups(opt, str, unset);
+}
+
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
@@ -1094,7 +1105,9 @@ static struct option stat_options[] = {
OPT_STRING('x', "field-separator", &stat_config.csv_sep, "separator",
"print counts with custom separator"),
OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
- "monitor event in cgroup name only", parse_cgroups),
+ "monitor event in cgroup name only", parse_stat_cgroups),
+ OPT_STRING(0, "for-each-cgroup", &stat_config.cgroup_list, "name",
+ "expand events for each cgroup"),
OPT_STRING('o', "output", &output_name, "file", "output file name"),
OPT_BOOLEAN(0, "append", &append_file, "append to the output file"),
OPT_INTEGER(0, "log-fd", &output_fd,
@@ -2234,6 +2247,11 @@ int cmd_stat(int argc, const char **argv)
if (add_default_attributes())
goto out;
+ if (stat_config.cgroup_list) {
+ if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list) < 0)
+ goto out;
+ }
+
target__validate(&target);
if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide))
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 050dea9f1e88..e4916ed740ac 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -12,6 +12,7 @@
#include <api/fs/fs.h>
int nr_cgroups;
+bool multiply_cgroup;
static int open_cgroup(const char *name)
{
@@ -156,6 +157,10 @@ int parse_cgroups(const struct option *opt, const char *str,
return -1;
}
+ /* delay processing cgroups after it sees all events */
+ if (multiply_cgroup)
+ return 0;
+
for (;;) {
p = strchr(str, ',');
e = p ? p : eos;
@@ -193,6 +198,85 @@ int parse_cgroups(const struct option *opt, const char *str,
return 0;
}
+int evlist__expand_cgroup(struct evlist *evlist, const char *str)
+{
+ struct evlist *orig_list, *tmp_list;
+ struct evsel *pos, *evsel, *leader;
+ struct cgroup *cgrp = NULL;
+ const char *p, *e, *eos = str + strlen(str);
+ int ret = -1;
+
+ if (evlist->core.nr_entries == 0) {
+ fprintf(stderr, "must define events before cgroups\n");
+ return -EINVAL;
+ }
+
+ orig_list = evlist__new();
+ tmp_list = evlist__new();
+ if (orig_list == NULL || tmp_list == NULL) {
+ fprintf(stderr, "memory allocation failed\n");
+ return -ENOMEM;
+ }
+
+ /* save original events and init evlist */
+ perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
+ evlist->core.nr_entries = 0;
+
+ for (;;) {
+ p = strchr(str, ',');
+ e = p ? p : eos;
+
+ /* allow empty cgroups, i.e., skip */
+ if (e - str) {
+ /* termination added */
+ char *name = strndup(str, e - str);
+ if (!name)
+ goto out_err;
+
+ cgrp = cgroup__new(name);
+ free(name);
+ if (cgrp == NULL)
+ goto out_err;
+ } else {
+ cgrp = NULL;
+ }
+
+ leader = NULL;
+ evlist__for_each_entry(orig_list, pos) {
+ evsel = evsel__clone(pos);
+ if (evsel == NULL)
+ goto out_err;
+
+ cgroup__put(evsel->cgrp);
+ evsel->cgrp = cgroup__get(cgrp);
+
+ if (evsel__is_group_leader(pos))
+ leader = evsel;
+ evsel->leader = leader;
+
+ evlist__add(tmp_list, evsel);
+ }
+ /* cgroup__new() has a refcount, release it here */
+ cgroup__put(cgrp);
+ nr_cgroups++;
+
+ perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
+ tmp_list->core.nr_entries = 0;
+
+ if (!p) {
+ ret = 0;
+ break;
+ }
+ str = p+1;
+ }
+
+out_err:
+ evlist__delete(orig_list);
+ evlist__delete(tmp_list);
+
+ return ret;
+}
+
static struct cgroup *__cgroup__findnew(struct rb_root *root, uint64_t id,
bool create, const char *path)
{
diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
index e98d5975fe55..32893018296f 100644
--- a/tools/perf/util/cgroup.h
+++ b/tools/perf/util/cgroup.h
@@ -24,6 +24,7 @@ void cgroup__put(struct cgroup *cgroup);
struct evlist;
struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
+int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 9911fc6adbfd..7325de5bf2a6 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -137,6 +137,7 @@ struct perf_stat_config {
int ctl_fd;
int ctl_fd_ack;
bool ctl_fd_close;
+ const char *cgroup_list;
};
void perf_stat__set_big_num(int set);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-21 9:46 ` [PATCH 2/5] perf stat: Add --for-each-cgroup option Namhyung Kim
@ 2020-09-22 21:40 ` Jiri Olsa
2020-09-22 22:51 ` Namhyung Kim
2020-09-22 21:52 ` Stephane Eranian
2020-09-22 22:57 ` Namhyung Kim
2 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2020-09-22 21:40 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Andi Kleen, Ian Rogers
On Mon, Sep 21, 2020 at 06:46:07PM +0900, Namhyung Kim wrote:
SNIP
> +int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> +{
> + struct evlist *orig_list, *tmp_list;
> + struct evsel *pos, *evsel, *leader;
> + struct cgroup *cgrp = NULL;
> + const char *p, *e, *eos = str + strlen(str);
> + int ret = -1;
> +
> + if (evlist->core.nr_entries == 0) {
> + fprintf(stderr, "must define events before cgroups\n");
> + return -EINVAL;
> + }
> +
> + orig_list = evlist__new();
> + tmp_list = evlist__new();
> + if (orig_list == NULL || tmp_list == NULL) {
> + fprintf(stderr, "memory allocation failed\n");
> + return -ENOMEM;
> + }
> +
> + /* save original events and init evlist */
> + perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
> + evlist->core.nr_entries = 0;
> +
> + for (;;) {
> + p = strchr(str, ',');
> + e = p ? p : eos;
> +
> + /* allow empty cgroups, i.e., skip */
> + if (e - str) {
> + /* termination added */
> + char *name = strndup(str, e - str);
> + if (!name)
> + goto out_err;
> +
> + cgrp = cgroup__new(name);
> + free(name);
> + if (cgrp == NULL)
> + goto out_err;
> + } else {
> + cgrp = NULL;
> + }
> +
> + leader = NULL;
> + evlist__for_each_entry(orig_list, pos) {
> + evsel = evsel__clone(pos);
> + if (evsel == NULL)
> + goto out_err;
> +
> + cgroup__put(evsel->cgrp);
> + evsel->cgrp = cgroup__get(cgrp);
> +
> + if (evsel__is_group_leader(pos))
> + leader = evsel;
> + evsel->leader = leader;
hum, will this work if there's standalone event after group? like:
{cycles,instructions},cache-misses
cache-misses will get cycles as group leader no?
jirka
> +
> + evlist__add(tmp_list, evsel);
> + }
> + /* cgroup__new() has a refcount, release it here */
> + cgroup__put(cgrp);
> + nr_cgroups++;
SNIP
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-22 21:40 ` Jiri Olsa
@ 2020-09-22 22:51 ` Namhyung Kim
2020-09-23 9:30 ` Jiri Olsa
0 siblings, 1 reply; 16+ messages in thread
From: Namhyung Kim @ 2020-09-22 22:51 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Andi Kleen, Ian Rogers
On Wed, Sep 23, 2020 at 6:40 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Sep 21, 2020 at 06:46:07PM +0900, Namhyung Kim wrote:
>
> SNIP
>
> > +int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> > +{
> > + struct evlist *orig_list, *tmp_list;
> > + struct evsel *pos, *evsel, *leader;
> > + struct cgroup *cgrp = NULL;
> > + const char *p, *e, *eos = str + strlen(str);
> > + int ret = -1;
> > +
> > + if (evlist->core.nr_entries == 0) {
> > + fprintf(stderr, "must define events before cgroups\n");
> > + return -EINVAL;
> > + }
> > +
> > + orig_list = evlist__new();
> > + tmp_list = evlist__new();
> > + if (orig_list == NULL || tmp_list == NULL) {
> > + fprintf(stderr, "memory allocation failed\n");
> > + return -ENOMEM;
> > + }
> > +
> > + /* save original events and init evlist */
> > + perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
> > + evlist->core.nr_entries = 0;
> > +
> > + for (;;) {
> > + p = strchr(str, ',');
> > + e = p ? p : eos;
> > +
> > + /* allow empty cgroups, i.e., skip */
> > + if (e - str) {
> > + /* termination added */
> > + char *name = strndup(str, e - str);
> > + if (!name)
> > + goto out_err;
> > +
> > + cgrp = cgroup__new(name);
> > + free(name);
> > + if (cgrp == NULL)
> > + goto out_err;
> > + } else {
> > + cgrp = NULL;
> > + }
> > +
> > + leader = NULL;
> > + evlist__for_each_entry(orig_list, pos) {
> > + evsel = evsel__clone(pos);
> > + if (evsel == NULL)
> > + goto out_err;
> > +
> > + cgroup__put(evsel->cgrp);
> > + evsel->cgrp = cgroup__get(cgrp);
> > +
> > + if (evsel__is_group_leader(pos))
> > + leader = evsel;
> > + evsel->leader = leader;
>
> hum, will this work if there's standalone event after group? like:
>
> {cycles,instructions},cache-misses
>
> cache-misses will get cycles as group leader no?
AFAIK non-group events are treated as a leader of its own group.
So evsel__is_group_leader() will return true for cache-misses.
Thanks
Namhyung
>
> > +
> > + evlist__add(tmp_list, evsel);
> > + }
> > + /* cgroup__new() has a refcount, release it here */
> > + cgroup__put(cgrp);
> > + nr_cgroups++;
>
> SNIP
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-22 22:51 ` Namhyung Kim
@ 2020-09-23 9:30 ` Jiri Olsa
0 siblings, 0 replies; 16+ messages in thread
From: Jiri Olsa @ 2020-09-23 9:30 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Andi Kleen, Ian Rogers
On Wed, Sep 23, 2020 at 07:51:33AM +0900, Namhyung Kim wrote:
> On Wed, Sep 23, 2020 at 6:40 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > On Mon, Sep 21, 2020 at 06:46:07PM +0900, Namhyung Kim wrote:
> >
> > SNIP
> >
> > > +int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> > > +{
> > > + struct evlist *orig_list, *tmp_list;
> > > + struct evsel *pos, *evsel, *leader;
> > > + struct cgroup *cgrp = NULL;
> > > + const char *p, *e, *eos = str + strlen(str);
> > > + int ret = -1;
> > > +
> > > + if (evlist->core.nr_entries == 0) {
> > > + fprintf(stderr, "must define events before cgroups\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > + orig_list = evlist__new();
> > > + tmp_list = evlist__new();
> > > + if (orig_list == NULL || tmp_list == NULL) {
> > > + fprintf(stderr, "memory allocation failed\n");
> > > + return -ENOMEM;
> > > + }
> > > +
> > > + /* save original events and init evlist */
> > > + perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
> > > + evlist->core.nr_entries = 0;
> > > +
> > > + for (;;) {
> > > + p = strchr(str, ',');
> > > + e = p ? p : eos;
> > > +
> > > + /* allow empty cgroups, i.e., skip */
> > > + if (e - str) {
> > > + /* termination added */
> > > + char *name = strndup(str, e - str);
> > > + if (!name)
> > > + goto out_err;
> > > +
> > > + cgrp = cgroup__new(name);
> > > + free(name);
> > > + if (cgrp == NULL)
> > > + goto out_err;
> > > + } else {
> > > + cgrp = NULL;
> > > + }
> > > +
> > > + leader = NULL;
> > > + evlist__for_each_entry(orig_list, pos) {
> > > + evsel = evsel__clone(pos);
> > > + if (evsel == NULL)
> > > + goto out_err;
> > > +
> > > + cgroup__put(evsel->cgrp);
> > > + evsel->cgrp = cgroup__get(cgrp);
> > > +
> > > + if (evsel__is_group_leader(pos))
> > > + leader = evsel;
> > > + evsel->leader = leader;
> >
> > hum, will this work if there's standalone event after group? like:
> >
> > {cycles,instructions},cache-misses
> >
> > cache-misses will get cycles as group leader no?
>
> AFAIK non-group events are treated as a leader of its own group.
> So evsel__is_group_leader() will return true for cache-misses.
right, then it's fine
thanks,
jirka
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-21 9:46 ` [PATCH 2/5] perf stat: Add --for-each-cgroup option Namhyung Kim
2020-09-22 21:40 ` Jiri Olsa
@ 2020-09-22 21:52 ` Stephane Eranian
2020-09-22 22:55 ` Namhyung Kim
2020-09-22 22:57 ` Namhyung Kim
2 siblings, 1 reply; 16+ messages in thread
From: Stephane Eranian @ 2020-09-22 21:52 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Andi Kleen, Ian Rogers
Hi,
On Mon, Sep 21, 2020 at 2:46 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The --for-each-cgroup option is a syntax sugar to monitor large number
> of cgroups easily. Current command line requires to list all the
> events and cgroups even if users want to monitor same events for each
> cgroup. This patch addresses that usage by copying given events for
> each cgroup on user's behalf.
>
> For instance, if they want to monitor 6 events for 200 cgroups each
> they should write 1200 event names (with -e) AND 1200 cgroup names
> (with -G) on the command line. But with this change, they can just
> specify 6 events and 200 cgroups with a new option.
>
> A simpler example below: It wants to measure 3 events for 2 cgroups
> ('A' and 'B'). The result is that total 6 events are counted like
> below.
>
> $ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup A,B sleep 1
>
You could also do it by keeping the -G option and providing
--for-each-cgroup as a modifier
of the behavior of -G:
$ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup -G
A,B sleep 1
That way, you do not have to handle the case where both are used.
And it makes transitioning to the new style simpler, i.e., the -G
option remains, just need
to trim the number of cgroups to 200 in your example.
Just a suggestion.
> Performance counter stats for 'system wide':
>
> 988.18 msec cpu-clock A # 0.987 CPUs utilized
> 3,153,761,702 cycles A # 3.200 GHz (100.00%)
> 8,067,769,847 instructions A # 2.57 insn per cycle (100.00%)
> 982.71 msec cpu-clock B # 0.982 CPUs utilized
> 3,136,093,298 cycles B # 3.182 GHz (99.99%)
> 8,109,619,327 instructions B # 2.58 insn per cycle (99.99%)
>
> 1.001228054 seconds time elapsed
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-stat.c | 20 +++++++++-
> tools/perf/util/cgroup.c | 84 +++++++++++++++++++++++++++++++++++++++
> tools/perf/util/cgroup.h | 1 +
> tools/perf/util/stat.h | 1 +
> 4 files changed, 105 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index 7f8d756d9408..a43e58e0a088 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -1051,6 +1051,17 @@ static int parse_control_option(const struct option *opt,
> return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack, &config->ctl_fd_close);
> }
>
> +static int parse_stat_cgroups(const struct option *opt,
> + const char *str, int unset)
> +{
> + if (stat_config.cgroup_list) {
> + pr_err("--cgroup and --for-each-cgroup cannot be used together\n");
> + return -1;
> + }
> +
> + return parse_cgroups(opt, str, unset);
> +}
> +
> static struct option stat_options[] = {
> OPT_BOOLEAN('T', "transaction", &transaction_run,
> "hardware transaction statistics"),
> @@ -1094,7 +1105,9 @@ static struct option stat_options[] = {
> OPT_STRING('x', "field-separator", &stat_config.csv_sep, "separator",
> "print counts with custom separator"),
> OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
> - "monitor event in cgroup name only", parse_cgroups),
> + "monitor event in cgroup name only", parse_stat_cgroups),
> + OPT_STRING(0, "for-each-cgroup", &stat_config.cgroup_list, "name",
> + "expand events for each cgroup"),
> OPT_STRING('o', "output", &output_name, "file", "output file name"),
> OPT_BOOLEAN(0, "append", &append_file, "append to the output file"),
> OPT_INTEGER(0, "log-fd", &output_fd,
> @@ -2234,6 +2247,11 @@ int cmd_stat(int argc, const char **argv)
> if (add_default_attributes())
> goto out;
>
> + if (stat_config.cgroup_list) {
> + if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list) < 0)
> + goto out;
> + }
> +
> target__validate(&target);
>
> if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide))
> diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
> index 050dea9f1e88..e4916ed740ac 100644
> --- a/tools/perf/util/cgroup.c
> +++ b/tools/perf/util/cgroup.c
> @@ -12,6 +12,7 @@
> #include <api/fs/fs.h>
>
> int nr_cgroups;
> +bool multiply_cgroup;
>
> static int open_cgroup(const char *name)
> {
> @@ -156,6 +157,10 @@ int parse_cgroups(const struct option *opt, const char *str,
> return -1;
> }
>
> + /* delay processing cgroups after it sees all events */
> + if (multiply_cgroup)
> + return 0;
> +
> for (;;) {
> p = strchr(str, ',');
> e = p ? p : eos;
> @@ -193,6 +198,85 @@ int parse_cgroups(const struct option *opt, const char *str,
> return 0;
> }
>
> +int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> +{
> + struct evlist *orig_list, *tmp_list;
> + struct evsel *pos, *evsel, *leader;
> + struct cgroup *cgrp = NULL;
> + const char *p, *e, *eos = str + strlen(str);
> + int ret = -1;
> +
> + if (evlist->core.nr_entries == 0) {
> + fprintf(stderr, "must define events before cgroups\n");
> + return -EINVAL;
> + }
> +
> + orig_list = evlist__new();
> + tmp_list = evlist__new();
> + if (orig_list == NULL || tmp_list == NULL) {
> + fprintf(stderr, "memory allocation failed\n");
> + return -ENOMEM;
> + }
> +
> + /* save original events and init evlist */
> + perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
> + evlist->core.nr_entries = 0;
> +
> + for (;;) {
> + p = strchr(str, ',');
> + e = p ? p : eos;
> +
> + /* allow empty cgroups, i.e., skip */
> + if (e - str) {
> + /* termination added */
> + char *name = strndup(str, e - str);
> + if (!name)
> + goto out_err;
> +
> + cgrp = cgroup__new(name);
> + free(name);
> + if (cgrp == NULL)
> + goto out_err;
> + } else {
> + cgrp = NULL;
> + }
> +
> + leader = NULL;
> + evlist__for_each_entry(orig_list, pos) {
> + evsel = evsel__clone(pos);
> + if (evsel == NULL)
> + goto out_err;
> +
> + cgroup__put(evsel->cgrp);
> + evsel->cgrp = cgroup__get(cgrp);
> +
> + if (evsel__is_group_leader(pos))
> + leader = evsel;
> + evsel->leader = leader;
> +
> + evlist__add(tmp_list, evsel);
> + }
> + /* cgroup__new() has a refcount, release it here */
> + cgroup__put(cgrp);
> + nr_cgroups++;
> +
> + perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
> + tmp_list->core.nr_entries = 0;
> +
> + if (!p) {
> + ret = 0;
> + break;
> + }
> + str = p+1;
> + }
> +
> +out_err:
> + evlist__delete(orig_list);
> + evlist__delete(tmp_list);
> +
> + return ret;
> +}
> +
> static struct cgroup *__cgroup__findnew(struct rb_root *root, uint64_t id,
> bool create, const char *path)
> {
> diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
> index e98d5975fe55..32893018296f 100644
> --- a/tools/perf/util/cgroup.h
> +++ b/tools/perf/util/cgroup.h
> @@ -24,6 +24,7 @@ void cgroup__put(struct cgroup *cgroup);
> struct evlist;
>
> struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
> +int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
>
> void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
>
> diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
> index 9911fc6adbfd..7325de5bf2a6 100644
> --- a/tools/perf/util/stat.h
> +++ b/tools/perf/util/stat.h
> @@ -137,6 +137,7 @@ struct perf_stat_config {
> int ctl_fd;
> int ctl_fd_ack;
> bool ctl_fd_close;
> + const char *cgroup_list;
> };
>
> void perf_stat__set_big_num(int set);
> --
> 2.28.0.681.g6f77f65b4e-goog
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-22 21:52 ` Stephane Eranian
@ 2020-09-22 22:55 ` Namhyung Kim
0 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-22 22:55 UTC (permalink / raw)
To: Stephane Eranian
Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Andi Kleen, Ian Rogers
Hi Stephane,
On Wed, Sep 23, 2020 at 6:52 AM Stephane Eranian <eranian@google.com> wrote:
>
> Hi,
>
> On Mon, Sep 21, 2020 at 2:46 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > The --for-each-cgroup option is a syntax sugar to monitor large number
> > of cgroups easily. Current command line requires to list all the
> > events and cgroups even if users want to monitor same events for each
> > cgroup. This patch addresses that usage by copying given events for
> > each cgroup on user's behalf.
> >
> > For instance, if they want to monitor 6 events for 200 cgroups each
> > they should write 1200 event names (with -e) AND 1200 cgroup names
> > (with -G) on the command line. But with this change, they can just
> > specify 6 events and 200 cgroups with a new option.
> >
> > A simpler example below: It wants to measure 3 events for 2 cgroups
> > ('A' and 'B'). The result is that total 6 events are counted like
> > below.
> >
> > $ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup A,B sleep 1
> >
> You could also do it by keeping the -G option and providing
> --for-each-cgroup as a modifier
> of the behavior of -G:
>
> $ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup -G
> A,B sleep 1
>
> That way, you do not have to handle the case where both are used.
> And it makes transitioning to the new style simpler, i.e., the -G
> option remains, just need
> to trim the number of cgroups to 200 in your example.
>
> Just a suggestion.
Thanks for the suggestion. Actually that's the approach I took
in my v1 submission. And Jiri suggested the current way.
Personally I'm fine with either.
Thanks
Namhyung
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-21 9:46 ` [PATCH 2/5] perf stat: Add --for-each-cgroup option Namhyung Kim
2020-09-22 21:40 ` Jiri Olsa
2020-09-22 21:52 ` Stephane Eranian
@ 2020-09-22 22:57 ` Namhyung Kim
2 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-22 22:57 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers
On Mon, Sep 21, 2020 at 6:46 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> The --for-each-cgroup option is a syntax sugar to monitor large number
> of cgroups easily. Current command line requires to list all the
> events and cgroups even if users want to monitor same events for each
> cgroup. This patch addresses that usage by copying given events for
> each cgroup on user's behalf.
>
> For instance, if they want to monitor 6 events for 200 cgroups each
> they should write 1200 event names (with -e) AND 1200 cgroup names
> (with -G) on the command line. But with this change, they can just
> specify 6 events and 200 cgroups with a new option.
>
> A simpler example below: It wants to measure 3 events for 2 cgroups
> ('A' and 'B'). The result is that total 6 events are counted like
> below.
>
> $ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup A,B sleep 1
>
> Performance counter stats for 'system wide':
>
> 988.18 msec cpu-clock A # 0.987 CPUs utilized
> 3,153,761,702 cycles A # 3.200 GHz (100.00%)
> 8,067,769,847 instructions A # 2.57 insn per cycle (100.00%)
> 982.71 msec cpu-clock B # 0.982 CPUs utilized
> 3,136,093,298 cycles B # 3.182 GHz (99.99%)
> 8,109,619,327 instructions B # 2.58 insn per cycle (99.99%)
>
> 1.001228054 seconds time elapsed
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/builtin-stat.c | 20 +++++++++-
> tools/perf/util/cgroup.c | 84 +++++++++++++++++++++++++++++++++++++++
> tools/perf/util/cgroup.h | 1 +
> tools/perf/util/stat.h | 1 +
> 4 files changed, 105 insertions(+), 1 deletion(-)
Oh, I've realized that I didn't update the man page..
I'll send v4 soon.
Thanks
Namhyung
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups
2020-09-21 9:46 [PATCHSET v3 0/5] perf stat: Expand events for each cgroup Namhyung Kim
2020-09-21 9:46 ` [PATCH 1/5] perf evsel: Add evsel__clone() function Namhyung Kim
2020-09-21 9:46 ` [PATCH 2/5] perf stat: Add --for-each-cgroup option Namhyung Kim
@ 2020-09-21 9:46 ` Namhyung Kim
2020-09-22 21:29 ` Jiri Olsa
2020-09-21 9:46 ` [PATCH 4/5] perf tools: Allow creation of cgroup without open Namhyung Kim
2020-09-21 9:46 ` [PATCH 5/5] perf test: Add expand cgroup event test Namhyung Kim
4 siblings, 1 reply; 16+ messages in thread
From: Namhyung Kim @ 2020-09-21 9:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers, John Garry,
Kajol Jain
The metricgroup__copy_metric_events() is to handle metrics events when
expanding event for cgroups. As the metric events keep pointers to
evsel, it should be refreshed when events are cloned during the
operation.
The perf_stat__collect_metric_expr() is also called in case an event
has a metric directly.
During the copy, it references evsel by index as the evlist now has
cloned evsels for the given cgroup.
Cc: John Garry <john.garry@huawei.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Ian Rogers <irogers@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 3 +-
tools/perf/util/cgroup.c | 15 ++++++-
tools/perf/util/cgroup.h | 4 +-
tools/perf/util/evlist.c | 11 +++++
tools/perf/util/evlist.h | 1 +
tools/perf/util/metricgroup.c | 83 +++++++++++++++++++++++++++++++++++
tools/perf/util/metricgroup.h | 6 +++
7 files changed, 120 insertions(+), 3 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index a43e58e0a088..8b81d62ab18b 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2248,7 +2248,8 @@ int cmd_stat(int argc, const char **argv)
goto out;
if (stat_config.cgroup_list) {
- if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list) < 0)
+ if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list,
+ &stat_config.metric_events) < 0)
goto out;
}
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index e4916ed740ac..adf60597520b 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -3,6 +3,9 @@
#include "evsel.h"
#include "cgroup.h"
#include "evlist.h"
+#include "rblist.h"
+#include "metricgroup.h"
+#include "stat.h"
#include <linux/zalloc.h>
#include <sys/types.h>
#include <sys/stat.h>
@@ -198,10 +201,12 @@ int parse_cgroups(const struct option *opt, const char *str,
return 0;
}
-int evlist__expand_cgroup(struct evlist *evlist, const char *str)
+int evlist__expand_cgroup(struct evlist *evlist, const char *str,
+ struct rblist *metric_events)
{
struct evlist *orig_list, *tmp_list;
struct evsel *pos, *evsel, *leader;
+ struct rblist orig_metric_events;
struct cgroup *cgrp = NULL;
const char *p, *e, *eos = str + strlen(str);
int ret = -1;
@@ -221,6 +226,8 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
/* save original events and init evlist */
perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
evlist->core.nr_entries = 0;
+ orig_metric_events = *metric_events;
+ rblist__init(metric_events);
for (;;) {
p = strchr(str, ',');
@@ -260,6 +267,11 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
cgroup__put(cgrp);
nr_cgroups++;
+ perf_stat__collect_metric_expr(tmp_list);
+ if (metricgroup__copy_metric_events(tmp_list, cgrp, metric_events,
+ &orig_metric_events) < 0)
+ break;
+
perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
tmp_list->core.nr_entries = 0;
@@ -273,6 +285,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
out_err:
evlist__delete(orig_list);
evlist__delete(tmp_list);
+ rblist__exit(&orig_metric_events);
return ret;
}
diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
index 32893018296f..eea6df8ee373 100644
--- a/tools/perf/util/cgroup.h
+++ b/tools/perf/util/cgroup.h
@@ -22,9 +22,11 @@ struct cgroup *cgroup__get(struct cgroup *cgroup);
void cgroup__put(struct cgroup *cgroup);
struct evlist;
+struct rblist;
struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
-int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
+int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups,
+ struct rblist *metric_events);
void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index ee7b576d3b12..424209c4bcd2 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1964,3 +1964,14 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
return err;
}
+
+struct evsel *evlist__get_evsel(struct evlist *evlist, int idx)
+{
+ struct evsel *evsel;
+
+ evlist__for_each_entry(evlist, evsel) {
+ if (evsel->idx == idx)
+ return evsel;
+ }
+ return NULL;
+}
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index bc38a53f6a1a..8c5721cb14b2 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -386,4 +386,5 @@ int evlist__ctlfd_ack(struct evlist *evlist);
#define EVLIST_ENABLED_MSG "Events enabled\n"
#define EVLIST_DISABLED_MSG "Events disabled\n"
+struct evsel *evlist__get_evsel(struct evlist *evlist, int idx);
#endif /* __PERF_EVLIST_H */
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d03bac65a3c2..a88bd125880a 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -24,6 +24,7 @@
#include <api/fs/fs.h>
#include "util.h"
#include <asm/bug.h>
+#include "cgroup.h"
struct metric_event *metricgroup__lookup(struct rblist *metric_events,
struct evsel *evsel,
@@ -1105,3 +1106,85 @@ bool metricgroup__has_metric(const char *metric)
}
return false;
}
+
+int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
+ struct rblist *new_metric_events,
+ struct rblist *old_metric_events)
+{
+ unsigned i;
+
+ for (i = 0; i < rblist__nr_entries(old_metric_events); i++) {
+ struct rb_node *nd;
+ struct metric_event *old_me, *new_me;
+ struct metric_expr *old_expr, *new_expr;
+ struct evsel *evsel;
+ size_t alloc_size;
+ int idx, nr;
+
+ nd = rblist__entry(old_metric_events, i);
+ old_me = container_of(nd, struct metric_event, nd);
+
+ evsel = evlist__get_evsel(evlist, old_me->evsel->idx);
+ new_me = metricgroup__lookup(new_metric_events, evsel, true);
+ if (!new_me)
+ return -ENOMEM;
+
+ pr_debug("copying metric event for cgroup '%s': %s (idx=%d)\n",
+ cgrp ? cgrp->name : "root", evsel->name, evsel->idx);
+
+ list_for_each_entry(old_expr, &old_me->head, nd) {
+ new_expr = malloc(sizeof(*new_expr));
+ if (!new_expr)
+ return -ENOMEM;
+
+ new_expr->metric_expr = old_expr->metric_expr;
+ new_expr->metric_name = old_expr->metric_name;
+ new_expr->metric_unit = old_expr->metric_unit;
+ new_expr->runtime = old_expr->runtime;
+
+ if (old_expr->metric_refs) {
+ /* calculate number of metric_events */
+ for (nr = 0; old_expr->metric_refs[nr].metric_name; nr++)
+ continue;
+ alloc_size = sizeof(*new_expr->metric_refs);
+ new_expr->metric_refs = calloc(nr + 1, alloc_size);
+ if (!new_expr->metric_refs) {
+ free(new_expr);
+ return -ENOMEM;
+ }
+
+ memcpy(new_expr->metric_refs, old_expr->metric_refs,
+ nr * alloc_size);
+ } else {
+ new_expr->metric_refs = NULL;
+ }
+
+ /* calculate number of metric_events */
+ for (nr = 0; old_expr->metric_events[nr]; nr++)
+ continue;
+ alloc_size = sizeof(*new_expr->metric_events);
+ new_expr->metric_events = calloc(nr + 1, alloc_size);
+ if (!new_expr->metric_events) {
+ free(new_expr->metric_refs);
+ free(new_expr);
+ return -ENOMEM;
+ }
+
+ /* copy evsel in the same position */
+ for (idx = 0; idx < nr; idx++) {
+ evsel = old_expr->metric_events[idx];
+ evsel = evlist__get_evsel(evlist, evsel->idx);
+ if (evsel == NULL) {
+ free(new_expr->metric_events);
+ free(new_expr->metric_refs);
+ free(new_expr);
+ return -EINVAL;
+ }
+ new_expr->metric_events[idx] = evsel;
+ }
+
+ list_add(&new_expr->nd, &new_me->head);
+ }
+ }
+ return 0;
+}
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index 491a5d78252d..ed1b9392e624 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -7,11 +7,13 @@
#include <stdbool.h>
#include "pmu-events/pmu-events.h"
+struct evlist;
struct evsel;
struct evlist;
struct option;
struct rblist;
struct pmu_events_map;
+struct cgroup;
struct metric_event {
struct rb_node nd;
@@ -55,4 +57,8 @@ void metricgroup__print(bool metrics, bool groups, char *filter,
bool metricgroup__has_metric(const char *metric);
int arch_get_runtimeparam(struct pmu_event *pe __maybe_unused);
void metricgroup__rblist_exit(struct rblist *metric_events);
+
+int metricgroup__copy_metric_events(struct evlist *evlist, struct cgroup *cgrp,
+ struct rblist *new_metric_events,
+ struct rblist *old_metric_events);
#endif
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups
2020-09-21 9:46 ` [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups Namhyung Kim
@ 2020-09-22 21:29 ` Jiri Olsa
2020-09-22 22:46 ` Namhyung Kim
0 siblings, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2020-09-22 21:29 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Andi Kleen, Ian Rogers, John Garry, Kajol Jain
On Mon, Sep 21, 2020 at 06:46:08PM +0900, Namhyung Kim wrote:
SNIP
> @@ -260,6 +267,11 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> cgroup__put(cgrp);
> nr_cgroups++;
>
> + perf_stat__collect_metric_expr(tmp_list);
> + if (metricgroup__copy_metric_events(tmp_list, cgrp, metric_events,
> + &orig_metric_events) < 0)
> + break;
> +
> perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
> tmp_list->core.nr_entries = 0;
>
> @@ -273,6 +285,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> out_err:
> evlist__delete(orig_list);
> evlist__delete(tmp_list);
> + rblist__exit(&orig_metric_events);
>
> return ret;
> }
> diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
> index 32893018296f..eea6df8ee373 100644
> --- a/tools/perf/util/cgroup.h
> +++ b/tools/perf/util/cgroup.h
> @@ -22,9 +22,11 @@ struct cgroup *cgroup__get(struct cgroup *cgroup);
> void cgroup__put(struct cgroup *cgroup);
>
> struct evlist;
> +struct rblist;
>
> struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
> -int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
> +int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups,
> + struct rblist *metric_events);
>
> void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index ee7b576d3b12..424209c4bcd2 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -1964,3 +1964,14 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
>
> return err;
> }
> +
> +struct evsel *evlist__get_evsel(struct evlist *evlist, int idx)
perhaps evlist__find_evsel is better name?
we have get/put names for functions changing refcount
jirka
> +{
> + struct evsel *evsel;
> +
> + evlist__for_each_entry(evlist, evsel) {
> + if (evsel->idx == idx)
> + return evsel;
> + }
> + return NULL;
> +}
SNIP
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups
2020-09-22 21:29 ` Jiri Olsa
@ 2020-09-22 22:46 ` Namhyung Kim
0 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-22 22:46 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
Mark Rutland, Alexander Shishkin, LKML, Stephane Eranian,
Andi Kleen, Ian Rogers, John Garry, Kajol Jain
On Wed, Sep 23, 2020 at 6:29 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Mon, Sep 21, 2020 at 06:46:08PM +0900, Namhyung Kim wrote:
>
> SNIP
>
> > @@ -260,6 +267,11 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> > cgroup__put(cgrp);
> > nr_cgroups++;
> >
> > + perf_stat__collect_metric_expr(tmp_list);
> > + if (metricgroup__copy_metric_events(tmp_list, cgrp, metric_events,
> > + &orig_metric_events) < 0)
> > + break;
> > +
> > perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
> > tmp_list->core.nr_entries = 0;
> >
> > @@ -273,6 +285,7 @@ int evlist__expand_cgroup(struct evlist *evlist, const char *str)
> > out_err:
> > evlist__delete(orig_list);
> > evlist__delete(tmp_list);
> > + rblist__exit(&orig_metric_events);
> >
> > return ret;
> > }
> > diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
> > index 32893018296f..eea6df8ee373 100644
> > --- a/tools/perf/util/cgroup.h
> > +++ b/tools/perf/util/cgroup.h
> > @@ -22,9 +22,11 @@ struct cgroup *cgroup__get(struct cgroup *cgroup);
> > void cgroup__put(struct cgroup *cgroup);
> >
> > struct evlist;
> > +struct rblist;
> >
> > struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
> > -int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
> > +int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups,
> > + struct rblist *metric_events);
> >
> > void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
> >
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index ee7b576d3b12..424209c4bcd2 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -1964,3 +1964,14 @@ int evlist__ctlfd_process(struct evlist *evlist, enum evlist_ctl_cmd *cmd)
> >
> > return err;
> > }
> > +
> > +struct evsel *evlist__get_evsel(struct evlist *evlist, int idx)
>
> perhaps evlist__find_evsel is better name?
> we have get/put names for functions changing refcount
Looks better, will change.
Thanks
Namhyung
>
> > +{
> > + struct evsel *evsel;
> > +
> > + evlist__for_each_entry(evlist, evsel) {
> > + if (evsel->idx == idx)
> > + return evsel;
> > + }
> > + return NULL;
> > +}
>
> SNIP
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 4/5] perf tools: Allow creation of cgroup without open
2020-09-21 9:46 [PATCHSET v3 0/5] perf stat: Expand events for each cgroup Namhyung Kim
` (2 preceding siblings ...)
2020-09-21 9:46 ` [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups Namhyung Kim
@ 2020-09-21 9:46 ` Namhyung Kim
2020-09-21 9:46 ` [PATCH 5/5] perf test: Add expand cgroup event test Namhyung Kim
4 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-21 9:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers
This is a preparation for a test case of expanding events for multiple
cgroups. Instead of using real system cgroup, the test will use fake
cgroups so it needs a way to have them without a open file descriptor.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-stat.c | 2 +-
tools/perf/util/cgroup.c | 19 ++++++++++++-------
tools/perf/util/cgroup.h | 2 +-
3 files changed, 14 insertions(+), 9 deletions(-)
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,
- &stat_config.metric_events) < 0)
+ &stat_config.metric_events, true) < 0)
goto out;
}
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index adf60597520b..ee0b50f59977 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;
}
-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)
goto out_err;
- cgrp = cgroup__new(name);
+ cgrp = cgroup__new(name, open_cgroup);
free(name);
if (cgrp == NULL)
goto out_err;
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.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 5/5] perf test: Add expand cgroup event test
2020-09-21 9:46 [PATCHSET v3 0/5] perf stat: Expand events for each cgroup Namhyung Kim
` (3 preceding siblings ...)
2020-09-21 9:46 ` [PATCH 4/5] perf tools: Allow creation of cgroup without open Namhyung Kim
@ 2020-09-21 9:46 ` Namhyung Kim
4 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-21 9:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers, John Garry
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 <john.garry@huawei.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
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 +
4 files changed, 247 insertions(+)
create mode 100644 tools/perf/tests/expand-cgroup.c
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 000000000000..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 <subcmd/parse-options.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+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, evsel->name, ev_name[i % nr_events]);
+ goto out;
+ }
+ if (strcmp(evsel->cgrp->name, cgrp_name[i / nr_events])) {
+ pr_debug("cgroup name doesn't match:\n");
+ pr_debug(" evsel[%d]: %s\n expected: %s\n",
+ i, evsel->cgrp->name, cgrp_name[i / nr_events]);
+ goto out;
+ }
+
+ if ((i % nr_events) == 0) {
+ if (evsel__is_group_event(evsel) != was_group_event) {
+ pr_debug("event group doesn't match: got %s, expect %s\n",
+ evsel__is_group_event(evsel) ? "true" : "false",
+ was_group_event ? "true" : "false");
+ goto out;
+ }
+ if (evsel->core.nr_members != nr_members) {
+ pr_debug("event group member doesn't match: %d vs %d\n",
+ evsel->core.nr_members, nr_members);
+ goto out;
+ }
+ }
+ i++;
+ }
+ ret = TEST_OK;
+
+out: for (i = 0; i < nr_events; i++)
+ free(ev_name[i]);
+ free(ev_name);
+ return ret;
+}
+
+static int expand_default_events(void)
+{
+ int ret;
+ struct evlist *evlist;
+ struct rblist metric_events;
+
+ evlist = perf_evlist__new_default();
+ TEST_ASSERT_VAL("failed to get evlist", evlist);
+
+ rblist__init(&metric_events);
+ ret = test_expand_events(evlist, &metric_events);
+ evlist__delete(evlist);
+ return ret;
+}
+
+static int expand_group_events(void)
+{
+ int ret;
+ struct evlist *evlist;
+ struct rblist metric_events;
+ struct parse_events_error err;
+ const char event_str[] = "{cycles,instructions}";
+
+ symbol_conf.event_group = true;
+
+ evlist = evlist__new();
+ TEST_ASSERT_VAL("failed to get evlist", evlist);
+
+ ret = parse_events(evlist, event_str, &err);
+ if (ret < 0) {
+ pr_debug("failed to parse event '%s', err %d, str '%s'\n",
+ event_str, ret, err.str);
+ parse_events_print_error(&err, event_str);
+ goto out;
+ }
+
+ rblist__init(&metric_events);
+ ret = test_expand_events(evlist, &metric_events);
+out:
+ evlist__delete(evlist);
+ return ret;
+}
+
+static int expand_libpfm_events(void)
+{
+ int ret;
+ struct evlist *evlist;
+ struct rblist metric_events;
+ const char event_str[] = "UNHALTED_CORE_CYCLES";
+ struct option opt = {
+ .value = &evlist,
+ };
+
+ symbol_conf.event_group = true;
+
+ evlist = evlist__new();
+ TEST_ASSERT_VAL("failed to get evlist", evlist);
+
+ ret = parse_libpfm_events_option(&opt, event_str, 0);
+ if (ret < 0) {
+ pr_debug("failed to parse libpfm event '%s', err %d\n",
+ event_str, ret);
+ goto out;
+ }
+ if (perf_evlist__empty(evlist)) {
+ pr_debug("libpfm was not enabled\n");
+ goto out;
+ }
+
+ rblist__init(&metric_events);
+ ret = test_expand_events(evlist, &metric_events);
+out:
+ evlist__delete(evlist);
+ return ret;
+}
+
+static int expand_metric_events(void)
+{
+ int ret;
+ struct evlist *evlist;
+ struct rblist metric_events;
+ const char metric_str[] = "CPI";
+
+ struct pmu_event pme_test[] = {
+ {
+ .metric_expr = "instructions / cycles",
+ .metric_name = "IPC",
+ },
+ {
+ .metric_expr = "1 / IPC",
+ .metric_name = "CPI",
+ },
+ {
+ .metric_expr = NULL,
+ .metric_name = NULL,
+ },
+ };
+ struct pmu_events_map ev_map = {
+ .cpuid = "test",
+ .version = "1",
+ .type = "core",
+ .table = pme_test,
+ };
+
+ evlist = evlist__new();
+ TEST_ASSERT_VAL("failed to get evlist", evlist);
+
+ rblist__init(&metric_events);
+ ret = metricgroup__parse_groups_test(evlist, &ev_map, metric_str,
+ false, false, &metric_events);
+ if (ret < 0) {
+ pr_debug("failed to parse '%s' metric\n", metric_str);
+ goto out;
+ }
+
+ ret = test_expand_events(evlist, &metric_events);
+
+out:
+ metricgroup__rblist_exit(&metric_events);
+ evlist__delete(evlist);
+ return ret;
+}
+
+int test__expand_cgroup_events(struct test *test __maybe_unused,
+ int subtest __maybe_unused)
+{
+ int ret;
+
+ ret = expand_default_events();
+ TEST_ASSERT_EQUAL("failed to expand default events", ret, 0);
+
+ ret = expand_group_events();
+ TEST_ASSERT_EQUAL("failed to expand event group", ret, 0);
+
+ ret = expand_libpfm_events();
+ TEST_ASSERT_EQUAL("failed to expand event group", ret, 0);
+
+ ret = expand_metric_events();
+ TEST_ASSERT_EQUAL("failed to expand metric events", ret, 0);
+
+ return ret;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index ef0f33c6ba23..c85a2c08e407 100644
--- a/tools/perf/tests/tests.h
+++ 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);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-23 1:59 [PATCHSET v4 0/5] perf stat: Expand events for each cgroup Namhyung Kim
@ 2020-09-23 1:59 ` Namhyung Kim
0 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-23 1:59 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers
The --for-each-cgroup option is a syntax sugar to monitor large number
of cgroups easily. Current command line requires to list all the
events and cgroups even if users want to monitor same events for each
cgroup. This patch addresses that usage by copying given events for
each cgroup on user's behalf.
For instance, if they want to monitor 6 events for 200 cgroups each
they should write 1200 event names (with -e) AND 1200 cgroup names
(with -G) on the command line. But with this change, they can just
specify 6 events and 200 cgroups with a new option.
A simpler example below: It wants to measure 3 events for 2 cgroups
('A' and 'B'). The result is that total 6 events are counted like
below.
$ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup A,B sleep 1
Performance counter stats for 'system wide':
988.18 msec cpu-clock A # 0.987 CPUs utilized
3,153,761,702 cycles A # 3.200 GHz (100.00%)
8,067,769,847 instructions A # 2.57 insn per cycle (100.00%)
982.71 msec cpu-clock B # 0.982 CPUs utilized
3,136,093,298 cycles B # 3.182 GHz (99.99%)
8,109,619,327 instructions B # 2.58 insn per cycle (99.99%)
1.001228054 seconds time elapsed
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-stat.txt | 5 ++
tools/perf/builtin-stat.c | 27 ++++++++-
tools/perf/util/cgroup.c | 79 ++++++++++++++++++++++++++
tools/perf/util/cgroup.h | 1 +
tools/perf/util/stat.h | 1 +
5 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 7d18694e592a..bb17c9caec78 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -166,6 +166,11 @@ use '-e e1 -e e2 -G foo,foo' or just use '-e e1 -e e2 -G foo'.
If wanting to monitor, say, 'cycles' for a cgroup and also for system wide, this
command line can be used: 'perf stat -e cycles -G cgroup_name -a -e cycles'.
+--for-each-cgroup name::
+Expand event list for each cgroup in "name" (allow multiple cgroups separated
+by comma). This has same effect that repeating -e option and -G option for
+each event x name. This option cannot be used with -G/--cgroup option.
+
-o file::
--output file::
Print the output into the designated file.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 7f8d756d9408..23abf14b6e16 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1051,6 +1051,17 @@ static int parse_control_option(const struct option *opt,
return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack, &config->ctl_fd_close);
}
+static int parse_stat_cgroups(const struct option *opt,
+ const char *str, int unset)
+{
+ if (stat_config.cgroup_list) {
+ pr_err("--cgroup and --for-each-cgroup cannot be used together\n");
+ return -1;
+ }
+
+ return parse_cgroups(opt, str, unset);
+}
+
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
@@ -1094,7 +1105,9 @@ static struct option stat_options[] = {
OPT_STRING('x', "field-separator", &stat_config.csv_sep, "separator",
"print counts with custom separator"),
OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
- "monitor event in cgroup name only", parse_cgroups),
+ "monitor event in cgroup name only", parse_stat_cgroups),
+ OPT_STRING(0, "for-each-cgroup", &stat_config.cgroup_list, "name",
+ "expand events for each cgroup"),
OPT_STRING('o', "output", &output_name, "file", "output file name"),
OPT_BOOLEAN(0, "append", &append_file, "append to the output file"),
OPT_INTEGER(0, "log-fd", &output_fd,
@@ -2234,6 +2247,18 @@ int cmd_stat(int argc, const char **argv)
if (add_default_attributes())
goto out;
+ if (stat_config.cgroup_list) {
+ if (nr_cgroups > 0) {
+ pr_err("--cgroup and --for-each-cgroup cannot be used together\n");
+ parse_options_usage(stat_usage, stat_options, "G", 1);
+ parse_options_usage(NULL, stat_options, "for-each-cgroup", 0);
+ goto out;
+ }
+
+ if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list) < 0)
+ goto out;
+ }
+
target__validate(&target);
if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide))
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 050dea9f1e88..8b6a4fa49082 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -193,6 +193,85 @@ int parse_cgroups(const struct option *opt, const char *str,
return 0;
}
+int evlist__expand_cgroup(struct evlist *evlist, const char *str)
+{
+ struct evlist *orig_list, *tmp_list;
+ struct evsel *pos, *evsel, *leader;
+ struct cgroup *cgrp = NULL;
+ const char *p, *e, *eos = str + strlen(str);
+ int ret = -1;
+
+ if (evlist->core.nr_entries == 0) {
+ fprintf(stderr, "must define events before cgroups\n");
+ return -EINVAL;
+ }
+
+ orig_list = evlist__new();
+ tmp_list = evlist__new();
+ if (orig_list == NULL || tmp_list == NULL) {
+ fprintf(stderr, "memory allocation failed\n");
+ return -ENOMEM;
+ }
+
+ /* save original events and init evlist */
+ perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
+ evlist->core.nr_entries = 0;
+
+ for (;;) {
+ p = strchr(str, ',');
+ e = p ? p : eos;
+
+ /* allow empty cgroups, i.e., skip */
+ if (e - str) {
+ /* termination added */
+ char *name = strndup(str, e - str);
+ if (!name)
+ goto out_err;
+
+ cgrp = cgroup__new(name);
+ free(name);
+ if (cgrp == NULL)
+ goto out_err;
+ } else {
+ cgrp = NULL;
+ }
+
+ leader = NULL;
+ evlist__for_each_entry(orig_list, pos) {
+ evsel = evsel__clone(pos);
+ if (evsel == NULL)
+ goto out_err;
+
+ cgroup__put(evsel->cgrp);
+ evsel->cgrp = cgroup__get(cgrp);
+
+ if (evsel__is_group_leader(pos))
+ leader = evsel;
+ evsel->leader = leader;
+
+ evlist__add(tmp_list, evsel);
+ }
+ /* cgroup__new() has a refcount, release it here */
+ cgroup__put(cgrp);
+ nr_cgroups++;
+
+ perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
+ tmp_list->core.nr_entries = 0;
+
+ if (!p) {
+ ret = 0;
+ break;
+ }
+ str = p+1;
+ }
+
+out_err:
+ evlist__delete(orig_list);
+ evlist__delete(tmp_list);
+
+ return ret;
+}
+
static struct cgroup *__cgroup__findnew(struct rb_root *root, uint64_t id,
bool create, const char *path)
{
diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
index e98d5975fe55..32893018296f 100644
--- a/tools/perf/util/cgroup.h
+++ b/tools/perf/util/cgroup.h
@@ -24,6 +24,7 @@ void cgroup__put(struct cgroup *cgroup);
struct evlist;
struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
+int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index 9911fc6adbfd..7325de5bf2a6 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -137,6 +137,7 @@ struct perf_stat_config {
int ctl_fd;
int ctl_fd_ack;
bool ctl_fd_close;
+ const char *cgroup_list;
};
void perf_stat__set_big_num(int set);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/5] perf stat: Add --for-each-cgroup option
2020-09-24 12:44 [PATCHSET v5 0/5] perf stat: Expand events for each cgroup Namhyung Kim
@ 2020-09-24 12:44 ` Namhyung Kim
0 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2020-09-24 12:44 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Jiri Olsa
Cc: Ingo Molnar, Peter Zijlstra, Mark Rutland, Alexander Shishkin,
LKML, Stephane Eranian, Andi Kleen, Ian Rogers
The --for-each-cgroup option is a syntax sugar to monitor large number
of cgroups easily. Current command line requires to list all the
events and cgroups even if users want to monitor same events for each
cgroup. This patch addresses that usage by copying given events for
each cgroup on user's behalf.
For instance, if they want to monitor 6 events for 200 cgroups each
they should write 1200 event names (with -e) AND 1200 cgroup names
(with -G) on the command line. But with this change, they can just
specify 6 events and 200 cgroups with a new option.
A simpler example below: It wants to measure 3 events for 2 cgroups
('A' and 'B'). The result is that total 6 events are counted like
below.
$ ./perf stat -a -e cpu-clock,cycles,instructions --for-each-cgroup A,B sleep 1
Performance counter stats for 'system wide':
988.18 msec cpu-clock A # 0.987 CPUs utilized
3,153,761,702 cycles A # 3.200 GHz (100.00%)
8,067,769,847 instructions A # 2.57 insn per cycle (100.00%)
982.71 msec cpu-clock B # 0.982 CPUs utilized
3,136,093,298 cycles B # 3.182 GHz (99.99%)
8,109,619,327 instructions B # 2.58 insn per cycle (99.99%)
1.001228054 seconds time elapsed
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/Documentation/perf-stat.txt | 5 ++
tools/perf/builtin-stat.c | 27 ++++++++-
tools/perf/util/cgroup.c | 79 ++++++++++++++++++++++++++
tools/perf/util/cgroup.h | 1 +
tools/perf/util/stat.h | 1 +
5 files changed, 112 insertions(+), 1 deletion(-)
diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 5bf3d7ae4660..9f9f29025e49 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -166,6 +166,11 @@ use '-e e1 -e e2 -G foo,foo' or just use '-e e1 -e e2 -G foo'.
If wanting to monitor, say, 'cycles' for a cgroup and also for system wide, this
command line can be used: 'perf stat -e cycles -G cgroup_name -a -e cycles'.
+--for-each-cgroup name::
+Expand event list for each cgroup in "name" (allow multiple cgroups separated
+by comma). This has same effect that repeating -e option and -G option for
+each event x name. This option cannot be used with -G/--cgroup option.
+
-o file::
--output file::
Print the output into the designated file.
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index cbb2d977eec7..0c9e21a29795 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -1060,6 +1060,17 @@ static int parse_control_option(const struct option *opt,
return evlist__parse_control(str, &config->ctl_fd, &config->ctl_fd_ack, &config->ctl_fd_close);
}
+static int parse_stat_cgroups(const struct option *opt,
+ const char *str, int unset)
+{
+ if (stat_config.cgroup_list) {
+ pr_err("--cgroup and --for-each-cgroup cannot be used together\n");
+ return -1;
+ }
+
+ return parse_cgroups(opt, str, unset);
+}
+
static struct option stat_options[] = {
OPT_BOOLEAN('T', "transaction", &transaction_run,
"hardware transaction statistics"),
@@ -1103,7 +1114,9 @@ static struct option stat_options[] = {
OPT_STRING('x', "field-separator", &stat_config.csv_sep, "separator",
"print counts with custom separator"),
OPT_CALLBACK('G', "cgroup", &evsel_list, "name",
- "monitor event in cgroup name only", parse_cgroups),
+ "monitor event in cgroup name only", parse_stat_cgroups),
+ OPT_STRING(0, "for-each-cgroup", &stat_config.cgroup_list, "name",
+ "expand events for each cgroup"),
OPT_STRING('o', "output", &output_name, "file", "output file name"),
OPT_BOOLEAN(0, "append", &append_file, "append to the output file"),
OPT_INTEGER(0, "log-fd", &output_fd,
@@ -2213,6 +2226,18 @@ int cmd_stat(int argc, const char **argv)
if (add_default_attributes())
goto out;
+ if (stat_config.cgroup_list) {
+ if (nr_cgroups > 0) {
+ pr_err("--cgroup and --for-each-cgroup cannot be used together\n");
+ parse_options_usage(stat_usage, stat_options, "G", 1);
+ parse_options_usage(NULL, stat_options, "for-each-cgroup", 0);
+ goto out;
+ }
+
+ if (evlist__expand_cgroup(evsel_list, stat_config.cgroup_list) < 0)
+ goto out;
+ }
+
target__validate(&target);
if ((stat_config.aggr_mode == AGGR_THREAD) && (target.system_wide))
diff --git a/tools/perf/util/cgroup.c b/tools/perf/util/cgroup.c
index 050dea9f1e88..8b6a4fa49082 100644
--- a/tools/perf/util/cgroup.c
+++ b/tools/perf/util/cgroup.c
@@ -193,6 +193,85 @@ int parse_cgroups(const struct option *opt, const char *str,
return 0;
}
+int evlist__expand_cgroup(struct evlist *evlist, const char *str)
+{
+ struct evlist *orig_list, *tmp_list;
+ struct evsel *pos, *evsel, *leader;
+ struct cgroup *cgrp = NULL;
+ const char *p, *e, *eos = str + strlen(str);
+ int ret = -1;
+
+ if (evlist->core.nr_entries == 0) {
+ fprintf(stderr, "must define events before cgroups\n");
+ return -EINVAL;
+ }
+
+ orig_list = evlist__new();
+ tmp_list = evlist__new();
+ if (orig_list == NULL || tmp_list == NULL) {
+ fprintf(stderr, "memory allocation failed\n");
+ return -ENOMEM;
+ }
+
+ /* save original events and init evlist */
+ perf_evlist__splice_list_tail(orig_list, &evlist->core.entries);
+ evlist->core.nr_entries = 0;
+
+ for (;;) {
+ p = strchr(str, ',');
+ e = p ? p : eos;
+
+ /* allow empty cgroups, i.e., skip */
+ if (e - str) {
+ /* termination added */
+ char *name = strndup(str, e - str);
+ if (!name)
+ goto out_err;
+
+ cgrp = cgroup__new(name);
+ free(name);
+ if (cgrp == NULL)
+ goto out_err;
+ } else {
+ cgrp = NULL;
+ }
+
+ leader = NULL;
+ evlist__for_each_entry(orig_list, pos) {
+ evsel = evsel__clone(pos);
+ if (evsel == NULL)
+ goto out_err;
+
+ cgroup__put(evsel->cgrp);
+ evsel->cgrp = cgroup__get(cgrp);
+
+ if (evsel__is_group_leader(pos))
+ leader = evsel;
+ evsel->leader = leader;
+
+ evlist__add(tmp_list, evsel);
+ }
+ /* cgroup__new() has a refcount, release it here */
+ cgroup__put(cgrp);
+ nr_cgroups++;
+
+ perf_evlist__splice_list_tail(evlist, &tmp_list->core.entries);
+ tmp_list->core.nr_entries = 0;
+
+ if (!p) {
+ ret = 0;
+ break;
+ }
+ str = p+1;
+ }
+
+out_err:
+ evlist__delete(orig_list);
+ evlist__delete(tmp_list);
+
+ return ret;
+}
+
static struct cgroup *__cgroup__findnew(struct rb_root *root, uint64_t id,
bool create, const char *path)
{
diff --git a/tools/perf/util/cgroup.h b/tools/perf/util/cgroup.h
index e98d5975fe55..32893018296f 100644
--- a/tools/perf/util/cgroup.h
+++ b/tools/perf/util/cgroup.h
@@ -24,6 +24,7 @@ void cgroup__put(struct cgroup *cgroup);
struct evlist;
struct cgroup *evlist__findnew_cgroup(struct evlist *evlist, const char *name);
+int evlist__expand_cgroup(struct evlist *evlist, const char *cgroups);
void evlist__set_default_cgroup(struct evlist *evlist, struct cgroup *cgroup);
diff --git a/tools/perf/util/stat.h b/tools/perf/util/stat.h
index f36c8c92efa0..487010c624be 100644
--- a/tools/perf/util/stat.h
+++ b/tools/perf/util/stat.h
@@ -145,6 +145,7 @@ struct perf_stat_config {
int ctl_fd;
int ctl_fd_ack;
bool ctl_fd_close;
+ const char *cgroup_list;
};
void perf_stat__set_big_num(int set);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 16+ messages in thread