* [PATCH v1 1/2] perf bpf_counter: Move header declarations into C code @ 2025-10-01 18:12 Ian Rogers 2025-10-01 18:12 ` [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid Ian Rogers 0 siblings, 1 reply; 6+ messages in thread From: Ian Rogers @ 2025-10-01 18:12 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Tengda Wu, Gabriele Monaco, James Clark, Howard Chu, Athira Rajeev, Song Liu, linux-perf-users, linux-kernel, bpf Reduce the API surface that is in bpf_counter.h, this helps compiler analysis like unused static function, makes it easier to set a breakpoint and just makes it easier to see the code is self contained. When code is shared between BPF C code, put it inside HAVE_BPF_SKEL. Move transitively found #includes into appropriate C files. No functional change. Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf_counter.c | 62 ++++++++++++++++++++++- tools/perf/util/bpf_counter.h | 74 +++------------------------- tools/perf/util/bpf_counter_cgroup.c | 1 + tools/perf/util/bpf_ftrace.c | 1 + tools/perf/util/bpf_off_cpu.c | 1 + 5 files changed, 69 insertions(+), 70 deletions(-) diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c index 73fcafbffc6a..1c6cb5ea077e 100644 --- a/tools/perf/util/bpf_counter.c +++ b/tools/perf/util/bpf_counter.c @@ -6,10 +6,14 @@ #include <limits.h> #include <unistd.h> #include <sys/file.h> +#include <sys/resource.h> #include <sys/time.h> #include <linux/err.h> +#include <linux/list.h> #include <linux/zalloc.h> #include <api/fs/fs.h> +#include <bpf/bpf.h> +#include <bpf/btf.h> #include <perf/bpf_perf.h> #include "bpf_counter.h" @@ -28,13 +32,67 @@ #include "bpf_skel/bperf_leader.skel.h" #include "bpf_skel/bperf_follower.skel.h" +struct bpf_counter { + void *skel; + struct list_head list; +}; + #define ATTR_MAP_SIZE 16 -static inline void *u64_to_ptr(__u64 ptr) +static void *u64_to_ptr(__u64 ptr) { return (void *)(unsigned long)ptr; } + +void set_max_rlimit(void) +{ + struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY }; + + setrlimit(RLIMIT_MEMLOCK, &rinf); +} + +static __u32 bpf_link_get_id(int fd) +{ + struct bpf_link_info link_info = { .id = 0, }; + __u32 link_info_len = sizeof(link_info); + + bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len); + return link_info.id; +} + +static __u32 bpf_link_get_prog_id(int fd) +{ + struct bpf_link_info link_info = { .id = 0, }; + __u32 link_info_len = sizeof(link_info); + + bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len); + return link_info.prog_id; +} + +static __u32 bpf_map_get_id(int fd) +{ + struct bpf_map_info map_info = { .id = 0, }; + __u32 map_info_len = sizeof(map_info); + + bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len); + return map_info.id; +} + +/* trigger the leader program on a cpu */ +int bperf_trigger_reading(int prog_fd, int cpu) +{ + DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts, + .ctx_in = NULL, + .ctx_size_in = 0, + .flags = BPF_F_TEST_RUN_ON_CPU, + .cpu = cpu, + .retval = 0, + ); + + return bpf_prog_test_run_opts(prog_fd, &opts); +} + static struct bpf_counter *bpf_counter_alloc(void) { struct bpf_counter *counter; @@ -785,7 +843,7 @@ struct bpf_counter_ops bperf_ops = { extern struct bpf_counter_ops bperf_cgrp_ops; -static inline bool bpf_counter_skip(struct evsel *evsel) +static bool bpf_counter_skip(struct evsel *evsel) { return evsel->bpf_counter_ops == NULL; } diff --git a/tools/perf/util/bpf_counter.h b/tools/perf/util/bpf_counter.h index c6d21c07b14c..658d8e7d507e 100644 --- a/tools/perf/util/bpf_counter.h +++ b/tools/perf/util/bpf_counter.h @@ -2,18 +2,10 @@ #ifndef __PERF_BPF_COUNTER_H #define __PERF_BPF_COUNTER_H 1 -#include <linux/list.h> -#include <sys/resource.h> - -#ifdef HAVE_LIBBPF_SUPPORT -#include <bpf/bpf.h> -#include <bpf/btf.h> -#include <bpf/libbpf.h> -#endif - struct evsel; struct target; -struct bpf_counter; + +#ifdef HAVE_BPF_SKEL typedef int (*bpf_counter_evsel_op)(struct evsel *evsel); typedef int (*bpf_counter_evsel_target_op)(struct evsel *evsel, @@ -22,6 +14,7 @@ typedef int (*bpf_counter_evsel_install_pe_op)(struct evsel *evsel, int cpu_map_idx, int fd); +/* Shared ops between bpf_counter, bpf_counter_cgroup, etc. */ struct bpf_counter_ops { bpf_counter_evsel_target_op load; bpf_counter_evsel_op enable; @@ -31,13 +24,6 @@ struct bpf_counter_ops { bpf_counter_evsel_install_pe_op install_pe; }; -struct bpf_counter { - void *skel; - struct list_head list; -}; - -#ifdef HAVE_BPF_SKEL - int bpf_counter__load(struct evsel *evsel, struct target *target); int bpf_counter__enable(struct evsel *evsel); int bpf_counter__disable(struct evsel *evsel); @@ -45,6 +31,9 @@ int bpf_counter__read(struct evsel *evsel); void bpf_counter__destroy(struct evsel *evsel); int bpf_counter__install_pe(struct evsel *evsel, int cpu_map_idx, int fd); +int bperf_trigger_reading(int prog_fd, int cpu); +void set_max_rlimit(void); + #else /* HAVE_BPF_SKEL */ #include <linux/err.h> @@ -83,55 +72,4 @@ static inline int bpf_counter__install_pe(struct evsel *evsel __maybe_unused, #endif /* HAVE_BPF_SKEL */ -static inline void set_max_rlimit(void) -{ - struct rlimit rinf = { RLIM_INFINITY, RLIM_INFINITY }; - - setrlimit(RLIMIT_MEMLOCK, &rinf); -} - -#ifdef HAVE_BPF_SKEL - -static inline __u32 bpf_link_get_id(int fd) -{ - struct bpf_link_info link_info = { .id = 0, }; - __u32 link_info_len = sizeof(link_info); - - bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len); - return link_info.id; -} - -static inline __u32 bpf_link_get_prog_id(int fd) -{ - struct bpf_link_info link_info = { .id = 0, }; - __u32 link_info_len = sizeof(link_info); - - bpf_obj_get_info_by_fd(fd, &link_info, &link_info_len); - return link_info.prog_id; -} - -static inline __u32 bpf_map_get_id(int fd) -{ - struct bpf_map_info map_info = { .id = 0, }; - __u32 map_info_len = sizeof(map_info); - - bpf_obj_get_info_by_fd(fd, &map_info, &map_info_len); - return map_info.id; -} - -/* trigger the leader program on a cpu */ -static inline int bperf_trigger_reading(int prog_fd, int cpu) -{ - DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts, - .ctx_in = NULL, - .ctx_size_in = 0, - .flags = BPF_F_TEST_RUN_ON_CPU, - .cpu = cpu, - .retval = 0, - ); - - return bpf_prog_test_run_opts(prog_fd, &opts); -} -#endif /* HAVE_BPF_SKEL */ - #endif /* __PERF_BPF_COUNTER_H */ diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c index 6ff42619de12..ed6a29b106b4 100644 --- a/tools/perf/util/bpf_counter_cgroup.c +++ b/tools/perf/util/bpf_counter_cgroup.c @@ -13,6 +13,7 @@ #include <linux/zalloc.h> #include <linux/perf_event.h> #include <api/fs/fs.h> +#include <bpf/bpf.h> #include <perf/bpf_perf.h> #include "affinity.h" diff --git a/tools/perf/util/bpf_ftrace.c b/tools/perf/util/bpf_ftrace.c index 0cb02412043c..e61a3b20be0a 100644 --- a/tools/perf/util/bpf_ftrace.c +++ b/tools/perf/util/bpf_ftrace.c @@ -3,6 +3,7 @@ #include <stdint.h> #include <stdlib.h> +#include <bpf/bpf.h> #include <linux/err.h> #include "util/ftrace.h" diff --git a/tools/perf/util/bpf_off_cpu.c b/tools/perf/util/bpf_off_cpu.c index c367fefe6ecb..88e0660c4bff 100644 --- a/tools/perf/util/bpf_off_cpu.c +++ b/tools/perf/util/bpf_off_cpu.c @@ -13,6 +13,7 @@ #include "util/cgroup.h" #include "util/strlist.h" #include <bpf/bpf.h> +#include <bpf/btf.h> #include <internal/xyarray.h> #include <linux/time64.h> -- 2.51.0.618.g983fd99d29-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid 2025-10-01 18:12 [PATCH v1 1/2] perf bpf_counter: Move header declarations into C code Ian Rogers @ 2025-10-01 18:12 ` Ian Rogers 2025-10-06 16:18 ` Ian Rogers 0 siblings, 1 reply; 6+ messages in thread From: Ian Rogers @ 2025-10-01 18:12 UTC (permalink / raw) To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Alexander Shishkin, Jiri Olsa, Ian Rogers, Adrian Hunter, Tengda Wu, Gabriele Monaco, James Clark, Howard Chu, Athira Rajeev, Song Liu, linux-perf-users, linux-kernel, bpf Don't open evsels on all CPUs, open them just on the CPUs they support. This avoids opening say an e-core event on a p-core and getting a failure - achieve this by getting rid of the "all_cpu_map". In install_pe functions don't use the cpu_map_idx as a CPU number, translate the cpu_map_idx, which is a dense index into the cpu_map skipping holes at the beginning, to a proper CPU number. Before: ``` $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 Performance counter stats for 'system wide': <not supported> cpu_atom/cycles/ 566,270,672 cpu_core/cycles/ <not supported> cpu_atom/instructions/ 572,792,836 cpu_core/instructions/ # 1.01 insn per cycle 1.001595384 seconds time elapsed ``` After: ``` $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 Performance counter stats for 'system wide': 443,299,201 cpu_atom/cycles/ 1,233,919,737 cpu_core/cycles/ 213,634,112 cpu_atom/instructions/ # 0.48 insn per cycle 2,758,965,527 cpu_core/instructions/ # 2.24 insn per cycle 1.001699485 seconds time elapsed ``` Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share hardware PMCs with BPF") Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/bpf_counter.c | 26 ++++++++++---------------- tools/perf/util/bpf_counter_cgroup.c | 3 ++- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c index 1c6cb5ea077e..ca5d01b9017d 100644 --- a/tools/perf/util/bpf_counter.c +++ b/tools/perf/util/bpf_counter.c @@ -336,6 +336,7 @@ static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu_map_idx { struct bpf_prog_profiler_bpf *skel; struct bpf_counter *counter; + int cpu = perf_cpu_map__cpu(evsel->core.cpus, cpu_map_idx).cpu; int ret; list_for_each_entry(counter, &evsel->bpf_counter_list, list) { @@ -343,7 +344,7 @@ static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu_map_idx assert(skel != NULL); ret = bpf_map_update_elem(bpf_map__fd(skel->maps.events), - &cpu_map_idx, &fd, BPF_ANY); + &cpu, &fd, BPF_ANY); if (ret) return ret; } @@ -451,7 +452,6 @@ static int bperf_check_target(struct evsel *evsel, return 0; } -static struct perf_cpu_map *all_cpu_map; static __u32 filter_entry_cnt; static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd, @@ -495,7 +495,7 @@ static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd, * following evsel__open_per_cpu call */ evsel->leader_skel = skel; - evsel__open_per_cpu(evsel, all_cpu_map, -1); + evsel__open(evsel, evsel->core.cpus, evsel->core.threads); out: bperf_leader_bpf__destroy(skel); @@ -533,12 +533,6 @@ static int bperf__load(struct evsel *evsel, struct target *target) if (bperf_check_target(evsel, target, &filter_type, &filter_entry_cnt)) return -1; - if (!all_cpu_map) { - all_cpu_map = perf_cpu_map__new_online_cpus(); - if (!all_cpu_map) - return -1; - } - evsel->bperf_leader_prog_fd = -1; evsel->bperf_leader_link_fd = -1; @@ -656,9 +650,10 @@ static int bperf__load(struct evsel *evsel, struct target *target) static int bperf__install_pe(struct evsel *evsel, int cpu_map_idx, int fd) { struct bperf_leader_bpf *skel = evsel->leader_skel; + int cpu = perf_cpu_map__cpu(evsel->core.cpus, cpu_map_idx).cpu; return bpf_map_update_elem(bpf_map__fd(skel->maps.events), - &cpu_map_idx, &fd, BPF_ANY); + &cpu, &fd, BPF_ANY); } /* @@ -667,13 +662,12 @@ static int bperf__install_pe(struct evsel *evsel, int cpu_map_idx, int fd) */ static int bperf_sync_counters(struct evsel *evsel) { - int num_cpu, i, cpu; + struct perf_cpu cpu; + int idx; + + perf_cpu_map__for_each_cpu(cpu, idx, evsel->core.cpus) + bperf_trigger_reading(evsel->bperf_leader_prog_fd, cpu.cpu); - num_cpu = perf_cpu_map__nr(all_cpu_map); - for (i = 0; i < num_cpu; i++) { - cpu = perf_cpu_map__cpu(all_cpu_map, i).cpu; - bperf_trigger_reading(evsel->bperf_leader_prog_fd, cpu); - } return 0; } diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c index ed6a29b106b4..690be3ce3e11 100644 --- a/tools/perf/util/bpf_counter_cgroup.c +++ b/tools/perf/util/bpf_counter_cgroup.c @@ -186,7 +186,8 @@ static int bperf_cgrp__load(struct evsel *evsel, } static int bperf_cgrp__install_pe(struct evsel *evsel __maybe_unused, - int cpu __maybe_unused, int fd __maybe_unused) + int cpu_map_idx __maybe_unused, + int fd __maybe_unused) { /* nothing to do */ return 0; -- 2.51.0.618.g983fd99d29-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid 2025-10-01 18:12 ` [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid Ian Rogers @ 2025-10-06 16:18 ` Ian Rogers 2025-10-06 19:57 ` Arnaldo Carvalho de Melo 2025-10-06 20:47 ` Falcon, Thomas 0 siblings, 2 replies; 6+ messages in thread From: Ian Rogers @ 2025-10-06 16:18 UTC (permalink / raw) To: Thomas Falcon, Namhyung Kim, Jiri Olsa, Song Liu, Arnaldo Carvalho de Melo Cc: bpf, linux-kernel, linux-perf-users, Howard Chu, Gabriele Monaco, Athira Rajeev, James Clark, Alexander Shishkin, Adrian Hunter, Tengda Wu, Peter Zijlstra, Ingo Molnar On Wed, Oct 1, 2025 at 11:12 AM Ian Rogers <irogers@google.com> wrote: > > Don't open evsels on all CPUs, open them just on the CPUs they > support. This avoids opening say an e-core event on a p-core and > getting a failure - achieve this by getting rid of the "all_cpu_map". > > In install_pe functions don't use the cpu_map_idx as a CPU number, > translate the cpu_map_idx, which is a dense index into the cpu_map > skipping holes at the beginning, to a proper CPU number. > > Before: > ``` > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > Performance counter stats for 'system wide': > > <not supported> cpu_atom/cycles/ > 566,270,672 cpu_core/cycles/ > <not supported> cpu_atom/instructions/ > 572,792,836 cpu_core/instructions/ # 1.01 insn per cycle > > 1.001595384 seconds time elapsed > ``` > > After: > ``` > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > Performance counter stats for 'system wide': > > 443,299,201 cpu_atom/cycles/ > 1,233,919,737 cpu_core/cycles/ > 213,634,112 cpu_atom/instructions/ # 0.48 insn per cycle > 2,758,965,527 cpu_core/instructions/ # 2.24 insn per cycle > > 1.001699485 seconds time elapsed > ``` > > Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share hardware PMCs with BPF") > Signed-off-by: Ian Rogers <irogers@google.com> +Thomas Falcon I think it'd be nice to get this quite major fix for --bpf-counters/bperf for hybrid architectures into v6.18 and stable builds. Thomas would it be possible for you to give a Tested-by tag using the reproduction in the commit message? Thanks, Ian > --- > tools/perf/util/bpf_counter.c | 26 ++++++++++---------------- > tools/perf/util/bpf_counter_cgroup.c | 3 ++- > 2 files changed, 12 insertions(+), 17 deletions(-) > > diff --git a/tools/perf/util/bpf_counter.c b/tools/perf/util/bpf_counter.c > index 1c6cb5ea077e..ca5d01b9017d 100644 > --- a/tools/perf/util/bpf_counter.c > +++ b/tools/perf/util/bpf_counter.c > @@ -336,6 +336,7 @@ static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu_map_idx > { > struct bpf_prog_profiler_bpf *skel; > struct bpf_counter *counter; > + int cpu = perf_cpu_map__cpu(evsel->core.cpus, cpu_map_idx).cpu; > int ret; > > list_for_each_entry(counter, &evsel->bpf_counter_list, list) { > @@ -343,7 +344,7 @@ static int bpf_program_profiler__install_pe(struct evsel *evsel, int cpu_map_idx > assert(skel != NULL); > > ret = bpf_map_update_elem(bpf_map__fd(skel->maps.events), > - &cpu_map_idx, &fd, BPF_ANY); > + &cpu, &fd, BPF_ANY); > if (ret) > return ret; > } > @@ -451,7 +452,6 @@ static int bperf_check_target(struct evsel *evsel, > return 0; > } > > -static struct perf_cpu_map *all_cpu_map; > static __u32 filter_entry_cnt; > > static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd, > @@ -495,7 +495,7 @@ static int bperf_reload_leader_program(struct evsel *evsel, int attr_map_fd, > * following evsel__open_per_cpu call > */ > evsel->leader_skel = skel; > - evsel__open_per_cpu(evsel, all_cpu_map, -1); > + evsel__open(evsel, evsel->core.cpus, evsel->core.threads); > > out: > bperf_leader_bpf__destroy(skel); > @@ -533,12 +533,6 @@ static int bperf__load(struct evsel *evsel, struct target *target) > if (bperf_check_target(evsel, target, &filter_type, &filter_entry_cnt)) > return -1; > > - if (!all_cpu_map) { > - all_cpu_map = perf_cpu_map__new_online_cpus(); > - if (!all_cpu_map) > - return -1; > - } > - > evsel->bperf_leader_prog_fd = -1; > evsel->bperf_leader_link_fd = -1; > > @@ -656,9 +650,10 @@ static int bperf__load(struct evsel *evsel, struct target *target) > static int bperf__install_pe(struct evsel *evsel, int cpu_map_idx, int fd) > { > struct bperf_leader_bpf *skel = evsel->leader_skel; > + int cpu = perf_cpu_map__cpu(evsel->core.cpus, cpu_map_idx).cpu; > > return bpf_map_update_elem(bpf_map__fd(skel->maps.events), > - &cpu_map_idx, &fd, BPF_ANY); > + &cpu, &fd, BPF_ANY); > } > > /* > @@ -667,13 +662,12 @@ static int bperf__install_pe(struct evsel *evsel, int cpu_map_idx, int fd) > */ > static int bperf_sync_counters(struct evsel *evsel) > { > - int num_cpu, i, cpu; > + struct perf_cpu cpu; > + int idx; > + > + perf_cpu_map__for_each_cpu(cpu, idx, evsel->core.cpus) > + bperf_trigger_reading(evsel->bperf_leader_prog_fd, cpu.cpu); > > - num_cpu = perf_cpu_map__nr(all_cpu_map); > - for (i = 0; i < num_cpu; i++) { > - cpu = perf_cpu_map__cpu(all_cpu_map, i).cpu; > - bperf_trigger_reading(evsel->bperf_leader_prog_fd, cpu); > - } > return 0; > } > > diff --git a/tools/perf/util/bpf_counter_cgroup.c b/tools/perf/util/bpf_counter_cgroup.c > index ed6a29b106b4..690be3ce3e11 100644 > --- a/tools/perf/util/bpf_counter_cgroup.c > +++ b/tools/perf/util/bpf_counter_cgroup.c > @@ -186,7 +186,8 @@ static int bperf_cgrp__load(struct evsel *evsel, > } > > static int bperf_cgrp__install_pe(struct evsel *evsel __maybe_unused, > - int cpu __maybe_unused, int fd __maybe_unused) > + int cpu_map_idx __maybe_unused, > + int fd __maybe_unused) > { > /* nothing to do */ > return 0; > -- > 2.51.0.618.g983fd99d29-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid 2025-10-06 16:18 ` Ian Rogers @ 2025-10-06 19:57 ` Arnaldo Carvalho de Melo 2025-10-06 21:20 ` Ian Rogers 2025-10-06 20:47 ` Falcon, Thomas 1 sibling, 1 reply; 6+ messages in thread From: Arnaldo Carvalho de Melo @ 2025-10-06 19:57 UTC (permalink / raw) To: Ian Rogers Cc: Thomas Falcon, Namhyung Kim, Jiri Olsa, Song Liu, bpf, linux-kernel, linux-perf-users, Howard Chu, Gabriele Monaco, Athira Rajeev, James Clark, Alexander Shishkin, Adrian Hunter, Tengda Wu, Peter Zijlstra, Ingo Molnar On Mon, Oct 06, 2025 at 09:18:22AM -0700, Ian Rogers wrote: > On Wed, Oct 1, 2025 at 11:12 AM Ian Rogers <irogers@google.com> wrote: > > > > Don't open evsels on all CPUs, open them just on the CPUs they > > support. This avoids opening say an e-core event on a p-core and > > getting a failure - achieve this by getting rid of the "all_cpu_map". > > > > In install_pe functions don't use the cpu_map_idx as a CPU number, > > translate the cpu_map_idx, which is a dense index into the cpu_map > > skipping holes at the beginning, to a proper CPU number. > > > > Before: > > ``` > > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > > > Performance counter stats for 'system wide': > > > > <not supported> cpu_atom/cycles/ > > 566,270,672 cpu_core/cycles/ > > <not supported> cpu_atom/instructions/ > > 572,792,836 cpu_core/instructions/ # 1.01 insn per cycle > > > > 1.001595384 seconds time elapsed > > ``` > > > > After: > > ``` > > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > > > Performance counter stats for 'system wide': > > > > 443,299,201 cpu_atom/cycles/ > > 1,233,919,737 cpu_core/cycles/ > > 213,634,112 cpu_atom/instructions/ # 0.48 insn per cycle > > 2,758,965,527 cpu_core/instructions/ # 2.24 insn per cycle > > > > 1.001699485 seconds time elapsed > > ``` > > > > Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share hardware PMCs with BPF") > > Signed-off-by: Ian Rogers <irogers@google.com> > > +Thomas Falcon > > I think it'd be nice to get this quite major fix for > --bpf-counters/bperf for hybrid architectures into v6.18 and stable > builds. Thomas would it be possible for you to give a Tested-by tag > using the reproduction in the commit message? Its even already in linux-next: ⬢ [acme@toolbx perf-tools-next]$ git log -5 --oneline linux-next/master tools/perf/util/bpf_counter.c b91917c0c6fa6df9 perf bpf_counter: Fix handling of cpumap fixing hybrid 8c519a825b4add85 perf bpf_counter: Move header declarations into C code 07dc3a6de33098b0 perf stat: Support inherit events during fork() for bperf effe957c6bb70cac libperf cpumap: Replace usage of perf_cpu_map__new(NULL) with perf_cpu_map__new_online_cpus() b84b3f47921568a8 perf bpf_counter: Fix a few memory leaks ⬢ [acme@toolbx perf-tools-next]$ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid 2025-10-06 19:57 ` Arnaldo Carvalho de Melo @ 2025-10-06 21:20 ` Ian Rogers 0 siblings, 0 replies; 6+ messages in thread From: Ian Rogers @ 2025-10-06 21:20 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Thomas Falcon, Namhyung Kim, Jiri Olsa, Song Liu, bpf, linux-kernel, linux-perf-users, Howard Chu, Gabriele Monaco, Athira Rajeev, James Clark, Alexander Shishkin, Adrian Hunter, Tengda Wu, Peter Zijlstra, Ingo Molnar On Mon, Oct 6, 2025 at 12:57 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > On Mon, Oct 06, 2025 at 09:18:22AM -0700, Ian Rogers wrote: > > On Wed, Oct 1, 2025 at 11:12 AM Ian Rogers <irogers@google.com> wrote: > > > > > > Don't open evsels on all CPUs, open them just on the CPUs they > > > support. This avoids opening say an e-core event on a p-core and > > > getting a failure - achieve this by getting rid of the "all_cpu_map". > > > > > > In install_pe functions don't use the cpu_map_idx as a CPU number, > > > translate the cpu_map_idx, which is a dense index into the cpu_map > > > skipping holes at the beginning, to a proper CPU number. > > > > > > Before: > > > ``` > > > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > > > > > Performance counter stats for 'system wide': > > > > > > <not supported> cpu_atom/cycles/ > > > 566,270,672 cpu_core/cycles/ > > > <not supported> cpu_atom/instructions/ > > > 572,792,836 cpu_core/instructions/ # 1.01 insn per cycle > > > > > > 1.001595384 seconds time elapsed > > > ``` > > > > > > After: > > > ``` > > > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > > > > > Performance counter stats for 'system wide': > > > > > > 443,299,201 cpu_atom/cycles/ > > > 1,233,919,737 cpu_core/cycles/ > > > 213,634,112 cpu_atom/instructions/ # 0.48 insn per cycle > > > 2,758,965,527 cpu_core/instructions/ # 2.24 insn per cycle > > > > > > 1.001699485 seconds time elapsed > > > ``` > > > > > > Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share hardware PMCs with BPF") > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > > +Thomas Falcon > > > > I think it'd be nice to get this quite major fix for > > --bpf-counters/bperf for hybrid architectures into v6.18 and stable > > builds. Thomas would it be possible for you to give a Tested-by tag > > using the reproduction in the commit message? > > Its even already in linux-next: > > ⬢ [acme@toolbx perf-tools-next]$ git log -5 --oneline linux-next/master tools/perf/util/bpf_counter.c > b91917c0c6fa6df9 perf bpf_counter: Fix handling of cpumap fixing hybrid > 8c519a825b4add85 perf bpf_counter: Move header declarations into C code > 07dc3a6de33098b0 perf stat: Support inherit events during fork() for bperf > effe957c6bb70cac libperf cpumap: Replace usage of perf_cpu_map__new(NULL) with perf_cpu_map__new_online_cpus() > b84b3f47921568a8 perf bpf_counter: Fix a few memory leaks > ⬢ [acme@toolbx perf-tools-next]$ Oh, thanks! Sorry for missing it. Thanks Thomas for the tag! Ian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid 2025-10-06 16:18 ` Ian Rogers 2025-10-06 19:57 ` Arnaldo Carvalho de Melo @ 2025-10-06 20:47 ` Falcon, Thomas 1 sibling, 0 replies; 6+ messages in thread From: Falcon, Thomas @ 2025-10-06 20:47 UTC (permalink / raw) To: namhyung@kernel.org, jolsa@kernel.org, songliubraving@fb.com, acme@kernel.org, irogers@google.com Cc: james.clark@linaro.org, alexander.shishkin@linux.intel.com, wutengda@huaweicloud.com, peterz@infradead.org, mingo@redhat.com, howardchu95@gmail.com, atrajeev@linux.vnet.ibm.com, Hunter, Adrian, linux-kernel@vger.kernel.org, bpf@vger.kernel.org, gmonaco@redhat.com, linux-perf-users@vger.kernel.org On Mon, 2025-10-06 at 09:18 -0700, Ian Rogers wrote: > On Wed, Oct 1, 2025 at 11:12 AM Ian Rogers <irogers@google.com> > wrote: > > > > Don't open evsels on all CPUs, open them just on the CPUs they > > support. This avoids opening say an e-core event on a p-core and > > getting a failure - achieve this by getting rid of the > > "all_cpu_map". > > > > In install_pe functions don't use the cpu_map_idx as a CPU number, > > translate the cpu_map_idx, which is a dense index into the cpu_map > > skipping holes at the beginning, to a proper CPU number. > > > > Before: > > ``` > > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > > > Performance counter stats for 'system wide': > > > > <not supported> cpu_atom/cycles/ > > 566,270,672 cpu_core/cycles/ > > <not supported> cpu_atom/instructions/ > > 572,792,836 cpu_core/instructions/ # 1.01 > > insn per cycle > > > > 1.001595384 seconds time elapsed > > ``` > > > > After: > > ``` > > $ perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 > > > > Performance counter stats for 'system wide': > > > > 443,299,201 cpu_atom/cycles/ > > 1,233,919,737 cpu_core/cycles/ > > 213,634,112 cpu_atom/instructions/ # 0.48 > > insn per cycle > > 2,758,965,527 cpu_core/instructions/ # 2.24 > > insn per cycle > > > > 1.001699485 seconds time elapsed > > ``` > > > > Fixes: 7fac83aaf2ee ("perf stat: Introduce 'bperf' to share > > hardware PMCs with BPF") > > Signed-off-by: Ian Rogers <irogers@google.com> > > +Thomas Falcon > > I think it'd be nice to get this quite major fix for > --bpf-counters/bperf for hybrid architectures into v6.18 and stable > builds. Thomas would it be possible for you to give a Tested-by tag > using the reproduction in the commit message? > > Thanks, > Ian > Sorry for missing this. Here's my reproduction on an alder lake sudo ./perf stat --bpf-counters -a -e cycles,instructions -- sleep 1 ... Performance counter stats for 'system wide': 364,715,896 cpu_atom/cycles/ 946,331,957 cpu_core/cycles/ 169,842,929 cpu_atom/instructions/ # 0.47 insn per cycle 1,338,720,324 cpu_core/instructions/ # 1.41 insn per cycle 1.001667769 seconds time elapsed It looks like it's in perf-tools-next already but have my tested by for what it's worth. Tested-by: Thomas Falcon <thomas.falcon@intel.com> > > --- > > tools/perf/util/bpf_counter.c | 26 ++++++++++-------------- > > -- > > tools/perf/util/bpf_counter_cgroup.c | 3 ++- > > 2 files changed, 12 insertions(+), 17 deletions(-) > > > > diff --git a/tools/perf/util/bpf_counter.c > > b/tools/perf/util/bpf_counter.c > > index 1c6cb5ea077e..ca5d01b9017d 100644 > > --- a/tools/perf/util/bpf_counter.c > > +++ b/tools/perf/util/bpf_counter.c > > @@ -336,6 +336,7 @@ static int > > bpf_program_profiler__install_pe(struct evsel *evsel, int > > cpu_map_idx > > { > > struct bpf_prog_profiler_bpf *skel; > > struct bpf_counter *counter; > > + int cpu = perf_cpu_map__cpu(evsel->core.cpus, > > cpu_map_idx).cpu; > > int ret; > > > > list_for_each_entry(counter, &evsel->bpf_counter_list, > > list) { > > @@ -343,7 +344,7 @@ static int > > bpf_program_profiler__install_pe(struct evsel *evsel, int > > cpu_map_idx > > assert(skel != NULL); > > > > ret = bpf_map_update_elem(bpf_map__fd(skel- > > >maps.events), > > - &cpu_map_idx, &fd, > > BPF_ANY); > > + &cpu, &fd, BPF_ANY); > > if (ret) > > return ret; > > } > > @@ -451,7 +452,6 @@ static int bperf_check_target(struct evsel > > *evsel, > > return 0; > > } > > > > -static struct perf_cpu_map *all_cpu_map; > > static __u32 filter_entry_cnt; > > > > static int bperf_reload_leader_program(struct evsel *evsel, int > > attr_map_fd, > > @@ -495,7 +495,7 @@ static int bperf_reload_leader_program(struct > > evsel *evsel, int attr_map_fd, > > * following evsel__open_per_cpu call > > */ > > evsel->leader_skel = skel; > > - evsel__open_per_cpu(evsel, all_cpu_map, -1); > > + evsel__open(evsel, evsel->core.cpus, evsel->core.threads); > > > > out: > > bperf_leader_bpf__destroy(skel); > > @@ -533,12 +533,6 @@ static int bperf__load(struct evsel *evsel, > > struct target *target) > > if (bperf_check_target(evsel, target, &filter_type, > > &filter_entry_cnt)) > > return -1; > > > > - if (!all_cpu_map) { > > - all_cpu_map = perf_cpu_map__new_online_cpus(); > > - if (!all_cpu_map) > > - return -1; > > - } > > - > > evsel->bperf_leader_prog_fd = -1; > > evsel->bperf_leader_link_fd = -1; > > > > @@ -656,9 +650,10 @@ static int bperf__load(struct evsel *evsel, > > struct target *target) > > static int bperf__install_pe(struct evsel *evsel, int cpu_map_idx, > > int fd) > > { > > struct bperf_leader_bpf *skel = evsel->leader_skel; > > + int cpu = perf_cpu_map__cpu(evsel->core.cpus, > > cpu_map_idx).cpu; > > > > return bpf_map_update_elem(bpf_map__fd(skel->maps.events), > > - &cpu_map_idx, &fd, BPF_ANY); > > + &cpu, &fd, BPF_ANY); > > } > > > > /* > > @@ -667,13 +662,12 @@ static int bperf__install_pe(struct evsel > > *evsel, int cpu_map_idx, int fd) > > */ > > static int bperf_sync_counters(struct evsel *evsel) > > { > > - int num_cpu, i, cpu; > > + struct perf_cpu cpu; > > + int idx; > > + > > + perf_cpu_map__for_each_cpu(cpu, idx, evsel->core.cpus) > > + bperf_trigger_reading(evsel->bperf_leader_prog_fd, > > cpu.cpu); > > > > - num_cpu = perf_cpu_map__nr(all_cpu_map); > > - for (i = 0; i < num_cpu; i++) { > > - cpu = perf_cpu_map__cpu(all_cpu_map, i).cpu; > > - bperf_trigger_reading(evsel->bperf_leader_prog_fd, > > cpu); > > - } > > return 0; > > } > > > > diff --git a/tools/perf/util/bpf_counter_cgroup.c > > b/tools/perf/util/bpf_counter_cgroup.c > > index ed6a29b106b4..690be3ce3e11 100644 > > --- a/tools/perf/util/bpf_counter_cgroup.c > > +++ b/tools/perf/util/bpf_counter_cgroup.c > > @@ -186,7 +186,8 @@ static int bperf_cgrp__load(struct evsel > > *evsel, > > } > > > > static int bperf_cgrp__install_pe(struct evsel *evsel > > __maybe_unused, > > - int cpu __maybe_unused, int fd > > __maybe_unused) > > + int cpu_map_idx __maybe_unused, > > + int fd __maybe_unused) > > { > > /* nothing to do */ > > return 0; > > -- > > 2.51.0.618.g983fd99d29-goog > > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-06 21:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-01 18:12 [PATCH v1 1/2] perf bpf_counter: Move header declarations into C code Ian Rogers 2025-10-01 18:12 ` [PATCH v1 2/2] perf bpf_counter: Fix handling of cpumap fixing hybrid Ian Rogers 2025-10-06 16:18 ` Ian Rogers 2025-10-06 19:57 ` Arnaldo Carvalho de Melo 2025-10-06 21:20 ` Ian Rogers 2025-10-06 20:47 ` Falcon, Thomas
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).