* [PATCH v4 0/4] Add support for cpu event term
@ 2025-01-08 5:34 Ian Rogers
2025-01-08 5:34 ` [PATCH v4 1/4] libperf cpumap: Add ability to create CPU from a single CPU number Ian Rogers
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Ian Rogers @ 2025-01-08 5:34 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera,
Weilin Wang, Andi Kleen, James Clark, Dominique Martinet,
Yicong Yang, linux-perf-users, linux-kernel
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.
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.47.1.613.gc27f4b7a9f-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 1/4] libperf cpumap: Add ability to create CPU from a single CPU number 2025-01-08 5:34 [PATCH v4 0/4] Add support for cpu event term Ian Rogers @ 2025-01-08 5:34 ` Ian Rogers 2025-01-08 5:34 ` [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values Ian Rogers ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Ian Rogers @ 2025-01-08 5:34 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel 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.47.1.613.gc27f4b7a9f-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values 2025-01-08 5:34 [PATCH v4 0/4] Add support for cpu event term Ian Rogers 2025-01-08 5:34 ` [PATCH v4 1/4] libperf cpumap: Add ability to create CPU from a single CPU number Ian Rogers @ 2025-01-08 5:34 ` Ian Rogers 2025-01-08 19:45 ` Namhyung Kim 2025-01-08 5:34 ` [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events Ian Rogers 2025-01-08 5:34 ` [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on Ian Rogers 3 siblings, 1 reply; 10+ messages in thread From: Ian Rogers @ 2025-01-08 5:34 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel 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 ba79f73e1cf5..32badf623267 100644 --- a/tools/perf/util/stat-display.c +++ b/tools/perf/util/stat-display.c @@ -1042,8 +1042,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 = @@ -1051,15 +1059,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.47.1.613.gc27f4b7a9f-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values 2025-01-08 5:34 ` [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values Ian Rogers @ 2025-01-08 19:45 ` Namhyung Kim 2025-01-09 19:25 ` Ian Rogers 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2025-01-08 19:45 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel On Tue, Jan 07, 2025 at 09:34:26PM -0800, Ian Rogers wrote: > 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. It's not clear to me how uncore PMU works with CPU overrides. I thought it's ignored and the kernel changed the CPU internally using the cpumask. But it may be transparent to userspace and we can think it works as what we expect. Anyway, the commit dd15480a3d67b9cf ("perf stat: Hide invalid uncore event output for aggr mode") added the code and the concern was like $ sudo ./perf stat -a --per-core -e power/energy-pkg/ sleep 1 So it should be fine as long as the output remains the same. > 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. Do you mean hybrid PMUs? I guess they won't open events on not supported/requested CPUs in the first place, right? Thanks, Namhyung > > 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 ba79f73e1cf5..32badf623267 100644 > --- a/tools/perf/util/stat-display.c > +++ b/tools/perf/util/stat-display.c > @@ -1042,8 +1042,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 = > @@ -1051,15 +1059,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.47.1.613.gc27f4b7a9f-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values 2025-01-08 19:45 ` Namhyung Kim @ 2025-01-09 19:25 ` Ian Rogers 0 siblings, 0 replies; 10+ messages in thread From: Ian Rogers @ 2025-01-09 19:25 UTC (permalink / raw) To: Namhyung Kim Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel On Wed, Jan 8, 2025 at 11:45 AM Namhyung Kim <namhyung@kernel.org> wrote: > > On Tue, Jan 07, 2025 at 09:34:26PM -0800, Ian Rogers wrote: > > 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. > > It's not clear to me how uncore PMU works with CPU overrides. > I thought it's ignored and the kernel changed the CPU internally > using the cpumask. But it may be transparent to userspace and > we can think it works as what we expect. > > Anyway, the commit dd15480a3d67b9cf ("perf stat: Hide invalid uncore > event output for aggr mode") added the code and the concern was like > > $ sudo ./perf stat -a --per-core -e power/energy-pkg/ sleep 1 > > So it should be fine as long as the output remains the same. Confirmed the output remains the same: ``` $ perf stat -a --per-core -e energy-pkg sleep 1 Performance counter stats for 'system wide': S0-D0-C0 1 22.94 Joules energy-pkg 1.000934566 seconds time elapsed ``` > > 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. > > Do you mean hybrid PMUs? I guess they won't open events on not > supported/requested CPUs in the first place, right? Right. The notion of uncore on a PMU is not the opposite of the notion of core, it's all a bit of a muddle because of the kernel PMU drivers. The previous code always shows 0 when `!pmu->is_uncore` and is_uncore is set when a PMU has a `/sys/devices/<pmu name>/cpumask` file - core PMUs should either have no cpumask or a cpus file instead. In general the evsel cpumask should match the PMU cpumask. The change here is that we will use the cpumask regardless of the PMU having or not having the `/sys/devices/<pmu name>/cpumask` file, where not having the file may reflect hybrid, a single core PMU, a PMU driver bug, different core PMUs like AMD IBS and ARM SPE, etc. The output change from this could be that a 0 on a `!pmu->is_uncore` PMU was previously shown but now it is not. For that to happen the aggregation would need to skip that CPU and as you say that shouldn't happen. Thanks, Ian ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events 2025-01-08 5:34 [PATCH v4 0/4] Add support for cpu event term Ian Rogers 2025-01-08 5:34 ` [PATCH v4 1/4] libperf cpumap: Add ability to create CPU from a single CPU number Ian Rogers 2025-01-08 5:34 ` [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values Ian Rogers @ 2025-01-08 5:34 ` Ian Rogers 2025-01-08 19:45 ` Namhyung Kim 2025-01-08 5:34 ` [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on Ian Rogers 3 siblings, 1 reply; 10+ messages in thread From: Ian Rogers @ 2025-01-08 5:34 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel 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 1e23faa364b1..4bda4141e9e7 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.47.1.613.gc27f4b7a9f-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events 2025-01-08 5:34 ` [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events Ian Rogers @ 2025-01-08 19:45 ` Namhyung Kim 0 siblings, 0 replies; 10+ messages in thread From: Namhyung Kim @ 2025-01-08 19:45 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel On Tue, Jan 07, 2025 at 09:34:27PM -0800, 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"); Looks like it needs to be in a separate commit. Thanks, Namhyung > > 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 1e23faa364b1..4bda4141e9e7 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.47.1.613.gc27f4b7a9f-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on 2025-01-08 5:34 [PATCH v4 0/4] Add support for cpu event term Ian Rogers ` (2 preceding siblings ...) 2025-01-08 5:34 ` [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events Ian Rogers @ 2025-01-08 5:34 ` Ian Rogers 2025-01-08 19:51 ` Namhyung Kim 3 siblings, 1 reply; 10+ messages in thread From: Ian Rogers @ 2025-01-08 5:34 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel 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 4bda4141e9e7..ec8429f0cf6d 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 6206c8fe2bf9..cffb4eb2696b 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1400,7 +1400,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: @@ -1775,6 +1775,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.47.1.613.gc27f4b7a9f-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on 2025-01-08 5:34 ` [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on Ian Rogers @ 2025-01-08 19:51 ` Namhyung Kim 2025-01-09 19:34 ` Ian Rogers 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2025-01-08 19:51 UTC (permalink / raw) To: Ian Rogers Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel On Tue, Jan 07, 2025 at 09:34:28PM -0800, Ian Rogers wrote: > 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. It'd be nice if you could add a description why it's needed and/or some use cases. I'm afraid it'd make the CPU map handling more complicated. Thanks, Namhyung > > 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 4bda4141e9e7..ec8429f0cf6d 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 6206c8fe2bf9..cffb4eb2696b 100644 > --- a/tools/perf/util/pmu.c > +++ b/tools/perf/util/pmu.c > @@ -1400,7 +1400,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: > @@ -1775,6 +1775,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.47.1.613.gc27f4b7a9f-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on 2025-01-08 19:51 ` Namhyung Kim @ 2025-01-09 19:34 ` Ian Rogers 0 siblings, 0 replies; 10+ messages in thread From: Ian Rogers @ 2025-01-09 19:34 UTC (permalink / raw) To: Namhyung Kim Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang, Leo Yan, Yoshihiro Furudera, Weilin Wang, Andi Kleen, James Clark, Dominique Martinet, Yicong Yang, linux-perf-users, linux-kernel On Wed, Jan 8, 2025 at 11:51 AM Namhyung Kim <namhyung@kernel.org> wrote: > > On Tue, Jan 07, 2025 at 09:34:28PM -0800, Ian Rogers wrote: > > 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. > > It'd be nice if you could add a description why it's needed and/or some > use cases. I'm afraid it'd make the CPU map handling more complicated. Not following. There is an example below and I'd like CPU map handling to be robust to always work - this is why I've spent time cleaning up the evlist behavior. Being able to set the CPU per event means, for example, the perf_event_open for 2 uncore events can be sent to 2 different CPUs: ``` $ perf stat -vv -e data_read/cpu=0/,data_write/cpu=1/ -a sleep 1 Using CPUID GenuineIntel-6-8D-1 Attempt to add: uncore_imc_free_running_0/cpu=0,data_read/ ..after resolving event: uncore_imc_free_running_0/cpu=0,event=0xff,umask=0x20/ data_read -> uncore_imc_free_running_0/cpu=0,data_read/ Attempt to add: uncore_imc_free_running_1/cpu=0,data_read/ ..after resolving event: uncore_imc_free_running_1/cpu=0,event=0xff,umask=0x20/ data_read -> uncore_imc_free_running_1/cpu=0,data_read/ Attempt to add: uncore_imc_free_running_0/cpu=0x1,data_write/ ..after resolving event: uncore_imc_free_running_0/cpu=0x1,event=0xff,umask=0x30/ data_write -> uncore_imc_free_running_0/cpu=0x1,data_write/ Attempt to add: uncore_imc_free_running_1/cpu=0x1,data_write/ ..after resolving event: uncore_imc_free_running_1/cpu=0x1,event=0xff,umask=0x30/ data_write -> uncore_imc_free_running_1/cpu=0x1,data_write/ Control descriptor is not initialized ------------------------------------------------------------ perf_event_attr: type 24 (uncore_imc_free_running_0) size 136 config 0x20ff (data_read) sample_type IDENTIFIER read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING disabled 1 inherit 1 ------------------------------------------------------------ sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 3 ------------------------------------------------------------ perf_event_attr: type 25 (uncore_imc_free_running_1) size 136 config 0x20ff (data_read) sample_type IDENTIFIER read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING disabled 1 inherit 1 ------------------------------------------------------------ sys_perf_event_open: pid -1 cpu 0 group_fd -1 flags 0x8 = 13 ------------------------------------------------------------ perf_event_attr: type 24 (uncore_imc_free_running_0) size 136 config 0x30ff (data_write) sample_type IDENTIFIER read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING disabled 1 inherit 1 ------------------------------------------------------------ sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 14 ------------------------------------------------------------ perf_event_attr: type 25 (uncore_imc_free_running_1) size 136 config 0x30ff (data_write) sample_type IDENTIFIER read_format TOTAL_TIME_ENABLED|TOTAL_TIME_RUNNING disabled 1 inherit 1 ------------------------------------------------------------ sys_perf_event_open: pid -1 cpu 1 group_fd -1 flags 0x8 = 16 data_read/cpu=0/: 0: 35597083 1003987395 1003987395 data_read/cpu=0/: 0: 35593630 1003987549 1003987549 data_write/cpu=1/: 1: 1674310 1003942165 1003942165 data_write/cpu=1/: 1: 1672233 1003941578 1003941578 data_read/cpu=0/: 35597083 1003987395 1003987395 data_read/cpu=0/: 35593630 1003987549 1003987549 data_write/cpu=1/: 1674310 1003942165 1003942165 data_write/cpu=1/: 1672233 1003941578 1003941578 Performance counter stats for 'system wide': 4,345.14 MiB data_read/cpu=0/ 204.26 MiB data_write/cpu=1/ 1.003931032 seconds time elapsed ``` If the uncore PMU driver respects the perf_event_open CPU option then for things like cgroup profiling, the context switch cost can be spread over CPUs from a single perf command line. You could get similar behavior by running perf multiple times and that may workaround CPU map bugs, but that feels sub-optimal to me. Thanks, Ian > > > > > 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 4bda4141e9e7..ec8429f0cf6d 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 6206c8fe2bf9..cffb4eb2696b 100644 > > --- a/tools/perf/util/pmu.c > > +++ b/tools/perf/util/pmu.c > > @@ -1400,7 +1400,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: > > @@ -1775,6 +1775,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.47.1.613.gc27f4b7a9f-goog > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-01-09 19:34 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-01-08 5:34 [PATCH v4 0/4] Add support for cpu event term Ian Rogers 2025-01-08 5:34 ` [PATCH v4 1/4] libperf cpumap: Add ability to create CPU from a single CPU number Ian Rogers 2025-01-08 5:34 ` [PATCH v4 2/4] perf stat: Use counter cpumask to skip zero values Ian Rogers 2025-01-08 19:45 ` Namhyung Kim 2025-01-09 19:25 ` Ian Rogers 2025-01-08 5:34 ` [PATCH v4 3/4] perf parse-events: Set is_pmu_core for legacy hardware events Ian Rogers 2025-01-08 19:45 ` Namhyung Kim 2025-01-08 5:34 ` [PATCH v4 4/4] perf parse-events: Add "cpu" term to set the CPU an event is recorded on Ian Rogers 2025-01-08 19:51 ` Namhyung Kim 2025-01-09 19:34 ` Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).