[RFC PATCH 1/3] perf: Add cpumask for uncore pmu

[RFC PATCH 1/3] perf: Add cpumask for uncore pmu

[Yan, Zheng]

Hi,

This patchset add a cpumask file to the uncore pmu sysfs directory.
If user doesn't explicitly specify CPU list, perf-stat only collects
uncore events on CPUs listed in the cpumask file.

As Stephane suggested, make perf-stat recognize wildcard in pmu name.
So we can easily measure event on all uncore boxes of same type.
	
Any comment is appreciated.
Thank you

[RFC PATCH 1/3] perf/x86: Add cpumask for uncore pmu

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

This patch adds a cpumask file to the uncore pmu sysfs directory.
The cpumask file contains one active cpu for every socket.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c | 28 ++++++++++++++++++++++++---
 arch/x86/kernel/cpu/perf_event_intel_uncore.h |  6 ++++--
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 0a55710..62ec3e6 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2341,6 +2341,27 @@ int uncore_pmu_event_init(struct perf_event *event)
 	return ret;
 }
 
+static ssize_t uncore_get_attr_cpumask(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &uncore_cpu_mask);
+
+	buf[n++] = '\n';
+	buf[n] = '\0';
+	return n;
+}
+
+static DEVICE_ATTR(cpumask, S_IRUGO, uncore_get_attr_cpumask, NULL);
+
+static struct attribute *uncore_pmu_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group uncore_pmu_attr_group = {
+	.attrs = uncore_pmu_attrs,
+};
+
 static int __init uncore_pmu_register(struct intel_uncore_pmu *pmu)
 {
 	int ret;
@@ -2378,8 +2399,8 @@ static void __init uncore_type_exit(struct intel_uncore_type *type)
 		free_percpu(type->pmus[i].box);
 	kfree(type->pmus);
 	type->pmus = NULL;
-	kfree(type->attr_groups[1]);
-	type->attr_groups[1] = NULL;
+	kfree(type->events_group);
+	type->events_group = NULL;
 }
 
 static void __init uncore_types_exit(struct intel_uncore_type **types)
@@ -2431,9 +2452,10 @@ static int __init uncore_type_init(struct intel_uncore_type *type)
 		for (j = 0; j < i; j++)
 			attrs[j] = &type->event_descs[j].attr.attr;
 
-		type->attr_groups[1] = events_group;
+		type->events_group = events_group;
 	}
 
+	type->pmu_group = &uncore_pmu_attr_group;
 	type->pmus = pmus;
 	return 0;
 fail:
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
index 5b81c18..e68a455 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -369,10 +369,12 @@ struct intel_uncore_type {
 	struct intel_uncore_pmu *pmus;
 	struct intel_uncore_ops *ops;
 	struct uncore_event_desc *event_descs;
-	const struct attribute_group *attr_groups[3];
+	const struct attribute_group *attr_groups[4];
 };
 
-#define format_group attr_groups[0]
+#define pmu_group attr_groups[0]
+#define format_group attr_groups[1]
+#define events_group attr_groups[2]
 
 struct intel_uncore_ops {
 	void (*init_box)(struct intel_uncore_box *);
-- 
1.7.11.4

[tip:perf/core] perf/x86: Add cpumask for uncore pmu

[tip-bot for Yan, Zheng]

Commit-ID:  314d9f63f385096580e9e2a06eaa0745d92fe4ac
Gitweb:     http://git.kernel.org/tip/314d9f63f385096580e9e2a06eaa0745d92fe4ac
Author:     Yan, Zheng <zheng.z.yan@intel.com>
AuthorDate: Mon, 10 Sep 2012 15:53:49 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:11:43 -0300

perf/x86: Add cpumask for uncore pmu

This patch adds a cpumask file to the uncore pmu sysfs directory.  The
cpumask file contains one active cpu for every socket.

Signed-off-by: "Yan, Zheng" <zheng.z.yan@intel.com>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: "Yan, Zheng" <zheng.z.yan@intel.com>
Link: http://lkml.kernel.org/r/1347263631-23175-2-git-send-email-zheng.z.yan@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 arch/x86/kernel/cpu/perf_event_intel_uncore.c |   28 ++++++++++++++++++++++--
 arch/x86/kernel/cpu/perf_event_intel_uncore.h |    6 +++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.c b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
index 0a55710..62ec3e6 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.c
@@ -2341,6 +2341,27 @@ int uncore_pmu_event_init(struct perf_event *event)
 	return ret;
 }
 
+static ssize_t uncore_get_attr_cpumask(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	int n = cpulist_scnprintf(buf, PAGE_SIZE - 2, &uncore_cpu_mask);
+
+	buf[n++] = '\n';
+	buf[n] = '\0';
+	return n;
+}
+
+static DEVICE_ATTR(cpumask, S_IRUGO, uncore_get_attr_cpumask, NULL);
+
+static struct attribute *uncore_pmu_attrs[] = {
+	&dev_attr_cpumask.attr,
+	NULL,
+};
+
+static struct attribute_group uncore_pmu_attr_group = {
+	.attrs = uncore_pmu_attrs,
+};
+
 static int __init uncore_pmu_register(struct intel_uncore_pmu *pmu)
 {
 	int ret;
@@ -2378,8 +2399,8 @@ static void __init uncore_type_exit(struct intel_uncore_type *type)
 		free_percpu(type->pmus[i].box);
 	kfree(type->pmus);
 	type->pmus = NULL;
-	kfree(type->attr_groups[1]);
-	type->attr_groups[1] = NULL;
+	kfree(type->events_group);
+	type->events_group = NULL;
 }
 
 static void __init uncore_types_exit(struct intel_uncore_type **types)
@@ -2431,9 +2452,10 @@ static int __init uncore_type_init(struct intel_uncore_type *type)
 		for (j = 0; j < i; j++)
 			attrs[j] = &type->event_descs[j].attr.attr;
 
-		type->attr_groups[1] = events_group;
+		type->events_group = events_group;
 	}
 
+	type->pmu_group = &uncore_pmu_attr_group;
 	type->pmus = pmus;
 	return 0;
 fail:
diff --git a/arch/x86/kernel/cpu/perf_event_intel_uncore.h b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
index 5b81c18..e68a455 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_uncore.h
+++ b/arch/x86/kernel/cpu/perf_event_intel_uncore.h
@@ -369,10 +369,12 @@ struct intel_uncore_type {
 	struct intel_uncore_pmu *pmus;
 	struct intel_uncore_ops *ops;
 	struct uncore_event_desc *event_descs;
-	const struct attribute_group *attr_groups[3];
+	const struct attribute_group *attr_groups[4];
 };
 
-#define format_group attr_groups[0]
+#define pmu_group attr_groups[0]
+#define format_group attr_groups[1]
+#define events_group attr_groups[2]
 
 struct intel_uncore_ops {
 	void (*init_box)(struct intel_uncore_box *);

[RFC PATCH 2/3] perf tool: Make perf-stat check PMU cpumask file

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

If user doesn't explicitly specify CPU list, perf-stat only
collects events on CPUs listed in the PMU cpumask file.

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 tools/perf/builtin-stat.c      | 30 ++++++++++++++++++++----------
 tools/perf/util/cpumap.c       | 22 +++++++++++++++-------
 tools/perf/util/cpumap.h       |  2 +-
 tools/perf/util/evsel.h        |  1 +
 tools/perf/util/parse-events.c | 18 ++++++++++++++----
 tools/perf/util/pmu.c          | 30 ++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h          |  1 +
 7 files changed, 82 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d53d8ab..acdfd24 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -220,6 +220,16 @@ static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
 	evsel->priv = NULL;
 }
 
+static inline struct cpu_map *perf_evsel__cpus(struct perf_evsel *evsel)
+{
+	return (evsel->cpus && !target.cpu_list) ? evsel->cpus : evsel_list->cpus;
+}
+
+static inline int perf_evsel__nr_cpus(struct perf_evsel *evsel)
+{
+	return perf_evsel__cpus(evsel)->nr;
+}
+
 static void update_stats(struct stats *stats, u64 val)
 {
 	double delta;
@@ -295,7 +305,7 @@ retry:
 		evsel->attr.exclude_guest = evsel->attr.exclude_host = 0;
 
 	if (perf_target__has_cpu(&target)) {
-		ret = perf_evsel__open_per_cpu(evsel, evsel_list->cpus);
+		ret = perf_evsel__open_per_cpu(evsel, perf_evsel__cpus(evsel));
 		if (ret)
 			goto check_ret;
 		return 0;
@@ -376,7 +386,7 @@ static int read_counter_aggr(struct perf_evsel *counter)
 	u64 *count = counter->counts->aggr.values;
 	int i;
 
-	if (__perf_evsel__read(counter, evsel_list->cpus->nr,
+	if (__perf_evsel__read(counter, perf_evsel__nr_cpus(counter),
 			       evsel_list->threads->nr, scale) < 0)
 		return -1;
 
@@ -405,7 +415,7 @@ static int read_counter(struct perf_evsel *counter)
 	u64 *count;
 	int cpu;
 
-	for (cpu = 0; cpu < evsel_list->cpus->nr; cpu++) {
+	for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
 		if (__perf_evsel__read_on_cpu(counter, cpu, 0, scale) < 0)
 			return -1;
 
@@ -543,12 +553,12 @@ static int run_perf_stat(int argc __used, const char **argv)
 	if (no_aggr) {
 		list_for_each_entry(counter, &evsel_list->entries, node) {
 			read_counter(counter);
-			perf_evsel__close_fd(counter, evsel_list->cpus->nr, 1);
+			perf_evsel__close_fd(counter, perf_evsel__nr_cpus(counter), 1);
 		}
 	} else {
 		list_for_each_entry(counter, &evsel_list->entries, node) {
 			read_counter_aggr(counter);
-			perf_evsel__close_fd(counter, evsel_list->cpus->nr,
+			perf_evsel__close_fd(counter, perf_evsel__nr_cpus(counter),
 					     evsel_list->threads->nr);
 		}
 	}
@@ -589,7 +599,7 @@ static void nsec_printout(int cpu, struct perf_evsel *evsel, double avg)
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
-			evsel_list->cpus->map[cpu], csv_sep);
+			perf_evsel__cpus(evsel)->map[cpu], csv_sep);
 
 	fprintf(output, fmt, cpustr, msecs, csv_sep, perf_evsel__name(evsel));
 
@@ -785,7 +795,7 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
-			evsel_list->cpus->map[cpu], csv_sep);
+			perf_evsel__cpus(evsel)->map[cpu], csv_sep);
 	else
 		cpu = 0;
 
@@ -946,14 +956,14 @@ static void print_counter(struct perf_evsel *counter)
 	u64 ena, run, val;
 	int cpu;
 
-	for (cpu = 0; cpu < evsel_list->cpus->nr; cpu++) {
+	for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
 		val = counter->counts->cpu[cpu].val;
 		ena = counter->counts->cpu[cpu].ena;
 		run = counter->counts->cpu[cpu].run;
 		if (run == 0 || ena == 0) {
 			fprintf(output, "CPU%*d%s%*s%s%*s",
 				csv_output ? 0 : -4,
-				evsel_list->cpus->map[cpu], csv_sep,
+				perf_evsel__cpus(counter)->map[cpu], csv_sep,
 				csv_output ? 0 : 18,
 				counter->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED,
 				csv_sep,
@@ -1252,7 +1262,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 
 	list_for_each_entry(pos, &evsel_list->entries, node) {
 		if (perf_evsel__alloc_stat_priv(pos) < 0 ||
-		    perf_evsel__alloc_counts(pos, evsel_list->cpus->nr) < 0)
+		    perf_evsel__alloc_counts(pos, perf_evsel__nr_cpus(pos)) < 0)
 			goto out_free_fd;
 	}
 
diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index adc72f0..2b32ffa 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -38,24 +38,19 @@ static struct cpu_map *cpu_map__trim_new(int nr_cpus, int *tmp_cpus)
 	return cpus;
 }
 
-static struct cpu_map *cpu_map__read_all_cpu_map(void)
+struct cpu_map *cpu_map__read(FILE *file)
 {
 	struct cpu_map *cpus = NULL;
-	FILE *onlnf;
 	int nr_cpus = 0;
 	int *tmp_cpus = NULL, *tmp;
 	int max_entries = 0;
 	int n, cpu, prev;
 	char sep;
 
-	onlnf = fopen("/sys/devices/system/cpu/online", "r");
-	if (!onlnf)
-		return cpu_map__default_new();
-
 	sep = 0;
 	prev = -1;
 	for (;;) {
-		n = fscanf(onlnf, "%u%c", &cpu, &sep);
+		n = fscanf(file, "%u%c", &cpu, &sep);
 		if (n <= 0)
 			break;
 		if (prev >= 0) {
@@ -95,6 +90,19 @@ static struct cpu_map *cpu_map__read_all_cpu_map(void)
 		cpus = cpu_map__default_new();
 out_free_tmp:
 	free(tmp_cpus);
+	return cpus;
+}
+
+static struct cpu_map *cpu_map__read_all_cpu_map(void)
+{
+	struct cpu_map *cpus = NULL;
+	FILE *onlnf;
+
+	onlnf = fopen("/sys/devices/system/cpu/online", "r");
+	if (!onlnf)
+		return cpu_map__default_new();
+
+	cpus = cpu_map__read(onlnf);
 	fclose(onlnf);
 	return cpus;
 }
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index c415185..17b5264 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -11,7 +11,7 @@ struct cpu_map {
 struct cpu_map *cpu_map__new(const char *cpu_list);
 struct cpu_map *cpu_map__dummy_new(void);
 void cpu_map__delete(struct cpu_map *map);
-
+struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
 
 #endif /* __PERF_CPUMAP_H */
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 94f6ba1..956c0a0 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -66,6 +66,7 @@ struct perf_evsel {
 		void		*func;
 		void		*data;
 	} handler;
+	struct cpu_map		*cpus;
 	unsigned int		sample_size;
 	bool 			supported;
 	/* parse modifier helper */
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index b246303..3aa9be4 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -239,8 +239,11 @@ const char *event_type(int type)
 	return "unknown";
 }
 
-static int add_event(struct list_head **_list, int *idx,
-		     struct perf_event_attr *attr, char *name)
+
+
+static int __add_event(struct list_head **_list, int *idx,
+		       struct perf_event_attr *attr,
+		       char *name, struct cpu_map *cpus)
 {
 	struct perf_evsel *evsel;
 	struct list_head *list = *_list;
@@ -260,6 +263,7 @@ static int add_event(struct list_head **_list, int *idx,
 		return -ENOMEM;
 	}
 
+	evsel->cpus = cpus;
 	if (name)
 		evsel->name = strdup(name);
 	list_add_tail(&evsel->node, list);
@@ -267,6 +271,12 @@ static int add_event(struct list_head **_list, int *idx,
 	return 0;
 }
 
+static int add_event(struct list_head **_list, int *idx,
+		     struct perf_event_attr *attr, char *name)
+{
+	return __add_event(_list, idx, attr, name, NULL);
+}
+
 static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
 {
 	int i, j;
@@ -607,8 +617,8 @@ int parse_events_add_pmu(struct list_head **list, int *idx,
 	if (perf_pmu__config(pmu, &attr, head_config))
 		return -EINVAL;
 
-	return add_event(list, idx, &attr,
-			 pmu_event_name(head_config));
+	return __add_event(list, idx, &attr, pmu_event_name(head_config),
+			   pmu->cpus);
 }
 
 int parse_events__modifier_group(struct list_head *list,
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6631d82..8a2229d 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -9,6 +9,7 @@
 #include "util.h"
 #include "pmu.h"
 #include "parse-events.h"
+#include "cpumap.h"
 
 #define EVENT_SOURCE_DEVICE_PATH "/bus/event_source/devices/"
 
@@ -253,6 +254,33 @@ static void pmu_read_sysfs(void)
 	closedir(dir);
 }
 
+static struct cpu_map *pmu_cpumask(char *name)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	const char *sysfs;
+	FILE *file;
+	struct cpu_map *cpus;
+
+	sysfs = sysfs_find_mountpoint();
+	if (!sysfs)
+		return NULL;
+
+	snprintf(path, PATH_MAX,
+		 "%s/bus/event_source/devices/%s/cpumask", sysfs, name);
+
+	if (stat(path, &st) < 0)
+		return NULL;
+
+	file = fopen(path, "r");
+	if (!file)
+		return NULL;
+
+	cpus = cpu_map__read(file);
+	fclose(file);
+	return cpus;
+}
+
 static struct perf_pmu *pmu_lookup(char *name)
 {
 	struct perf_pmu *pmu;
@@ -275,6 +303,8 @@ static struct perf_pmu *pmu_lookup(char *name)
 	if (!pmu)
 		return NULL;
 
+	pmu->cpus = pmu_cpumask(name);
+
 	pmu_aliases(name, &aliases);
 
 	INIT_LIST_HEAD(&pmu->format);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 47f68d3..53c7794 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -28,6 +28,7 @@ struct perf_pmu__alias {
 struct perf_pmu {
 	char *name;
 	__u32 type;
+	struct cpu_map *cpus;
 	struct list_head format;
 	struct list_head aliases;
 	struct list_head list;
-- 
1.7.11.4

[tip:perf/core] perf stat: Check PMU cpumask file

[tip-bot for Yan, Zheng]

Commit-ID:  7ae92e744e3fb389afb1e24920ecda331d360c61
Gitweb:     http://git.kernel.org/tip/7ae92e744e3fb389afb1e24920ecda331d360c61
Author:     Yan, Zheng <zheng.z.yan@intel.com>
AuthorDate: Mon, 10 Sep 2012 15:53:50 +0800
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:12:02 -0300

perf stat: Check PMU cpumask file

If user doesn't explicitly specify CPU list, perf-stat only collects
events on CPUs listed in the PMU cpumask file.

Signed-off-by: "Yah, Zheng" <zheng.z.yan@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1347263631-23175-3-git-send-email-zheng.z.yan@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c      |   30 ++++++++++++++++++++----------
 tools/perf/util/cpumap.c       |   22 +++++++++++++++-------
 tools/perf/util/cpumap.h       |    2 +-
 tools/perf/util/evsel.h        |    1 +
 tools/perf/util/parse-events.c |   18 ++++++++++++++----
 tools/perf/util/pmu.c          |   30 ++++++++++++++++++++++++++++++
 tools/perf/util/pmu.h          |    1 +
 7 files changed, 82 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 3c43a35..e0f65fe 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -215,6 +215,16 @@ static void perf_evsel__free_stat_priv(struct perf_evsel *evsel)
 	evsel->priv = NULL;
 }
 
+static inline struct cpu_map *perf_evsel__cpus(struct perf_evsel *evsel)
+{
+	return (evsel->cpus && !target.cpu_list) ? evsel->cpus : evsel_list->cpus;
+}
+
+static inline int perf_evsel__nr_cpus(struct perf_evsel *evsel)
+{
+	return perf_evsel__cpus(evsel)->nr;
+}
+
 static struct stats runtime_nsecs_stats[MAX_NR_CPUS];
 static struct stats runtime_cycles_stats[MAX_NR_CPUS];
 static struct stats runtime_stalled_cycles_front_stats[MAX_NR_CPUS];
@@ -246,7 +256,7 @@ retry:
 		evsel->attr.exclude_guest = evsel->attr.exclude_host = 0;
 
 	if (perf_target__has_cpu(&target)) {
-		ret = perf_evsel__open_per_cpu(evsel, evsel_list->cpus);
+		ret = perf_evsel__open_per_cpu(evsel, perf_evsel__cpus(evsel));
 		if (ret)
 			goto check_ret;
 		return 0;
@@ -327,7 +337,7 @@ static int read_counter_aggr(struct perf_evsel *counter)
 	u64 *count = counter->counts->aggr.values;
 	int i;
 
-	if (__perf_evsel__read(counter, evsel_list->cpus->nr,
+	if (__perf_evsel__read(counter, perf_evsel__nr_cpus(counter),
 			       evsel_list->threads->nr, scale) < 0)
 		return -1;
 
@@ -356,7 +366,7 @@ static int read_counter(struct perf_evsel *counter)
 	u64 *count;
 	int cpu;
 
-	for (cpu = 0; cpu < evsel_list->cpus->nr; cpu++) {
+	for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
 		if (__perf_evsel__read_on_cpu(counter, cpu, 0, scale) < 0)
 			return -1;
 
@@ -495,12 +505,12 @@ static int run_perf_stat(int argc __maybe_unused, const char **argv)
 	if (no_aggr) {
 		list_for_each_entry(counter, &evsel_list->entries, node) {
 			read_counter(counter);
-			perf_evsel__close_fd(counter, evsel_list->cpus->nr, 1);
+			perf_evsel__close_fd(counter, perf_evsel__nr_cpus(counter), 1);
 		}
 	} else {
 		list_for_each_entry(counter, &evsel_list->entries, node) {
 			read_counter_aggr(counter);
-			perf_evsel__close_fd(counter, evsel_list->cpus->nr,
+			perf_evsel__close_fd(counter, perf_evsel__nr_cpus(counter),
 					     evsel_list->threads->nr);
 		}
 	}
@@ -538,7 +548,7 @@ static void nsec_printout(int cpu, struct perf_evsel *evsel, double avg)
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
-			evsel_list->cpus->map[cpu], csv_sep);
+			perf_evsel__cpus(evsel)->map[cpu], csv_sep);
 
 	fprintf(output, fmt, cpustr, msecs, csv_sep, perf_evsel__name(evsel));
 
@@ -750,7 +760,7 @@ static void abs_printout(int cpu, struct perf_evsel *evsel, double avg)
 	if (no_aggr)
 		sprintf(cpustr, "CPU%*d%s",
 			csv_output ? 0 : -4,
-			evsel_list->cpus->map[cpu], csv_sep);
+			perf_evsel__cpus(evsel)->map[cpu], csv_sep);
 	else
 		cpu = 0;
 
@@ -911,14 +921,14 @@ static void print_counter(struct perf_evsel *counter)
 	u64 ena, run, val;
 	int cpu;
 
-	for (cpu = 0; cpu < evsel_list->cpus->nr; cpu++) {
+	for (cpu = 0; cpu < perf_evsel__nr_cpus(counter); cpu++) {
 		val = counter->counts->cpu[cpu].val;
 		ena = counter->counts->cpu[cpu].ena;
 		run = counter->counts->cpu[cpu].run;
 		if (run == 0 || ena == 0) {
 			fprintf(output, "CPU%*d%s%*s%s%*s",
 				csv_output ? 0 : -4,
-				evsel_list->cpus->map[cpu], csv_sep,
+				perf_evsel__cpus(counter)->map[cpu], csv_sep,
 				csv_output ? 0 : 18,
 				counter->supported ? CNTR_NOT_COUNTED : CNTR_NOT_SUPPORTED,
 				csv_sep,
@@ -1217,7 +1227,7 @@ int cmd_stat(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	list_for_each_entry(pos, &evsel_list->entries, node) {
 		if (perf_evsel__alloc_stat_priv(pos) < 0 ||
-		    perf_evsel__alloc_counts(pos, evsel_list->cpus->nr) < 0)
+		    perf_evsel__alloc_counts(pos, perf_evsel__nr_cpus(pos)) < 0)
 			goto out_free_fd;
 	}
 
diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index adc72f0..2b32ffa 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -38,24 +38,19 @@ static struct cpu_map *cpu_map__trim_new(int nr_cpus, int *tmp_cpus)
 	return cpus;
 }
 
-static struct cpu_map *cpu_map__read_all_cpu_map(void)
+struct cpu_map *cpu_map__read(FILE *file)
 {
 	struct cpu_map *cpus = NULL;
-	FILE *onlnf;
 	int nr_cpus = 0;
 	int *tmp_cpus = NULL, *tmp;
 	int max_entries = 0;
 	int n, cpu, prev;
 	char sep;
 
-	onlnf = fopen("/sys/devices/system/cpu/online", "r");
-	if (!onlnf)
-		return cpu_map__default_new();
-
 	sep = 0;
 	prev = -1;
 	for (;;) {
-		n = fscanf(onlnf, "%u%c", &cpu, &sep);
+		n = fscanf(file, "%u%c", &cpu, &sep);
 		if (n <= 0)
 			break;
 		if (prev >= 0) {
@@ -95,6 +90,19 @@ static struct cpu_map *cpu_map__read_all_cpu_map(void)
 		cpus = cpu_map__default_new();
 out_free_tmp:
 	free(tmp_cpus);
+	return cpus;
+}
+
+static struct cpu_map *cpu_map__read_all_cpu_map(void)
+{
+	struct cpu_map *cpus = NULL;
+	FILE *onlnf;
+
+	onlnf = fopen("/sys/devices/system/cpu/online", "r");
+	if (!onlnf)
+		return cpu_map__default_new();
+
+	cpus = cpu_map__read(onlnf);
 	fclose(onlnf);
 	return cpus;
 }
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index c415185..17b5264 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -11,7 +11,7 @@ struct cpu_map {
 struct cpu_map *cpu_map__new(const char *cpu_list);
 struct cpu_map *cpu_map__dummy_new(void);
 void cpu_map__delete(struct cpu_map *map);
-
+struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
 
 #endif /* __PERF_CPUMAP_H */
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index dc40fe3..93876ba 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -66,6 +66,7 @@ struct perf_evsel {
 		void		*func;
 		void		*data;
 	} handler;
+	struct cpu_map		*cpus;
 	unsigned int		sample_size;
 	bool 			supported;
 	/* parse modifier helper */
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 44afcf4..bf5d033 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -239,8 +239,11 @@ const char *event_type(int type)
 	return "unknown";
 }
 
-static int add_event(struct list_head **_list, int *idx,
-		     struct perf_event_attr *attr, char *name)
+
+
+static int __add_event(struct list_head **_list, int *idx,
+		       struct perf_event_attr *attr,
+		       char *name, struct cpu_map *cpus)
 {
 	struct perf_evsel *evsel;
 	struct list_head *list = *_list;
@@ -260,6 +263,7 @@ static int add_event(struct list_head **_list, int *idx,
 		return -ENOMEM;
 	}
 
+	evsel->cpus = cpus;
 	if (name)
 		evsel->name = strdup(name);
 	list_add_tail(&evsel->node, list);
@@ -267,6 +271,12 @@ static int add_event(struct list_head **_list, int *idx,
 	return 0;
 }
 
+static int add_event(struct list_head **_list, int *idx,
+		     struct perf_event_attr *attr, char *name)
+{
+	return __add_event(_list, idx, attr, name, NULL);
+}
+
 static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
 {
 	int i, j;
@@ -607,8 +617,8 @@ int parse_events_add_pmu(struct list_head **list, int *idx,
 	if (perf_pmu__config(pmu, &attr, head_config))
 		return -EINVAL;
 
-	return add_event(list, idx, &attr,
-			 pmu_event_name(head_config));
+	return __add_event(list, idx, &attr, pmu_event_name(head_config),
+			   pmu->cpus);
 }
 
 int parse_events__modifier_group(struct list_head *list,
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6631d82..8a2229d 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -9,6 +9,7 @@
 #include "util.h"
 #include "pmu.h"
 #include "parse-events.h"
+#include "cpumap.h"
 
 #define EVENT_SOURCE_DEVICE_PATH "/bus/event_source/devices/"
 
@@ -253,6 +254,33 @@ static void pmu_read_sysfs(void)
 	closedir(dir);
 }
 
+static struct cpu_map *pmu_cpumask(char *name)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	const char *sysfs;
+	FILE *file;
+	struct cpu_map *cpus;
+
+	sysfs = sysfs_find_mountpoint();
+	if (!sysfs)
+		return NULL;
+
+	snprintf(path, PATH_MAX,
+		 "%s/bus/event_source/devices/%s/cpumask", sysfs, name);
+
+	if (stat(path, &st) < 0)
+		return NULL;
+
+	file = fopen(path, "r");
+	if (!file)
+		return NULL;
+
+	cpus = cpu_map__read(file);
+	fclose(file);
+	return cpus;
+}
+
 static struct perf_pmu *pmu_lookup(char *name)
 {
 	struct perf_pmu *pmu;
@@ -275,6 +303,8 @@ static struct perf_pmu *pmu_lookup(char *name)
 	if (!pmu)
 		return NULL;
 
+	pmu->cpus = pmu_cpumask(name);
+
 	pmu_aliases(name, &aliases);
 
 	INIT_LIST_HEAD(&pmu->format);
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 47f68d3..53c7794 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -28,6 +28,7 @@ struct perf_pmu__alias {
 struct perf_pmu {
 	char *name;
 	__u32 type;
+	struct cpu_map *cpus;
 	struct list_head format;
 	struct list_head aliases;
 	struct list_head list;

[RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Yan, Zheng]

From: "Yan, Zheng" <zheng.z.yan@intel.com>

Allow wildcard in PMU name, so we can measure events on all
uncore boxes of same type. For example:

Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>
---
 tools/perf/builtin-stat.c      |   8 ++-
 tools/perf/util/evsel.c        |  31 ++++++++--
 tools/perf/util/evsel.h        |   2 +
 tools/perf/util/parse-events.c | 126 ++++++++++++++++++++++++++++++-----------
 4 files changed, 128 insertions(+), 39 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index acdfd24..bebfd8c 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -557,7 +557,8 @@ static int run_perf_stat(int argc __used, const char **argv)
 		}
 	} else {
 		list_for_each_entry(counter, &evsel_list->entries, node) {
-			read_counter_aggr(counter);
+			if (!counter->aggr_slave)
+				read_counter_aggr(counter);
 			perf_evsel__close_fd(counter, perf_evsel__nr_cpus(counter),
 					     evsel_list->threads->nr);
 		}
@@ -1023,8 +1024,11 @@ static void print_stat(int argc, const char **argv)
 		list_for_each_entry(counter, &evsel_list->entries, node)
 			print_counter(counter);
 	} else {
-		list_for_each_entry(counter, &evsel_list->entries, node)
+		list_for_each_entry(counter, &evsel_list->entries, node) {
+			if (counter->aggr_slave)
+				continue;
 			print_counter_aggr(counter);
+		}
 	}
 
 	if (!csv_output) {
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 7ff3c8f..b99b892 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -54,6 +54,7 @@ void perf_evsel__init(struct perf_evsel *evsel,
 	evsel->idx	   = idx;
 	evsel->attr	   = *attr;
 	INIT_LIST_HEAD(&evsel->node);
+	INIT_LIST_HEAD(&evsel->sibling);
 	hists__init(&evsel->hists);
 	evsel->sample_size = __perf_evsel__sample_size(attr->sample_type);
 }
@@ -525,14 +526,13 @@ int __perf_evsel__read_on_cpu(struct perf_evsel *evsel,
 	return 0;
 }
 
-int __perf_evsel__read(struct perf_evsel *evsel,
-		       int ncpus, int nthreads, bool scale)
+static int read_evsel(struct perf_counts_values *aggr,
+		      struct perf_evsel *evsel,
+		      int ncpus, int nthreads, bool scale)
 {
+	struct perf_counts_values count;
 	size_t nv = scale ? 3 : 1;
 	int cpu, thread;
-	struct perf_counts_values *aggr = &evsel->counts->aggr, count;
-
-	aggr->val = aggr->ena = aggr->run = 0;
 
 	for (cpu = 0; cpu < ncpus; cpu++) {
 		for (thread = 0; thread < nthreads; thread++) {
@@ -550,6 +550,27 @@ int __perf_evsel__read(struct perf_evsel *evsel,
 			}
 		}
 	}
+	return 0;
+}
+
+int __perf_evsel__read(struct perf_evsel *evsel,
+		       int ncpus, int nthreads, bool scale)
+{
+	int ret;
+	struct perf_counts_values *aggr = &evsel->counts->aggr;
+	struct perf_evsel *child;
+
+	aggr->val = aggr->ena = aggr->run = 0;
+
+	ret = read_evsel(aggr, evsel, ncpus, nthreads, scale);
+	if (ret)
+		return ret;
+
+	list_for_each_entry(child, &evsel->sibling, node) {
+		ret = read_evsel(aggr, evsel, ncpus, nthreads, scale);
+		if (ret)
+			return ret;
+	}
 
 	evsel->counts->scaled = 0;
 	if (scale) {
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 956c0a0..bcb5a8a 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -46,6 +46,7 @@ struct perf_sample_id {
  */
 struct perf_evsel {
 	struct list_head	node;
+	struct list_head	sibling;
 	struct perf_event_attr	attr;
 	char			*filter;
 	struct xyarray		*fd;
@@ -69,6 +70,7 @@ struct perf_evsel {
 	struct cpu_map		*cpus;
 	unsigned int		sample_size;
 	bool 			supported;
+	bool			aggr_slave;
 	/* parse modifier helper */
 	int			exclude_GH;
 	struct perf_evsel	*leader;
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3aa9be4..32288f2 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -239,14 +239,40 @@ const char *event_type(int type)
 	return "unknown";
 }
 
+static void free_event_list(struct list_head *list)
+{
+	struct perf_evsel *evsel;
 
+	while (!list_empty(list)) {
+		evsel = list_entry(list->next, struct perf_evsel, node);
+		list_del(&evsel->node);
+		perf_evsel__delete(evsel);
+	}
+	free(list);
+}
 
-static int __add_event(struct list_head **_list, int *idx,
-		       struct perf_event_attr *attr,
-		       char *name, struct cpu_map *cpus)
+static struct perf_evsel *__add_event(int *idx, struct perf_event_attr *attr,
+				      char *name, struct cpu_map *cpus)
 {
 	struct perf_evsel *evsel;
+
+	event_attr_init(attr);
+
+	evsel = perf_evsel__new(attr, (*idx)++);
+	if (!evsel)
+		return NULL;
+
+	evsel->cpus = cpus;
+	if (name)
+		evsel->name = strdup(name);
+	return evsel;
+}
+
+static int add_event(struct list_head **_list, int *idx,
+		     struct perf_event_attr *attr, char *name)
+{
 	struct list_head *list = *_list;
+	struct perf_evsel *evsel;
 
 	if (!list) {
 		list = malloc(sizeof(*list));
@@ -255,28 +281,17 @@ static int __add_event(struct list_head **_list, int *idx,
 		INIT_LIST_HEAD(list);
 	}
 
-	event_attr_init(attr);
-
-	evsel = perf_evsel__new(attr, (*idx)++);
+	evsel = __add_event(idx, attr, name, NULL);
 	if (!evsel) {
 		free(list);
 		return -ENOMEM;
 	}
 
-	evsel->cpus = cpus;
-	if (name)
-		evsel->name = strdup(name);
 	list_add_tail(&evsel->node, list);
 	*_list = list;
 	return 0;
 }
 
-static int add_event(struct list_head **_list, int *idx,
-		     struct perf_event_attr *attr, char *name)
-{
-	return __add_event(_list, idx, attr, name, NULL);
-}
-
 static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
 {
 	int i, j;
@@ -593,32 +608,79 @@ static char *pmu_event_name(struct list_head *head_terms)
 	return NULL;
 }
 
-int parse_events_add_pmu(struct list_head **list, int *idx,
+static int pmu_name_match(char *name, char *pattern)
+{
+	int idx;
+	char *c;
+
+	if (!strcasecmp(name, pattern))
+		return 0;
+
+	c = strchr(pattern, '*');
+	if (!c || c[1] != '\0')
+		return 1;
+
+	idx = c - pattern;
+	if (strncasecmp(name, pattern, idx))
+		return 1;
+
+	for (; name[idx]; idx++) {
+		if (name[idx] < '0' || name[idx] > '9')
+			return 1;
+	}
+	return 0;
+}
+
+int parse_events_add_pmu(struct list_head **_list, int *idx,
 			 char *name, struct list_head *head_config)
 {
 	struct perf_event_attr attr;
-	struct perf_pmu *pmu;
+	struct list_head *list;
+	struct perf_pmu *pmu = NULL;
+	struct perf_evsel *evsel, *first = NULL;
+	int orig_idx = *idx;
 
-	pmu = perf_pmu__find(name);
-	if (!pmu)
-		return -EINVAL;
+	list = malloc(sizeof(*list));
+	if (!list)
+		return -ENOMEM;
+	INIT_LIST_HEAD(list);
 
-	memset(&attr, 0, sizeof(attr));
+	while ((pmu = perf_pmu__scan(pmu))) {
+		if (pmu_name_match(pmu->name, name))
+			continue;
 
-	if (perf_pmu__check_alias(pmu, head_config))
-		return -EINVAL;
+		memset(&attr, 0, sizeof(attr));
 
-	/*
-	 * Configure hardcoded terms first, no need to check
-	 * return value when called with fail == 0 ;)
-	 */
-	config_attr(&attr, head_config, 0);
+		if (perf_pmu__check_alias(pmu, head_config))
+			return -EINVAL;
 
-	if (perf_pmu__config(pmu, &attr, head_config))
-		return -EINVAL;
+		/*
+		 * Configure hardcoded terms first, no need to check
+		 * return value when called with fail == 0 ;)
+		 */
+		config_attr(&attr, head_config, 0);
+
+		if (perf_pmu__config(pmu, &attr, head_config))
+			return -EINVAL;
 
-	return __add_event(list, idx, &attr, pmu_event_name(head_config),
-			   pmu->cpus);
+		evsel = __add_event(idx, &attr, pmu_event_name(head_config),
+				    pmu->cpus);
+		if (!evsel) {
+			*idx = orig_idx;
+			free_event_list(list);
+			return -ENOMEM;
+		}
+		list_add_tail(&evsel->node, list);
+		if (first) {
+			list_add_tail(&evsel->sibling, &first->sibling);
+			evsel->aggr_slave = true;
+		} else {
+			first = evsel;
+		}
+	}
+
+	*_list = list;
+	return 0;
 }
 
 int parse_events__modifier_group(struct list_head *list,
-- 
1.7.11.4

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Jiri Olsa]

On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 
> Allow wildcard in PMU name, so we can measure events on all
> uncore boxes of same type. For example:
> 
> Signed-off-by: Yan, Zheng <zheng.z.yan@intel.com>

example is missing ;)

jirka

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Jiri Olsa]

On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> 

SNIP

> +int parse_events_add_pmu(struct list_head **_list, int *idx,
>  			 char *name, struct list_head *head_config)
>  {
>  	struct perf_event_attr attr;
> -	struct perf_pmu *pmu;
> +	struct list_head *list;
> +	struct perf_pmu *pmu = NULL;
> +	struct perf_evsel *evsel, *first = NULL;
> +	int orig_idx = *idx;
>  
> -	pmu = perf_pmu__find(name);
> -	if (!pmu)
> -		return -EINVAL;
> +	list = malloc(sizeof(*list));
> +	if (!list)
> +		return -ENOMEM;
> +	INIT_LIST_HEAD(list);

list should be allocated only if (!*_list)) same as in add_event function

I haven't test, but I think you'll leak/loose events if there's another pmu
event defined after ','

jirka

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Yan, Zheng]

On 09/11/2012 10:05 PM, Jiri Olsa wrote:
> On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
>> From: "Yan, Zheng" <zheng.z.yan@intel.com>
>>
> 
> SNIP
> 
>> +int parse_events_add_pmu(struct list_head **_list, int *idx,
>>  			 char *name, struct list_head *head_config)
>>  {
>>  	struct perf_event_attr attr;
>> -	struct perf_pmu *pmu;
>> +	struct list_head *list;
>> +	struct perf_pmu *pmu = NULL;
>> +	struct perf_evsel *evsel, *first = NULL;
>> +	int orig_idx = *idx;
>>  
>> -	pmu = perf_pmu__find(name);
>> -	if (!pmu)
>> -		return -EINVAL;
>> +	list = malloc(sizeof(*list));
>> +	if (!list)
>> +		return -ENOMEM;
>> +	INIT_LIST_HEAD(list);
> 
> list should be allocated only if (!*_list)) same as in add_event function
> 
> I haven't test, but I think you'll leak/loose events if there's another pmu
> event defined after ','
> 

I think *_list is always NULL, because the code in parse-event.y is:

---
PE_NAME '/' event_config '/'
{
	struct parse_events_data__events *data = _data;
	struct list_head *list = NULL;

	ABORT_ON(parse_events_add_pmu(&list, &data->idx, $1, $3));
	parse_events__free_terms($3);
	$$ = list;
}

---

Regards
Yan, Zheng

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Jiri Olsa]

On Wed, Sep 12, 2012 at 09:30:19AM +0800, Yan, Zheng wrote:
> On 09/11/2012 10:05 PM, Jiri Olsa wrote:
> > On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
> >> From: "Yan, Zheng" <zheng.z.yan@intel.com>
> >>
> > 
> > SNIP
> > 
> >> +int parse_events_add_pmu(struct list_head **_list, int *idx,
> >>  			 char *name, struct list_head *head_config)
> >>  {
> >>  	struct perf_event_attr attr;
> >> -	struct perf_pmu *pmu;
> >> +	struct list_head *list;
> >> +	struct perf_pmu *pmu = NULL;
> >> +	struct perf_evsel *evsel, *first = NULL;
> >> +	int orig_idx = *idx;
> >>  
> >> -	pmu = perf_pmu__find(name);
> >> -	if (!pmu)
> >> -		return -EINVAL;
> >> +	list = malloc(sizeof(*list));
> >> +	if (!list)
> >> +		return -ENOMEM;
> >> +	INIT_LIST_HEAD(list);
> > 
> > list should be allocated only if (!*_list)) same as in add_event function
> > 
> > I haven't test, but I think you'll leak/loose events if there's another pmu
> > event defined after ','
> > 
> 
> I think *_list is always NULL, because the code in parse-event.y is:
> 
> ---
> PE_NAME '/' event_config '/'
> {
> 	struct parse_events_data__events *data = _data;
> 	struct list_head *list = NULL;
> 
> 	ABORT_ON(parse_events_add_pmu(&list, &data->idx, $1, $3));
> 	parse_events__free_terms($3);
> 	$$ = list;
> }
> 
> ---
> 
> Regards
> Yan, Zheng

ouch, we update it in parse_events_update_lists.. then the '!list' check
is not needed.. I haven't realized we changed that, sry.

jirka

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Jiri Olsa]

On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
> From: "Yan, Zheng" <zheng.z.yan@intel.com>

SNIP

> +{
> +	struct perf_evsel *evsel;
>  
> +	while (!list_empty(list)) {
> +		evsel = list_entry(list->next, struct perf_evsel, node);
> +		list_del(&evsel->node);
> +		perf_evsel__delete(evsel);
> +	}
> +	free(list);
> +}
>  
> -static int __add_event(struct list_head **_list, int *idx,
> -		       struct perf_event_attr *attr,
> -		       char *name, struct cpu_map *cpus)
> +static struct perf_evsel *__add_event(int *idx, struct perf_event_attr *attr,
> +				      char *name, struct cpu_map *cpus)
>  {
>  	struct perf_evsel *evsel;
> +
> +	event_attr_init(attr);
> +
> +	evsel = perf_evsel__new(attr, (*idx)++);
> +	if (!evsel)
> +		return NULL;
> +
> +	evsel->cpus = cpus;
> +	if (name)
> +		evsel->name = strdup(name);
> +	return evsel;
> +}
> +
> +static int add_event(struct list_head **_list, int *idx,
> +		     struct perf_event_attr *attr, char *name)
> +{
>  	struct list_head *list = *_list;
> +	struct perf_evsel *evsel;
>  
>  	if (!list) {
>  		list = malloc(sizeof(*list));
> @@ -255,28 +281,17 @@ static int __add_event(struct list_head **_list, int *idx,
>  		INIT_LIST_HEAD(list);
>  	}
>  
> -	event_attr_init(attr);
> -
> -	evsel = perf_evsel__new(attr, (*idx)++);
> +	evsel = __add_event(idx, attr, name, NULL);
>  	if (!evsel) {
>  		free(list);
>  		return -ENOMEM;
>  	}
>  
> -	evsel->cpus = cpus;
> -	if (name)
> -		evsel->name = strdup(name);
>  	list_add_tail(&evsel->node, list);
>  	*_list = list;
>  	return 0;
>  }
>  
> -static int add_event(struct list_head **_list, int *idx,
> -		     struct perf_event_attr *attr, char *name)
> -{
> -	return __add_event(_list, idx, attr, name, NULL);
> -}
> -

Would it be possible to have all '*add_event' more obvious for usage.
Also following code is duplicated after each call of __add_event:

      evsel = __add_event(idx, &attr, pmu_event_name(head_config),
                          pmu->cpus);
      if (!evsel) {
              *idx = orig_idx;
              free_event_list(list);
              return -ENOMEM;
      }
      list_add_tail(&evsel->node, list);

I tried some changes over your patch.. just compiled, not tested ;)
It should also solve the issue from my other comment.

thanks,
jirka


---
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 1b0a46f..f6bbd7a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -268,8 +268,10 @@ static struct perf_evsel *__add_event(int *idx, struct perf_event_attr *attr,
 	return evsel;
 }
 
-static int add_event(struct list_head **_list, int *idx,
-		     struct perf_event_attr *attr, char *name)
+static struct perf_evsel*
+add_event_list(struct list_head **_list, int *idx,
+	       struct perf_event_attr *attr, char *name,
+	       struct cpu_map *cpus)
 {
 	struct list_head *list = *_list;
 	struct perf_evsel *evsel;
@@ -277,19 +279,27 @@ static int add_event(struct list_head **_list, int *idx,
 	if (!list) {
 		list = malloc(sizeof(*list));
 		if (!list)
-			return -ENOMEM;
+			return NULL;
 		INIT_LIST_HEAD(list);
 	}
 
-	evsel = __add_event(idx, attr, name, NULL);
+	evsel = __add_event(idx, attr, name, cpus);
 	if (!evsel) {
-		free(list);
-		return -ENOMEM;
+		free_event_list(list);
+		return NULL;
 	}
 
 	list_add_tail(&evsel->node, list);
-	*_list = list;
-	return 0;
+	if (!*_list)
+		*_list = list;
+
+	return evsel;
+}
+
+static int add_event(struct list_head **_list, int *idx,
+		     struct perf_event_attr *attr, char *name)
+{
+	return add_event_list(_list, idx, attr, name, NULL) ? 0 : -ENOMEM;
 }
 
 static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)
@@ -635,16 +645,10 @@ int parse_events_add_pmu(struct list_head **_list, int *idx,
 			 char *name, struct list_head *head_config)
 {
 	struct perf_event_attr attr;
-	struct list_head *list;
 	struct perf_pmu *pmu = NULL;
 	struct perf_evsel *evsel, *first = NULL;
 	int orig_idx = *idx;
 
-	list = malloc(sizeof(*list));
-	if (!list)
-		return -ENOMEM;
-	INIT_LIST_HEAD(list);
-
 	while ((pmu = perf_pmu__scan(pmu))) {
 		if (pmu_name_match(pmu->name, name))
 			continue;
@@ -663,14 +667,12 @@ int parse_events_add_pmu(struct list_head **_list, int *idx,
 		if (perf_pmu__config(pmu, &attr, head_config))
 			return -EINVAL;
 
-		evsel = __add_event(idx, &attr, pmu_event_name(head_config),
-				    pmu->cpus);
+		evsel = add_event_list(_list, idx, &attr, pmu_event_name(head_config),
+				       pmu->cpus);
 		if (!evsel) {
 			*idx = orig_idx;
-			free_event_list(list);
 			return -ENOMEM;
 		}
-		list_add_tail(&evsel->node, list);
 		if (first) {
 			list_add_tail(&evsel->sibling, &first->sibling);
 			evsel->aggr_slave = true;
@@ -679,7 +681,6 @@ int parse_events_add_pmu(struct list_head **_list, int *idx,
 		}
 	}
 
-	*_list = list;
 	return 0;
 }
 

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Arnaldo Carvalho de Melo]

Em Tue, Sep 11, 2012 at 04:27:17PM +0200, Jiri Olsa escreveu:
> On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
> 
> Would it be possible to have all '*add_event' more obvious for usage.
> Also following code is duplicated after each call of __add_event:
> 
>       evsel = __add_event(idx, &attr, pmu_event_name(head_config),
>                           pmu->cpus);
>       if (!evsel) {
>               *idx = orig_idx;
>               free_event_list(list);
>               return -ENOMEM;
>       }
>       list_add_tail(&evsel->node, list);

Any update here? Yan, did you look at folding Jiri's patch? I'm merging
the first bit, the kernel part, that was Acked by Ingo and Peter,
waiting on this part, looking at 2/3 now

- Arnaldo

Re: [RFC PATCH 3/3] perf tool: Allow wildcard in PMU name

[Yan, Zheng]

On 09/17/2012 11:36 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Sep 11, 2012 at 04:27:17PM +0200, Jiri Olsa escreveu:
>> On Mon, Sep 10, 2012 at 03:53:51PM +0800, Yan, Zheng wrote:
>>
>> Would it be possible to have all '*add_event' more obvious for usage.
>> Also following code is duplicated after each call of __add_event:
>>
>>       evsel = __add_event(idx, &attr, pmu_event_name(head_config),
>>                           pmu->cpus);
>>       if (!evsel) {
>>               *idx = orig_idx;
>>               free_event_list(list);
>>               return -ENOMEM;
>>       }
>>       list_add_tail(&evsel->node, list);
> 
> Any update here? Yan, did you look at folding Jiri's patch? I'm merging
> the first bit, the kernel part, that was Acked by Ingo and Peter,
> waiting on this part, looking at 2/3 now
> 

Hi,

This patch still need some improvement, I don't expect it to get merged.
Please merged the patch 1 and patch 2 first. I will send a new one soon.

Regards
Yan, Zheng

Re: [RFC PATCH 1/3] perf: Add cpumask for uncore pmu

[Peter Zijlstra]

On Mon, 2012-09-10 at 15:53 +0800, Yan, Zheng wrote:
> Hi,
> 
> This patchset add a cpumask file to the uncore pmu sysfs directory.
> If user doesn't explicitly specify CPU list, perf-stat only collects
> uncore events on CPUs listed in the cpumask file.
> 
> As Stephane suggested, make perf-stat recognize wildcard in pmu name.
> So we can easily measure event on all uncore boxes of same type.
> 	
> Any comment is appreciated.

You forgot to CC acme who maintains the tools side.. Anyway, I think it
all looks ok, Arnaldo see anything dubious in there?

Re: [RFC PATCH 1/3] perf: Add cpumask for uncore pmu

[Arnaldo Carvalho de Melo]

Em Mon, Sep 10, 2012 at 06:34:26PM +0200, Peter Zijlstra escreveu:
> On Mon, 2012-09-10 at 15:53 +0800, Yan, Zheng wrote:
> > This patchset add a cpumask file to the uncore pmu sysfs directory.
> > If user doesn't explicitly specify CPU list, perf-stat only collects
> > uncore events on CPUs listed in the cpumask file.

> > As Stephane suggested, make perf-stat recognize wildcard in pmu name.
> > So we can easily measure event on all uncore boxes of same type.

> > Any comment is appreciated.
 
> You forgot to CC acme who maintains the tools side.. Anyway, I think it
> all looks ok, Arnaldo see anything dubious in there?

I'll look at that tomorrow, when my caffeine level is high and the other
substance is down, thanks.

- Arnaldo

P.S. thanks for pointing the (l)userlevel part, much appreciated.