[PATCHSET v3 0/5] perf stat: Expand events for each cgroup

[PATCHSET v3 0/5] perf stat: Expand events for each cgroup

[Namhyung Kim]

(Resending with +LKML, sorry)

Hello,

When we profile cgroup events with perf stat, it's very annoying to
specify events and cgroups on the command line as it requires the
mapping between events and cgroups.  (Note that perf record can use
cgroup sampling but it's not usable for perf stat).

I guess most cases we just want to use a same set of events (N) for
all cgroups (M), but we need to specify NxM events and NxM cgroups.
This is not good especially when profiling large number of cgroups:
say M=200.

So I added --for-each-cgroup option to make it easy for that case.  It
will create NxM events from N events and M cgroups.  One more upside
is that it can handle metrics too.

For example, the following example measures IPC metric for 3 cgroups

  $ cat perf-expand-cgrp.sh
  #!/bin/sh
  
  METRIC=${1:-IPC}
  CGROUP_DIR=/sys/fs/cgroup/perf_event
  
  sudo mkdir $CGROUP_DIR/A $CGROUP_DIR/B $CGROUP_DIR/C
  
  # add backgroupd workload for each cgroup
  echo $$ | sudo tee $CGROUP_DIR/A/cgroup.procs > /dev/null
  yes > /dev/null &
  echo $$ | sudo tee $CGROUP_DIR/B/cgroup.procs > /dev/null
  yes > /dev/null &
  echo $$ | sudo tee $CGROUP_DIR/C/cgroup.procs > /dev/null
  yes > /dev/null &

  # run 'perf stat' in the root cgroup
  echo $$ | sudo tee $CGROUP_DIR/cgroup.procs > /dev/null
  perf stat -a -M $METRIC --for-each-cgroup A,B,C sleep 1
  
  kill %1 %2 %3
  sudo rmdir $CGROUP_DIR/A $CGROUP_DIR/B $CGROUP_DIR/C

  
  $ ./perf-expand-cgrp.sh IPC
  
   Performance counter stats for 'system wide':
  
      11,284,850,010      inst_retired.any          A #     2.71 IPC                    
       4,157,915,982      cpu_clk_unhalted.thread   A                                   
      11,342,188,640      inst_retired.any          B #     2.72 IPC                    
       4,173,014,732      cpu_clk_unhalted.thread   B                                   
      11,135,863,604      inst_retired.any          C #     2.67 IPC                    
       4,171,375,184      cpu_clk_unhalted.thread   C                                   
  
         1.011948803 seconds time elapsed


* Changes from v2:
 - put relevant fields in evsel together  (Jiri)
 - add various error checks  (Jiri)
 - split cgroup open patch  (Jiri)

* Changes from v1:
 - rename the option to --for-each-cgroup  (Jiri)
 - copy evsel fields explicitly  (Jiri)
 - add libpfm4 test  (Ian)


The code is available at 'perf/cgroup-multiply-v3' branch on

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks
Namhyung


Namhyung Kim (5):
  perf evsel: Add evsel__clone() function
  perf stat: Add --for-each-cgroup option
  perf tools: Copy metric events properly when expand cgroups
  perf tools: Allow creation of cgroup without open
  perf test: Add expand cgroup event test

 tools/perf/builtin-stat.c        |  21 ++-
 tools/perf/tests/Build           |   1 +
 tools/perf/tests/builtin-test.c  |   4 +
 tools/perf/tests/expand-cgroup.c | 241 +++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h         |   1 +
 tools/perf/util/cgroup.c         | 112 +++++++++++++-
 tools/perf/util/cgroup.h         |   3 +
 tools/perf/util/evlist.c         |  11 ++
 tools/perf/util/evlist.h         |   1 +
 tools/perf/util/evsel.c          | 104 +++++++++++++
 tools/perf/util/evsel.h          |  93 +++++++-----
 tools/perf/util/metricgroup.c    |  83 +++++++++++
 tools/perf/util/metricgroup.h    |   6 +
 tools/perf/util/stat.h           |   1 +
 14 files changed, 637 insertions(+), 45 deletions(-)
 create mode 100644 tools/perf/tests/expand-cgroup.c

-- 
2.28.0.681.g6f77f65b4e-goog

[PATCH 1/5] perf evsel: Add evsel__clone() function

[Namhyung Kim]

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

[PATCH 2/5] perf stat: Add --for-each-cgroup option

[Namhyung Kim]

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

[PATCH 3/5] perf tools: Copy metric events properly when expand cgroups

[Namhyung Kim]

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

[PATCH 4/5] perf tools: Allow creation of cgroup without open

[Namhyung Kim]

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

[PATCH 5/5] perf test: Add expand cgroup event test

[Namhyung Kim]

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

Re: [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups

[Jiri Olsa]

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

Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option

[Jiri Olsa]

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

Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option

[Stephane Eranian]

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
>

Re: [PATCH 3/5] perf tools: Copy metric events properly when expand cgroups

[Namhyung Kim]

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
>

Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option

[Namhyung Kim]

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
>

Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option

[Namhyung Kim]

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

Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option

[Namhyung Kim]

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

Re: [PATCH 2/5] perf stat: Add --for-each-cgroup option

[Jiri Olsa]

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

[PATCH 2/5] perf stat: Add --for-each-cgroup option

[Namhyung Kim]

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

[PATCH 2/5] perf stat: Add --for-each-cgroup option

[Namhyung Kim]

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