[PATCH v5 0/4] Add support for cpu event term

[PATCH v5 0/4] Add support for cpu event term

[Ian Rogers]

Being able to set the cpu mask per event was discussed in the context
of a sysfs event.cpus file is discussed here:
https://lore.kernel.org/lkml/CAP-5=fXXuWchzUK0n5KTH8kamr=DQoEni+bUoo8f-4j8Y+eMBg@mail.gmail.com/
Ultimately Kan preferred to have multiple PMUs with a cpumask each
rather than an event.cpus file per event. It is still useful to have
the cpu event term and so the sysfs part of the original patch series
is dropped.

v5: Purely a rebase.
v4: Add the stat-display output change for zero counters Namhyung
    requested as part of the series:
    https://lore.kernel.org/lkml/Zvx9VbJWtmkcuSBs@google.com/
    This skips zero values for CPUs not in the evsel's cpumask rather
    than the evsel's PMU's cpumask.
v3: Drop sysfs event.cpus file support patch from series.  Reference
    to using cpu to modify uncore events is dropped from the commit
    message. Reference counting issues on the cpumap are addressed.
v2: Add support for multiple cpu terms on an event that are
    merged. For example, an event of "l1d-misses/cpu=4,cpu=5/" will
    now be opened on both CPU 4 and 5 rather than just CPU 4.

Ian Rogers (4):
  libperf cpumap: Add ability to create CPU from a single CPU number
  perf stat: Use counter cpumask to skip zero values
  perf parse-events: Set is_pmu_core for legacy hardware events
  perf parse-events: Add "cpu" term to set the CPU an event is recorded
    on

 tools/lib/perf/cpumap.c                |  10 +++
 tools/lib/perf/include/perf/cpumap.h   |   2 +
 tools/perf/Documentation/perf-list.txt |   9 ++
 tools/perf/tests/event_update.c        |   1 +
 tools/perf/util/evsel_config.h         |   1 +
 tools/perf/util/parse-events.c         | 113 ++++++++++++++++++-------
 tools/perf/util/parse-events.h         |   3 +-
 tools/perf/util/parse-events.l         |   1 +
 tools/perf/util/pmu.c                  |   3 +-
 tools/perf/util/stat-display.c         |  21 +++--
 10 files changed, 125 insertions(+), 39 deletions(-)

-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH v5 1/4] libperf cpumap: Add ability to create CPU from a single CPU number

[Ian Rogers]

Add perf_cpu_map__new_int so that a CPU map can be created from a
single integer.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/lib/perf/cpumap.c              | 10 ++++++++++
 tools/lib/perf/include/perf/cpumap.h |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c
index fcc47214062a..f3054894ff0c 100644
--- a/tools/lib/perf/cpumap.c
+++ b/tools/lib/perf/cpumap.c
@@ -242,6 +242,16 @@ struct perf_cpu_map *perf_cpu_map__new(const char *cpu_list)
 	return cpus;
 }
 
+struct perf_cpu_map *perf_cpu_map__new_int(int cpu)
+{
+	struct perf_cpu_map *cpus = perf_cpu_map__alloc(1);
+
+	if (cpus)
+		RC_CHK_ACCESS(cpus)->map[0].cpu = cpu;
+
+	return cpus;
+}
+
 static int __perf_cpu_map__nr(const struct perf_cpu_map *cpus)
 {
 	return RC_CHK_ACCESS(cpus)->nr;
diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h
index 188a667babc6..077133935bf4 100644
--- a/tools/lib/perf/include/perf/cpumap.h
+++ b/tools/lib/perf/include/perf/cpumap.h
@@ -36,6 +36,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__new_online_cpus(void);
  *                     perf_cpu_map__new_online_cpus is returned.
  */
 LIBPERF_API struct perf_cpu_map *perf_cpu_map__new(const char *cpu_list);
+/** perf_cpu_map__new_int - create a map with the one given cpu. */
+LIBPERF_API struct perf_cpu_map *perf_cpu_map__new_int(int cpu);
 LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map);
 LIBPERF_API int perf_cpu_map__merge(struct perf_cpu_map **orig,
 				    struct perf_cpu_map *other);
-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH v5 2/4] perf stat: Use counter cpumask to skip zero values

[Ian Rogers]

When a counter is 0 it may or may not be skipped. For uncore counters
it is common they are only valid on 1 logical CPU and all other CPUs
should be skipped. The PMU's cpumask was used for the skip
calculation, but that cpumask may not reflect user
overrides. Similarly a counter on a core PMU may explicitly not
request a CPU be gathered. If the counter on this CPU's value is 0
then the counter should be skipped as it wasn't requested. Switch from
using the PMU cpumask to that associated with the evsel to support
these cases.

Avoid potential crash with --per-thread mode where config->aggr_get_id
is NULL. Add some examples for the tool event 0 counter skipping.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/stat-display.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index e65c7e9f15d1..126fe6cc8893 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -1022,8 +1022,16 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
 		return true;
 
 	/*
-	 * Many tool events are only gathered on the first index, skip other
-	 * zero values.
+	 * In per-thread mode the aggr_map and aggr_get_id functions may be
+	 * NULL, assume all 0 values should be output in that case.
+	 */
+	if (!config->aggr_map || !config->aggr_get_id)
+		return false;
+
+	/*
+	 * Tool events may be gathered on all logical CPUs, for example
+	 * system_time, but for many the first index is the only one used, for
+	 * example num_cores. Don't skip for the first index.
 	 */
 	if (evsel__is_tool(counter)) {
 		struct aggr_cpu_id own_id =
@@ -1031,15 +1039,12 @@ static bool should_skip_zero_counter(struct perf_stat_config *config,
 
 		return !aggr_cpu_id__equal(id, &own_id);
 	}
-
 	/*
-	 * Skip value 0 when it's an uncore event and the given aggr id
-	 * does not belong to the PMU cpumask.
+	 * Skip value 0 when the counter's cpumask doesn't match the given aggr
+	 * id.
 	 */
-	if (!counter->pmu || !counter->pmu->is_uncore)
-		return false;
 
-	perf_cpu_map__for_each_cpu(cpu, idx, counter->pmu->cpus) {
+	perf_cpu_map__for_each_cpu(cpu, idx, counter->core.cpus) {
 		struct aggr_cpu_id own_id = config->aggr_get_id(config, cpu);
 
 		if (aggr_cpu_id__equal(id, &own_id))
-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH v5 3/4] perf parse-events: Set is_pmu_core for legacy hardware events

[Ian Rogers]

Also set the CPU map to all online CPU maps. This is done so the
behavior of legacy hardware and hardware cache events better matches
that of sysfs and json events during
__perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
attr update" test.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/tests/event_update.c |  1 +
 tools/perf/util/parse-events.c  | 37 ++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
index d6b4ce3ef4ee..9301fde11366 100644
--- a/tools/perf/tests/event_update.c
+++ b/tools/perf/tests/event_update.c
@@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
 	TEST_ASSERT_VAL("failed to synthesize attr update name",
 			!perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
 
+	perf_cpu_map__put(evsel->core.own_cpus);
 	evsel->core.own_cpus = perf_cpu_map__new("1,2,3");
 
 	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 6c36b98875bc..8cccf1e22cdf 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
 	    struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
 {
 	struct evsel *evsel;
-	struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
+	bool is_pmu_core;
+	struct perf_cpu_map *cpus;
 
-	cpus = perf_cpu_map__get(cpus);
-	if (pmu)
+	if (pmu) {
+		is_pmu_core = pmu->is_core;
+		cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
 		perf_pmu__warn_invalid_formats(pmu);
-
-	if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
-		perf_pmu__warn_invalid_config(pmu, attr->config, name,
-					      PERF_PMU_FORMAT_VALUE_CONFIG, "config");
-		perf_pmu__warn_invalid_config(pmu, attr->config1, name,
-					      PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
-		perf_pmu__warn_invalid_config(pmu, attr->config2, name,
-					      PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
-		perf_pmu__warn_invalid_config(pmu, attr->config3, name,
-					      PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
+		if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
+			perf_pmu__warn_invalid_config(pmu, attr->config, name,
+						PERF_PMU_FORMAT_VALUE_CONFIG, "config");
+			perf_pmu__warn_invalid_config(pmu, attr->config1, name,
+						PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
+			perf_pmu__warn_invalid_config(pmu, attr->config2, name,
+						PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
+			perf_pmu__warn_invalid_config(pmu, attr->config3, name,
+						PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
+		}
+	} else {
+		is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
+			       attr->type == PERF_TYPE_HW_CACHE);
+		if (perf_cpu_map__is_empty(cpu_list))
+			cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;
+		else
+			cpus = perf_cpu_map__get(cpu_list);
 	}
 	if (init_attr)
 		event_attr_init(attr);
@@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
 	evsel->core.cpus = cpus;
 	evsel->core.own_cpus = perf_cpu_map__get(cpus);
 	evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
-	evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
+	evsel->core.is_pmu_core = is_pmu_core;
 	evsel->auto_merge_stats = auto_merge_stats;
 	evsel->pmu = pmu;
 	evsel->alternate_hw_config = alternate_hw_config;
-- 
2.48.1.502.g6dc24dfdaf-goog

[PATCH v5 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on

[Ian Rogers]

The -C option allows the CPUs for a list of events to be specified but
its not possible to set the CPU for a single event. Add a term to
allow this. The term isn't a general CPU list due to ',' already being
a special character in event parsing instead multiple cpu= terms may
be provided and they will be merged/unioned together.

An example of mixing different types of events counted on different CPUs:
```
$ perf stat -A -C 0,4-5,8 -e "instructions/cpu=0/,l1d-misses/cpu=4,cpu=5/,inst_retired.any/cpu=8/,cycles" -a sleep 0.1

 Performance counter stats for 'system wide':

CPU0            6,979,225      instructions/cpu=0/              #    0.89  insn per cycle
CPU4               75,138      cpu/l1d-misses/
CPU5            1,418,939      cpu/l1d-misses/
CPU8              797,553      cpu/inst_retired.any,cpu=8/
CPU0            7,845,302      cycles
CPU4            6,546,859      cycles
CPU5          185,915,438      cycles
CPU8            2,065,668      cycles

       0.112449242 seconds time elapsed
```

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/Documentation/perf-list.txt |  9 +++
 tools/perf/util/evsel_config.h         |  1 +
 tools/perf/util/parse-events.c         | 76 +++++++++++++++++++++-----
 tools/perf/util/parse-events.h         |  3 +-
 tools/perf/util/parse-events.l         |  1 +
 tools/perf/util/pmu.c                  |  3 +-
 6 files changed, 76 insertions(+), 17 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index d0c65fad419a..d98de9adc515 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -289,6 +289,15 @@ Sums up the event counts for all hardware threads in a core, e.g.:
 
   perf stat -e cpu/event=0,umask=0x3,percore=1/
 
+cpu:
+
+Specifies the CPU to open the event upon. The value may be repeated to
+specify opening the event on multiple CPUs:
+
+
+  perf stat -e instructions/cpu=0,cpu=2/,cycles/cpu=1,cpu=2/ -a sleep 1
+  perf stat -e data_read/cpu=0/,data_write/cpu=1/ -a sleep 1
+
 
 EVENT GROUPS
 ------------
diff --git a/tools/perf/util/evsel_config.h b/tools/perf/util/evsel_config.h
index af52a1516d0b..94a1e9cf73d6 100644
--- a/tools/perf/util/evsel_config.h
+++ b/tools/perf/util/evsel_config.h
@@ -48,6 +48,7 @@ struct evsel_config_term {
 		u32	      aux_sample_size;
 		u64	      cfg_chg;
 		char	      *str;
+		int	      cpu;
 	} val;
 	bool weak;
 };
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 8cccf1e22cdf..55aa35ffbb4a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -7,6 +7,7 @@
 #include <errno.h>
 #include <sys/ioctl.h>
 #include <sys/param.h>
+#include "cpumap.h"
 #include "term.h"
 #include "env.h"
 #include "evlist.h"
@@ -178,6 +179,26 @@ static char *get_config_name(const struct parse_events_terms *head_terms)
 	return get_config_str(head_terms, PARSE_EVENTS__TERM_TYPE_NAME);
 }
 
+static struct perf_cpu_map *get_config_cpu(const struct parse_events_terms *head_terms)
+{
+	struct parse_events_term *term;
+	struct perf_cpu_map *cpus = NULL;
+
+	if (!head_terms)
+		return NULL;
+
+	list_for_each_entry(term, &head_terms->terms, list) {
+		if (term->type_term == PARSE_EVENTS__TERM_TYPE_CPU) {
+			struct perf_cpu_map *cpu = perf_cpu_map__new_int(term->val.num);
+
+			perf_cpu_map__merge(&cpus, cpu);
+			perf_cpu_map__put(cpu);
+		}
+	}
+
+	return cpus;
+}
+
 /**
  * fix_raw - For each raw term see if there is an event (aka alias) in pmu that
  *           matches the raw's string value. If the string value matches an
@@ -441,11 +462,12 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
 	bool found_supported = false;
 	const char *config_name = get_config_name(parsed_terms);
 	const char *metric_id = get_config_metric_id(parsed_terms);
+	struct perf_cpu_map *cpus = get_config_cpu(parsed_terms);
+	int ret = 0;
 
 	while ((pmu = perf_pmus__scan(pmu)) != NULL) {
 		LIST_HEAD(config_terms);
 		struct perf_event_attr attr;
-		int ret;
 
 		if (parse_events__filter_pmu(parse_state, pmu))
 			continue;
@@ -460,7 +482,7 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
 						   perf_pmu__auto_merge_stats(pmu),
 						   /*alternate_hw_config=*/PERF_COUNT_HW_MAX);
 			if (ret)
-				return ret;
+				goto out_err;
 			continue;
 		}
 
@@ -480,21 +502,27 @@ int parse_events_add_cache(struct list_head *list, int *idx, const char *name,
 
 		if (parsed_terms) {
 			if (config_attr(&attr, parsed_terms, parse_state->error,
-					config_term_common))
-				return -EINVAL;
-
-			if (get_config_terms(parsed_terms, &config_terms))
-				return -ENOMEM;
+					config_term_common)) {
+				ret = -EINVAL;
+				goto out_err;
+			}
+			if (get_config_terms(parsed_terms, &config_terms)) {
+				ret = -ENOMEM;
+				goto out_err;
+			}
 		}
 
 		if (__add_event(list, idx, &attr, /*init_attr*/true, config_name ?: name,
 				metric_id, pmu, &config_terms, /*auto_merge_stats=*/false,
-				/*cpu_list=*/NULL,
-				/*alternate_hw_config=*/PERF_COUNT_HW_MAX) == NULL)
-			return -ENOMEM;
+				cpus, /*alternate_hw_config=*/PERF_COUNT_HW_MAX) == NULL)
+			ret = -ENOMEM;
 
 		free_config_terms(&config_terms);
+		if (ret)
+			goto out_err;
 	}
+out_err:
+	perf_cpu_map__put(cpus);
 	return found_supported ? 0 : -EINVAL;
 }
 
@@ -808,6 +836,7 @@ const char *parse_events__term_type_str(enum parse_events__term_type term_type)
 		[PARSE_EVENTS__TERM_TYPE_RAW]                   = "raw",
 		[PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE]          = "legacy-cache",
 		[PARSE_EVENTS__TERM_TYPE_HARDWARE]              = "hardware",
+		[PARSE_EVENTS__TERM_TYPE_CPU]			= "cpu",
 	};
 	if ((unsigned int)term_type >= __PARSE_EVENTS__TERM_TYPE_NR)
 		return "unknown term";
@@ -837,6 +866,7 @@ config_term_avail(enum parse_events__term_type term_type, struct parse_events_er
 	case PARSE_EVENTS__TERM_TYPE_METRIC_ID:
 	case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
 	case PARSE_EVENTS__TERM_TYPE_PERCORE:
+	case PARSE_EVENTS__TERM_TYPE_CPU:
 		return true;
 	case PARSE_EVENTS__TERM_TYPE_USER:
 	case PARSE_EVENTS__TERM_TYPE_SAMPLE_FREQ:
@@ -984,6 +1014,15 @@ do {									   \
 			return -EINVAL;
 		}
 		break;
+	case PARSE_EVENTS__TERM_TYPE_CPU:
+		CHECK_TYPE_VAL(NUM);
+		if (term->val.num >= (u64)cpu__max_present_cpu().cpu) {
+			parse_events_error__handle(err, term->err_val,
+						strdup("too big"),
+						NULL);
+			return -EINVAL;
+		}
+		break;
 	case PARSE_EVENTS__TERM_TYPE_DRV_CFG:
 	case PARSE_EVENTS__TERM_TYPE_USER:
 	case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
@@ -1111,6 +1150,7 @@ static int config_term_tracepoint(struct perf_event_attr *attr,
 	case PARSE_EVENTS__TERM_TYPE_RAW:
 	case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
 	case PARSE_EVENTS__TERM_TYPE_HARDWARE:
+	case PARSE_EVENTS__TERM_TYPE_CPU:
 	default:
 		if (err) {
 			parse_events_error__handle(err, term->err_term,
@@ -1245,6 +1285,7 @@ do {								\
 		case PARSE_EVENTS__TERM_TYPE_RAW:
 		case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
 		case PARSE_EVENTS__TERM_TYPE_HARDWARE:
+		case PARSE_EVENTS__TERM_TYPE_CPU:
 		default:
 			break;
 		}
@@ -1299,6 +1340,7 @@ static int get_config_chgs(struct perf_pmu *pmu, struct parse_events_terms *head
 		case PARSE_EVENTS__TERM_TYPE_RAW:
 		case PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE:
 		case PARSE_EVENTS__TERM_TYPE_HARDWARE:
+		case PARSE_EVENTS__TERM_TYPE_CPU:
 		default:
 			break;
 		}
@@ -1343,6 +1385,7 @@ static int __parse_events_add_numeric(struct parse_events_state *parse_state,
 	struct perf_event_attr attr;
 	LIST_HEAD(config_terms);
 	const char *name, *metric_id;
+	struct perf_cpu_map *cpus;
 	int ret;
 
 	memset(&attr, 0, sizeof(attr));
@@ -1364,10 +1407,11 @@ static int __parse_events_add_numeric(struct parse_events_state *parse_state,
 
 	name = get_config_name(head_config);
 	metric_id = get_config_metric_id(head_config);
+	cpus = get_config_cpu(head_config);
 	ret = __add_event(list, &parse_state->idx, &attr, /*init_attr*/true, name,
-			  metric_id, pmu, &config_terms, /*auto_merge_stats=*/false,
-			  /*cpu_list=*/NULL, /*alternate_hw_config=*/PERF_COUNT_HW_MAX
-		) == NULL ? -ENOMEM : 0;
+			metric_id, pmu, &config_terms, /*auto_merge_stats=*/false,
+			cpus, /*alternate_hw_config=*/PERF_COUNT_HW_MAX) ? 0 : -ENOMEM;
+	perf_cpu_map__put(cpus);
 	free_config_terms(&config_terms);
 	return ret;
 }
@@ -1427,6 +1471,7 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 	LIST_HEAD(config_terms);
 	struct parse_events_terms parsed_terms;
 	bool alias_rewrote_terms = false;
+	struct perf_cpu_map *term_cpu = NULL;
 
 	if (verbose > 1) {
 		struct strbuf sb;
@@ -1521,11 +1566,12 @@ static int parse_events_add_pmu(struct parse_events_state *parse_state,
 		return -EINVAL;
 	}
 
+	term_cpu = get_config_cpu(&parsed_terms);
 	evsel = __add_event(list, &parse_state->idx, &attr, /*init_attr=*/true,
 			    get_config_name(&parsed_terms),
 			    get_config_metric_id(&parsed_terms), pmu,
-			    &config_terms, auto_merge_stats, /*cpu_list=*/NULL,
-			    alternate_hw_config);
+			    &config_terms, auto_merge_stats, term_cpu, alternate_hw_config);
+	perf_cpu_map__put(term_cpu);
 	if (!evsel) {
 		parse_events_terms__exit(&parsed_terms);
 		return -ENOMEM;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index e176a34ab088..ab242f671031 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -80,7 +80,8 @@ enum parse_events__term_type {
 	PARSE_EVENTS__TERM_TYPE_RAW,
 	PARSE_EVENTS__TERM_TYPE_LEGACY_CACHE,
 	PARSE_EVENTS__TERM_TYPE_HARDWARE,
-#define	__PARSE_EVENTS__TERM_TYPE_NR (PARSE_EVENTS__TERM_TYPE_HARDWARE + 1)
+	PARSE_EVENTS__TERM_TYPE_CPU,
+#define	__PARSE_EVENTS__TERM_TYPE_NR (PARSE_EVENTS__TERM_TYPE_CPU + 1)
 };
 
 struct parse_events_term {
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index bf7f73548605..14bff6f9731c 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -324,6 +324,7 @@ aux-output		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_OUTPUT); }
 aux-action		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_ACTION); }
 aux-sample-size		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_AUX_SAMPLE_SIZE); }
 metric-id		{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_METRIC_ID); }
+cpu			{ return term(yyscanner, PARSE_EVENTS__TERM_TYPE_CPU); }
 cpu-cycles|cycles				{ return hw_term(yyscanner, PERF_COUNT_HW_CPU_CYCLES); }
 stalled-cycles-frontend|idle-cycles-frontend	{ return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_FRONTEND); }
 stalled-cycles-backend|idle-cycles-backend	{ return hw_term(yyscanner, PERF_COUNT_HW_STALLED_CYCLES_BACKEND); }
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index c2a15b0259cf..f84a9bf810bb 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1433,7 +1433,7 @@ static int pmu_config_term(const struct perf_pmu *pmu,
 			break;
 		case PARSE_EVENTS__TERM_TYPE_USER: /* Not hardcoded. */
 			return -EINVAL;
-		case PARSE_EVENTS__TERM_TYPE_NAME ... PARSE_EVENTS__TERM_TYPE_HARDWARE:
+		case PARSE_EVENTS__TERM_TYPE_NAME ... PARSE_EVENTS__TERM_TYPE_CPU:
 			/* Skip non-config terms. */
 			break;
 		default:
@@ -1808,6 +1808,7 @@ int perf_pmu__for_each_format(struct perf_pmu *pmu, void *state, pmu_format_call
 		"aux-output",
 		"aux-action=(pause|resume|start-paused)",
 		"aux-sample-size=number",
+		"cpu=number",
 	};
 	struct perf_pmu_format *format;
 	int ret;
-- 
2.48.1.502.g6dc24dfdaf-goog

Re: [PATCH v5 0/4] Add support for cpu event term

[Ian Rogers]

On Mon, Feb 10, 2025 at 10:38 AM Ian Rogers <irogers@google.com> wrote:
>
> Being able to set the cpu mask per event was discussed in the context
> of a sysfs event.cpus file is discussed here:
> https://lore.kernel.org/lkml/CAP-5=fXXuWchzUK0n5KTH8kamr=DQoEni+bUoo8f-4j8Y+eMBg@mail.gmail.com/
> Ultimately Kan preferred to have multiple PMUs with a cpumask each
> rather than an event.cpus file per event. It is still useful to have
> the cpu event term and so the sysfs part of the original patch series
> is dropped.
>
> v5: Purely a rebase.
> v4: Add the stat-display output change for zero counters Namhyung
>     requested as part of the series:
>     https://lore.kernel.org/lkml/Zvx9VbJWtmkcuSBs@google.com/
>     This skips zero values for CPUs not in the evsel's cpumask rather
>     than the evsel's PMU's cpumask.
> v3: Drop sysfs event.cpus file support patch from series.  Reference
>     to using cpu to modify uncore events is dropped from the commit
>     message. Reference counting issues on the cpumap are addressed.
> v2: Add support for multiple cpu terms on an event that are
>     merged. For example, an event of "l1d-misses/cpu=4,cpu=5/" will
>     now be opened on both CPU 4 and 5 rather than just CPU 4.
>
> Ian Rogers (4):
>   libperf cpumap: Add ability to create CPU from a single CPU number
>   perf stat: Use counter cpumask to skip zero values
>   perf parse-events: Set is_pmu_core for legacy hardware events
>   perf parse-events: Add "cpu" term to set the CPU an event is recorded

Ping.

Thanks,
Ian

Re: [PATCH v5 0/4] Add support for cpu event term

[Ian Rogers]

On Mon, Mar 17, 2025 at 9:22 AM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Feb 10, 2025 at 10:38 AM Ian Rogers <irogers@google.com> wrote:
> >
> > Being able to set the cpu mask per event was discussed in the context
> > of a sysfs event.cpus file is discussed here:
> > https://lore.kernel.org/lkml/CAP-5=fXXuWchzUK0n5KTH8kamr=DQoEni+bUoo8f-4j8Y+eMBg@mail.gmail.com/
> > Ultimately Kan preferred to have multiple PMUs with a cpumask each
> > rather than an event.cpus file per event. It is still useful to have
> > the cpu event term and so the sysfs part of the original patch series
> > is dropped.
> >
> > v5: Purely a rebase.
> > v4: Add the stat-display output change for zero counters Namhyung
> >     requested as part of the series:
> >     https://lore.kernel.org/lkml/Zvx9VbJWtmkcuSBs@google.com/
> >     This skips zero values for CPUs not in the evsel's cpumask rather
> >     than the evsel's PMU's cpumask.
> > v3: Drop sysfs event.cpus file support patch from series.  Reference
> >     to using cpu to modify uncore events is dropped from the commit
> >     message. Reference counting issues on the cpumap are addressed.
> > v2: Add support for multiple cpu terms on an event that are
> >     merged. For example, an event of "l1d-misses/cpu=4,cpu=5/" will
> >     now be opened on both CPU 4 and 5 rather than just CPU 4.
> >
> > Ian Rogers (4):
> >   libperf cpumap: Add ability to create CPU from a single CPU number
> >   perf stat: Use counter cpumask to skip zero values
> >   perf parse-events: Set is_pmu_core for legacy hardware events
> >   perf parse-events: Add "cpu" term to set the CPU an event is recorded
>
> Ping.

Ping.

Thanks,
Ian

> Thanks,
> Ian

Re: [PATCH v5 3/4] perf parse-events: Set is_pmu_core for legacy hardware events

[Liang, Kan]

On 2025-02-10 1:38 p.m., Ian Rogers wrote:
> Also set the CPU map to all online CPU maps. This is done so the
> behavior of legacy hardware and hardware cache events better matches
> that of sysfs and json events during
> __perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
> attr update" test.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/tests/event_update.c |  1 +
>  tools/perf/util/parse-events.c  | 37 ++++++++++++++++++++-------------
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> index d6b4ce3ef4ee..9301fde11366 100644
> --- a/tools/perf/tests/event_update.c
> +++ b/tools/perf/tests/event_update.c
> @@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
>  	TEST_ASSERT_VAL("failed to synthesize attr update name",
>  			!perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
>  
> +	perf_cpu_map__put(evsel->core.own_cpus);
>  	evsel->core.own_cpus = perf_cpu_map__new("1,2,3");
>  
>  	TEST_ASSERT_VAL("failed to synthesize attr update cpus",
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 6c36b98875bc..8cccf1e22cdf 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
>  	    struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
>  {
>  	struct evsel *evsel;
> -	struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
> +	bool is_pmu_core;
> +	struct perf_cpu_map *cpus;
>  
> -	cpus = perf_cpu_map__get(cpus);
> -	if (pmu)
> +	if (pmu) {
> +		is_pmu_core = pmu->is_core;
> +		cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
>  		perf_pmu__warn_invalid_formats(pmu);
> -
> -	if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> -		perf_pmu__warn_invalid_config(pmu, attr->config, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> -		perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> -		perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> -		perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> -					      PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> +		if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
> +			perf_pmu__warn_invalid_config(pmu, attr->config, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> +			perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> +			perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> +			perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> +						PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> +		}
> +	} else {
> +		is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
> +			       attr->type == PERF_TYPE_HW_CACHE);
> +		if (perf_cpu_map__is_empty(cpu_list))
> +			cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;

All online CPUs? Is there a problem for hybrid?

Thanks,
Kan> +		else
> +			cpus = perf_cpu_map__get(cpu_list);
>  	}
>  	if (init_attr)
>  		event_attr_init(attr);
> @@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
>  	evsel->core.cpus = cpus;
>  	evsel->core.own_cpus = perf_cpu_map__get(cpus);
>  	evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
> -	evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
> +	evsel->core.is_pmu_core = is_pmu_core;
>  	evsel->auto_merge_stats = auto_merge_stats;
>  	evsel->pmu = pmu;
>  	evsel->alternate_hw_config = alternate_hw_config;

Re: [PATCH v5 3/4] perf parse-events: Set is_pmu_core for legacy hardware events

[Ian Rogers]

On Thu, Apr 3, 2025 at 8:17 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2025-02-10 1:38 p.m., Ian Rogers wrote:
> > Also set the CPU map to all online CPU maps. This is done so the
> > behavior of legacy hardware and hardware cache events better matches
> > that of sysfs and json events during
> > __perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
> > attr update" test.
> >
> > Signed-off-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/tests/event_update.c |  1 +
> >  tools/perf/util/parse-events.c  | 37 ++++++++++++++++++++-------------
> >  2 files changed, 24 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> > index d6b4ce3ef4ee..9301fde11366 100644
> > --- a/tools/perf/tests/event_update.c
> > +++ b/tools/perf/tests/event_update.c
> > @@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
> >       TEST_ASSERT_VAL("failed to synthesize attr update name",
> >                       !perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
> >
> > +     perf_cpu_map__put(evsel->core.own_cpus);
> >       evsel->core.own_cpus = perf_cpu_map__new("1,2,3");
> >
> >       TEST_ASSERT_VAL("failed to synthesize attr update cpus",
> > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> > index 6c36b98875bc..8cccf1e22cdf 100644
> > --- a/tools/perf/util/parse-events.c
> > +++ b/tools/perf/util/parse-events.c
> > @@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
> >           struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
> >  {
> >       struct evsel *evsel;
> > -     struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
> > +     bool is_pmu_core;
> > +     struct perf_cpu_map *cpus;
> >
> > -     cpus = perf_cpu_map__get(cpus);
> > -     if (pmu)
> > +     if (pmu) {
> > +             is_pmu_core = pmu->is_core;
> > +             cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
> >               perf_pmu__warn_invalid_formats(pmu);
> > -
> > -     if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> > -             perf_pmu__warn_invalid_config(pmu, attr->config, name,
> > -                                           PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> > -             perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> > -                                           PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> > -             perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> > -                                           PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> > -             perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> > -                                           PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> > +             if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
> > +                     perf_pmu__warn_invalid_config(pmu, attr->config, name,
> > +                                             PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> > +                     perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> > +                                             PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> > +                     perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> > +                                             PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> > +                     perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> > +                                             PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> > +             }
> > +     } else {
> > +             is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
> > +                            attr->type == PERF_TYPE_HW_CACHE);
> > +             if (perf_cpu_map__is_empty(cpu_list))
> > +                     cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;
>
> All online CPUs? Is there a problem for hybrid?

For hybrid if you do cpu_atom/cycles/ then the PMU is found by
perf_pmus__find here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1651

The wildcard case (e.g. perf stat -e cycles ...) scans all the PMUs
passing the respective pmu into __add_event:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1382

For non-hybrid the PMU is given by perf_pmus__find_by_type which only
returns a PMU if there is an exact match on the type, ie it won't
return perf_pmus__find_core_pmu  when asked to lookup the PMU for a
legacy type, evsel__find_pmu will do this though.

Anyway, the assumption in this legacy configuration no PMU case is
that the event is heading to all CPUs, which is true for software and
tracepoint. For hybrid we could get into this kind of problem is sysfs
isn't mounted, but I think we're pretty generally broken in that case
anyway.

Fwiw, I'm carrying these changes in google's tree and test it on my
hybrid desktop:
https://github.com/googleprodkernel/linux-perf

Thanks,
Ian

> Thanks,
> Kan> +          else
> > +                     cpus = perf_cpu_map__get(cpu_list);
> >       }
> >       if (init_attr)
> >               event_attr_init(attr);
> > @@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
> >       evsel->core.cpus = cpus;
> >       evsel->core.own_cpus = perf_cpu_map__get(cpus);
> >       evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
> > -     evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
> > +     evsel->core.is_pmu_core = is_pmu_core;
> >       evsel->auto_merge_stats = auto_merge_stats;
> >       evsel->pmu = pmu;
> >       evsel->alternate_hw_config = alternate_hw_config;
>

Re: [PATCH v5 3/4] perf parse-events: Set is_pmu_core for legacy hardware events

[Liang, Kan]

On 2025-04-03 11:44 a.m., Ian Rogers wrote:
> On Thu, Apr 3, 2025 at 8:17 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>>
>>
>> On 2025-02-10 1:38 p.m., Ian Rogers wrote:
>>> Also set the CPU map to all online CPU maps. This is done so the
>>> behavior of legacy hardware and hardware cache events better matches
>>> that of sysfs and json events during
>>> __perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
>>> attr update" test.
>>>
>>> Signed-off-by: Ian Rogers <irogers@google.com>
>>> ---
>>>  tools/perf/tests/event_update.c |  1 +
>>>  tools/perf/util/parse-events.c  | 37 ++++++++++++++++++++-------------
>>>  2 files changed, 24 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
>>> index d6b4ce3ef4ee..9301fde11366 100644
>>> --- a/tools/perf/tests/event_update.c
>>> +++ b/tools/perf/tests/event_update.c
>>> @@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
>>>       TEST_ASSERT_VAL("failed to synthesize attr update name",
>>>                       !perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
>>>
>>> +     perf_cpu_map__put(evsel->core.own_cpus);
>>>       evsel->core.own_cpus = perf_cpu_map__new("1,2,3");
>>>
>>>       TEST_ASSERT_VAL("failed to synthesize attr update cpus",


The above change should have been merged.
2c744f38da7a ("perf test: Fix leak in "Synthesize attr update" test").

It makes the patch failed to be applied to perf-tools-next.


>>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
>>> index 6c36b98875bc..8cccf1e22cdf 100644
>>> --- a/tools/perf/util/parse-events.c
>>> +++ b/tools/perf/util/parse-events.c
>>> @@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
>>>           struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
>>>  {
>>>       struct evsel *evsel;
>>> -     struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
>>> +     bool is_pmu_core;
>>> +     struct perf_cpu_map *cpus;
>>>
>>> -     cpus = perf_cpu_map__get(cpus);
>>> -     if (pmu)
>>> +     if (pmu) {
>>> +             is_pmu_core = pmu->is_core;
>>> +             cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
>>>               perf_pmu__warn_invalid_formats(pmu);
>>> -
>>> -     if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
>>> -             perf_pmu__warn_invalid_config(pmu, attr->config, name,
>>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG, "config");
>>> -             perf_pmu__warn_invalid_config(pmu, attr->config1, name,
>>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
>>> -             perf_pmu__warn_invalid_config(pmu, attr->config2, name,
>>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
>>> -             perf_pmu__warn_invalid_config(pmu, attr->config3, name,
>>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
>>> +             if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
>>> +                     perf_pmu__warn_invalid_config(pmu, attr->config, name,
>>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG, "config");
>>> +                     perf_pmu__warn_invalid_config(pmu, attr->config1, name,
>>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
>>> +                     perf_pmu__warn_invalid_config(pmu, attr->config2, name,
>>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
>>> +                     perf_pmu__warn_invalid_config(pmu, attr->config3, name,
>>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
>>> +             }
>>> +     } else {
>>> +             is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
>>> +                            attr->type == PERF_TYPE_HW_CACHE);
>>> +             if (perf_cpu_map__is_empty(cpu_list))
>>> +                     cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;
>>
>> All online CPUs? Is there a problem for hybrid?
> 
> For hybrid if you do cpu_atom/cycles/ then the PMU is found by
> perf_pmus__find here:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1651
>
> The wildcard case (e.g. perf stat -e cycles ...) scans all the PMUs>
passing the respective pmu into __add_event:
> https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1382

Yes, the pmu should always be available for hybrid case.

> 
> For non-hybrid the PMU is given by perf_pmus__find_by_type which only
> returns a PMU if there is an exact match on the type, ie it won't
> return perf_pmus__find_core_pmu  when asked to lookup the PMU for a
> legacy type, evsel__find_pmu will do this though.
> 
> Anyway, the assumption in this legacy configuration no PMU case is
> that the event is heading to all CPUs, which is true for software and
> tracepoint. For hybrid we could get into this kind of problem is sysfs
> isn't mounted, but I think we're pretty generally broken in that case
> anyway.
> 
> Fwiw, I'm carrying these changes in google's tree and test it on my
> hybrid desktop:
> https://github.com/googleprodkernel/linux-perf

OK. I will do some tests then.

Thanks,
Kan

> 
> Thanks,
> Ian
> 
>> Thanks,
>> Kan> +          else
>>> +                     cpus = perf_cpu_map__get(cpu_list);
>>>       }
>>>       if (init_attr)
>>>               event_attr_init(attr);
>>> @@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
>>>       evsel->core.cpus = cpus;
>>>       evsel->core.own_cpus = perf_cpu_map__get(cpus);
>>>       evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
>>> -     evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
>>> +     evsel->core.is_pmu_core = is_pmu_core;
>>>       evsel->auto_merge_stats = auto_merge_stats;
>>>       evsel->pmu = pmu;
>>>       evsel->alternate_hw_config = alternate_hw_config;
>>
> 

Re: [PATCH v5 3/4] perf parse-events: Set is_pmu_core for legacy hardware events

[Ian Rogers]

On Thu, Apr 3, 2025 at 12:25 PM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2025-04-03 11:44 a.m., Ian Rogers wrote:
> > On Thu, Apr 3, 2025 at 8:17 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >>
> >>
> >>
> >> On 2025-02-10 1:38 p.m., Ian Rogers wrote:
> >>> Also set the CPU map to all online CPU maps. This is done so the
> >>> behavior of legacy hardware and hardware cache events better matches
> >>> that of sysfs and json events during
> >>> __perf_evlist__propagate_maps. Fix missing cpumap put in "Synthesize
> >>> attr update" test.
> >>>
> >>> Signed-off-by: Ian Rogers <irogers@google.com>
> >>> ---
> >>>  tools/perf/tests/event_update.c |  1 +
> >>>  tools/perf/util/parse-events.c  | 37 ++++++++++++++++++++-------------
> >>>  2 files changed, 24 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/tools/perf/tests/event_update.c b/tools/perf/tests/event_update.c
> >>> index d6b4ce3ef4ee..9301fde11366 100644
> >>> --- a/tools/perf/tests/event_update.c
> >>> +++ b/tools/perf/tests/event_update.c
> >>> @@ -109,6 +109,7 @@ static int test__event_update(struct test_suite *test __maybe_unused, int subtes
> >>>       TEST_ASSERT_VAL("failed to synthesize attr update name",
> >>>                       !perf_event__synthesize_event_update_name(&tmp.tool, evsel, process_event_name));
> >>>
> >>> +     perf_cpu_map__put(evsel->core.own_cpus);
> >>>       evsel->core.own_cpus = perf_cpu_map__new("1,2,3");
> >>>
> >>>       TEST_ASSERT_VAL("failed to synthesize attr update cpus",
>
>
> The above change should have been merged.
> 2c744f38da7a ("perf test: Fix leak in "Synthesize attr update" test").
>
> It makes the patch failed to be applied to perf-tools-next.

Thanks Kan, I'll send out a rebase.

Ian

> >>> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> >>> index 6c36b98875bc..8cccf1e22cdf 100644
> >>> --- a/tools/perf/util/parse-events.c
> >>> +++ b/tools/perf/util/parse-events.c
> >>> @@ -231,21 +231,30 @@ __add_event(struct list_head *list, int *idx,
> >>>           struct perf_cpu_map *cpu_list, u64 alternate_hw_config)
> >>>  {
> >>>       struct evsel *evsel;
> >>> -     struct perf_cpu_map *cpus = perf_cpu_map__is_empty(cpu_list) && pmu ? pmu->cpus : cpu_list;
> >>> +     bool is_pmu_core;
> >>> +     struct perf_cpu_map *cpus;
> >>>
> >>> -     cpus = perf_cpu_map__get(cpus);
> >>> -     if (pmu)
> >>> +     if (pmu) {
> >>> +             is_pmu_core = pmu->is_core;
> >>> +             cpus = perf_cpu_map__get(perf_cpu_map__is_empty(cpu_list) ? pmu->cpus : cpu_list);
> >>>               perf_pmu__warn_invalid_formats(pmu);
> >>> -
> >>> -     if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> >>> -             perf_pmu__warn_invalid_config(pmu, attr->config, name,
> >>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> >>> -             perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> >>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> >>> -             perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> >>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> >>> -             perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> >>> -                                           PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> >>> +             if (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX) {
> >>> +                     perf_pmu__warn_invalid_config(pmu, attr->config, name,
> >>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> >>> +                     perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> >>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> >>> +                     perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> >>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> >>> +                     perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> >>> +                                             PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> >>> +             }
> >>> +     } else {
> >>> +             is_pmu_core = (attr->type == PERF_TYPE_HARDWARE ||
> >>> +                            attr->type == PERF_TYPE_HW_CACHE);
> >>> +             if (perf_cpu_map__is_empty(cpu_list))
> >>> +                     cpus = is_pmu_core ? perf_cpu_map__new_online_cpus() : NULL;
> >>
> >> All online CPUs? Is there a problem for hybrid?
> >
> > For hybrid if you do cpu_atom/cycles/ then the PMU is found by
> > perf_pmus__find here:
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1651
> >
> > The wildcard case (e.g. perf stat -e cycles ...) scans all the PMUs>
> passing the respective pmu into __add_event:
> > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/parse-events.c?h=perf-tools-next#n1382
>
> Yes, the pmu should always be available for hybrid case.
>
> >
> > For non-hybrid the PMU is given by perf_pmus__find_by_type which only
> > returns a PMU if there is an exact match on the type, ie it won't
> > return perf_pmus__find_core_pmu  when asked to lookup the PMU for a
> > legacy type, evsel__find_pmu will do this though.
> >
> > Anyway, the assumption in this legacy configuration no PMU case is
> > that the event is heading to all CPUs, which is true for software and
> > tracepoint. For hybrid we could get into this kind of problem is sysfs
> > isn't mounted, but I think we're pretty generally broken in that case
> > anyway.
> >
> > Fwiw, I'm carrying these changes in google's tree and test it on my
> > hybrid desktop:
> > https://github.com/googleprodkernel/linux-perf
>
> OK. I will do some tests then.
>
> Thanks,
> Kan
>
> >
> > Thanks,
> > Ian
> >
> >> Thanks,
> >> Kan> +          else
> >>> +                     cpus = perf_cpu_map__get(cpu_list);
> >>>       }
> >>>       if (init_attr)
> >>>               event_attr_init(attr);
> >>> @@ -260,7 +269,7 @@ __add_event(struct list_head *list, int *idx,
> >>>       evsel->core.cpus = cpus;
> >>>       evsel->core.own_cpus = perf_cpu_map__get(cpus);
> >>>       evsel->core.requires_cpu = pmu ? pmu->is_uncore : false;
> >>> -     evsel->core.is_pmu_core = pmu ? pmu->is_core : false;
> >>> +     evsel->core.is_pmu_core = is_pmu_core;
> >>>       evsel->auto_merge_stats = auto_merge_stats;
> >>>       evsel->pmu = pmu;
> >>>       evsel->alternate_hw_config = alternate_hw_config;
> >>
> >
>