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 <[email protected]>
---
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
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
Hi,
On Mon, Sep 21, 2020 at 2:46 AM Namhyung Kim <[email protected]> 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 <[email protected]>
> ---
> 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
>
On Wed, Sep 23, 2020 at 6:40 AM Jiri Olsa <[email protected]> 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
>
Hi Stephane,
On Wed, Sep 23, 2020 at 6:52 AM Stephane Eranian <[email protected]> wrote:
>
> Hi,
>
> On Mon, Sep 21, 2020 at 2:46 AM Namhyung Kim <[email protected]> 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
On Mon, Sep 21, 2020 at 6:46 PM Namhyung Kim <[email protected]> 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 <[email protected]>
> ---
> 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
On Wed, Sep 23, 2020 at 07:51:33AM +0900, Namhyung Kim wrote:
> On Wed, Sep 23, 2020 at 6:40 AM Jiri Olsa <[email protected]> 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