[PATCH bpf-next v3 0/3] bpftool: Improve perf counter reporting

[PATCH bpf-next v3 0/3] bpftool: Improve perf counter reporting

[Mykyta Yatsenko]

Track valid snapshots that have a zero counter value. Scale each per-CPU
counter before aggregation. Report cycles per included program run.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
Changes in v3:
- Keep the JSON value field raw. Add value_scaled and run_cnt_valid.
- Use the same CPU set for all metrics.
- Move snapshot state tracking to a separate patch.
- Link to v2: https://patch.msgid.link/20260902-bpftool_cyles_per_run-v2-1-bc5d14ad0ce9@meta.com

Changes in v2:
- Scale all counters.
- Use an enum for metric indexes.
- Link to v1: https://patch.msgid.link/20260827-bpftool_cyles_per_run-v1-1-77d7bfc3c065@meta.com

---
Mykyta Yatsenko (3):
      bpftool: Track perf counter snapshot state
      perf bpf_counter: Track valid BPF counter snapshots
      bpftool: Scale counters and report cycles per run

 tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
 tools/bpf/bpftool/prog.c                         | 159 +++++++++++++++++------
 tools/bpf/bpftool/skeleton/profiler.bpf.c        |  25 ++--
 tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c |  26 ++--
 4 files changed, 168 insertions(+), 55 deletions(-)
---
base-commit: 3fa3128887a4df6057c543aae7c5c7fd2ed41f12
change-id: 20260827-bpftool_cyles_per_run-93169fcb30b7

Best regards,
--  
Mykyta Yatsenko <yatsenko@meta.com>

[PATCH bpf-next v3 1/3] bpftool: Track perf counter snapshot state

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

A perf counter can be zero at fentry. PMU multiplexing can schedule the
event during the BPF program. The old counter check then drops a valid
sample.

Use an armed flag to track each successful fentry snapshot. Reset all
flags before new reads. The fexit path clears each flag when it consumes
the snapshot.

Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/bpf/bpftool/skeleton/profiler.bpf.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
index f48c783cb9f7..6c654bd9b346 100644
--- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
+++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
@@ -10,6 +10,11 @@ struct bpf_perf_event_value___local {
 	__u64 running;
 } __attribute__((preserve_access_index));
 
+struct profile_reading {
+	struct bpf_perf_event_value___local value;
+	bool armed;
+};
+
 /* map of perf event fds, num_cpu * num_metric entries */
 struct {
 	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
@@ -21,7 +26,7 @@ struct {
 struct {
 	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
 	__uint(key_size, sizeof(u32));
-	__uint(value_size, sizeof(struct bpf_perf_event_value___local));
+	__uint(value_size, sizeof(struct profile_reading));
 } fentry_readings SEC(".maps");
 
 /* accumulated readings */
@@ -45,7 +50,7 @@ const volatile __u32 num_metric = 1;
 SEC("fentry/XXX")
 int BPF_PROG(fentry_XXX)
 {
-	struct bpf_perf_event_value___local *ptrs[MAX_NUM_METRICS];
+	struct profile_reading *ptrs[MAX_NUM_METRICS];
 	u32 key = bpf_get_smp_processor_id();
 	u32 i;
 
@@ -56,6 +61,7 @@ int BPF_PROG(fentry_XXX)
 		ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
 		if (!ptrs[i])
 			return 0;
+		ptrs[i]->armed = false;
 	}
 
 	for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
@@ -66,7 +72,8 @@ int BPF_PROG(fentry_XXX)
 						sizeof(reading));
 		if (err)
 			return 0;
-		*(ptrs[i]) = reading;
+		ptrs[i]->value = reading;
+		ptrs[i]->armed = true;
 		key += num_cpu;
 	}
 
@@ -76,16 +83,18 @@ int BPF_PROG(fentry_XXX)
 static inline void
 fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
 {
-	struct bpf_perf_event_value___local *before, diff;
+	struct profile_reading *before;
+	struct bpf_perf_event_value___local diff;
 
 	before = bpf_map_lookup_elem(&fentry_readings, &id);
 	/* only account samples with a valid fentry_reading */
-	if (before && before->counter) {
+	if (before && before->armed) {
 		struct bpf_perf_event_value___local *accum;
 
-		diff.counter = after->counter - before->counter;
-		diff.enabled = after->enabled - before->enabled;
-		diff.running = after->running - before->running;
+		before->armed = false;
+		diff.counter = after->counter - before->value.counter;
+		diff.enabled = after->enabled - before->value.enabled;
+		diff.running = after->running - before->value.running;
 
 		accum = bpf_map_lookup_elem(&accum_readings, &id);
 		if (accum) {

-- 
2.53.0-Meta

Re: [PATCH bpf-next v3 1/3] bpftool: Track perf counter snapshot state

[sashiko-bot]

> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> A perf counter can be zero at fentry. PMU multiplexing can schedule the
> event during the BPF program. The old counter check then drops a valid
> sample.
> 
> Use an armed flag to track each successful fentry snapshot. Reset all
> flags before new reads. The fexit path clears each flag when it consumes
> the snapshot.
> 
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-bpftool_cyles_per_run-v3-0-60e86f325c35@meta.com?part=1

Re: [PATCH bpf-next v3 1/3] bpftool: Track perf counter snapshot state

[Quentin Monnet]

2026-09-08 07:27 UTC-0700 ~ Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> A perf counter can be zero at fentry. PMU multiplexing can schedule the
> event during the BPF program. The old counter check then drops a valid
> sample.
> 
> Use an armed flag to track each successful fentry snapshot. Reset all
> flags before new reads. The fexit path clears each flag when it consumes
> the snapshot.
> 
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

Acked-by: Quentin Monnet <qmo@kernel.org>

Re: [PATCH bpf-next v3 1/3] bpftool: Track perf counter snapshot state

[Andrii Nakryiko]

On Tue, Sep 8, 2026 at 7:27 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> A perf counter can be zero at fentry. PMU multiplexing can schedule the
> event during the BPF program. The old counter check then drops a valid
> sample.
>
> Use an armed flag to track each successful fentry snapshot. Reset all
> flags before new reads. The fexit path clears each flag when it consumes
> the snapshot.
>
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  tools/bpf/bpftool/skeleton/profiler.bpf.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
> index f48c783cb9f7..6c654bd9b346 100644
> --- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
> +++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
> @@ -10,6 +10,11 @@ struct bpf_perf_event_value___local {
>         __u64 running;
>  } __attribute__((preserve_access_index));
>
> +struct profile_reading {
> +       struct bpf_perf_event_value___local value;
> +       bool armed;
> +};
> +
>  /* map of perf event fds, num_cpu * num_metric entries */
>  struct {
>         __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> @@ -21,7 +26,7 @@ struct {
>  struct {
>         __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
>         __uint(key_size, sizeof(u32));
> -       __uint(value_size, sizeof(struct bpf_perf_event_value___local));

this is not a bug, but... ;)


struct bpf_perf_event_value is UAPI, why do we need CO-RE-relocatable
___local variant?...


> +       __uint(value_size, sizeof(struct profile_reading));
>  } fentry_readings SEC(".maps");
>
>  /* accumulated readings */
> @@ -45,7 +50,7 @@ const volatile __u32 num_metric = 1;
>  SEC("fentry/XXX")
>  int BPF_PROG(fentry_XXX)
>  {
> -       struct bpf_perf_event_value___local *ptrs[MAX_NUM_METRICS];
> +       struct profile_reading *ptrs[MAX_NUM_METRICS];
>         u32 key = bpf_get_smp_processor_id();
>         u32 i;
>
> @@ -56,6 +61,7 @@ int BPF_PROG(fentry_XXX)
>                 ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
>                 if (!ptrs[i])
>                         return 0;
> +               ptrs[i]->armed = false;
>         }

this is preexisting, but why do we have two separate loops: first
lookup up fentry_readings pointers, and then separately a) reading
perf counters into local variable just to b) immediately copy it into
map_value.

can you try simplifying this and doing bpf_perf_event_read_value()
into ptrs[i] directly? all within the same loop?

it might have been some verifier issue, not sure, but I think this
should work just fine

>
>         for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
> @@ -66,7 +72,8 @@ int BPF_PROG(fentry_XXX)
>                                                 sizeof(reading));
>                 if (err)
>                         return 0;
> -               *(ptrs[i]) = reading;
> +               ptrs[i]->value = reading;
> +               ptrs[i]->armed = true;
>                 key += num_cpu;
>         }
>
> @@ -76,16 +83,18 @@ int BPF_PROG(fentry_XXX)
>  static inline void
>  fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
>  {
> -       struct bpf_perf_event_value___local *before, diff;
> +       struct profile_reading *before;
> +       struct bpf_perf_event_value___local diff;
>
>         before = bpf_map_lookup_elem(&fentry_readings, &id);
>         /* only account samples with a valid fentry_reading */
> -       if (before && before->counter) {
> +       if (before && before->armed) {

this is such an unlikely situation that I wouldn't even bother
"fixing" it, tbh. alternatively we can check enabled or running for
zero, I don't think realistically enabled can be zero if we actually
captured it an fentry

pw-bot: cr


>                 struct bpf_perf_event_value___local *accum;
>
> -               diff.counter = after->counter - before->counter;
> -               diff.enabled = after->enabled - before->enabled;
> -               diff.running = after->running - before->running;
> +               before->armed = false;
> +               diff.counter = after->counter - before->value.counter;
> +               diff.enabled = after->enabled - before->value.enabled;
> +               diff.running = after->running - before->value.running;
>
>                 accum = bpf_map_lookup_elem(&accum_readings, &id);
>                 if (accum) {
>
> --
> 2.53.0-Meta
>

Re: [PATCH bpf-next v3 1/3] bpftool: Track perf counter snapshot state

[Mykyta Yatsenko]

On 9/9/26 12:56 AM, Andrii Nakryiko wrote:
> On Tue, Sep 8, 2026 at 7:27 AM Mykyta Yatsenko
> <mykyta.yatsenko5@gmail.com> wrote:
>>
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> A perf counter can be zero at fentry. PMU multiplexing can schedule the
>> event during the BPF program. The old counter check then drops a valid
>> sample.
>>
>> Use an armed flag to track each successful fentry snapshot. Reset all
>> flags before new reads. The fexit path clears each flag when it consumes
>> the snapshot.
>>
>> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
>>  tools/bpf/bpftool/skeleton/profiler.bpf.c | 25 +++++++++++++++++--------
>>  1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
>> index f48c783cb9f7..6c654bd9b346 100644
>> --- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
>> +++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
>> @@ -10,6 +10,11 @@ struct bpf_perf_event_value___local {
>>         __u64 running;
>>  } __attribute__((preserve_access_index));
>>
>> +struct profile_reading {
>> +       struct bpf_perf_event_value___local value;
>> +       bool armed;
>> +};
>> +
>>  /* map of perf event fds, num_cpu * num_metric entries */
>>  struct {
>>         __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
>> @@ -21,7 +26,7 @@ struct {
>>  struct {
>>         __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
>>         __uint(key_size, sizeof(u32));
>> -       __uint(value_size, sizeof(struct bpf_perf_event_value___local));
> 
> this is not a bug, but... ;)
> 
> 
> struct bpf_perf_event_value is UAPI, why do we need CO-RE-relocatable
> ___local variant?...
> 
> 
>> +       __uint(value_size, sizeof(struct profile_reading));
>>  } fentry_readings SEC(".maps");
>>
>>  /* accumulated readings */
>> @@ -45,7 +50,7 @@ const volatile __u32 num_metric = 1;
>>  SEC("fentry/XXX")
>>  int BPF_PROG(fentry_XXX)
>>  {
>> -       struct bpf_perf_event_value___local *ptrs[MAX_NUM_METRICS];
>> +       struct profile_reading *ptrs[MAX_NUM_METRICS];
>>         u32 key = bpf_get_smp_processor_id();
>>         u32 i;
>>
>> @@ -56,6 +61,7 @@ int BPF_PROG(fentry_XXX)
>>                 ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
>>                 if (!ptrs[i])
>>                         return 0;
>> +               ptrs[i]->armed = false;
>>         }
> 
> this is preexisting, but why do we have two separate loops: first
> lookup up fentry_readings pointers, and then separately a) reading
> perf counters into local variable just to b) immediately copy it into
> map_value.
> 
> can you try simplifying this and doing bpf_perf_event_read_value()
> into ptrs[i] directly? all within the same loop?
> 
> it might have been some verifier issue, not sure, but I think this
> should work just fine
> 
>>
>>         for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
>> @@ -66,7 +72,8 @@ int BPF_PROG(fentry_XXX)
>>                                                 sizeof(reading));
>>                 if (err)
>>                         return 0;
>> -               *(ptrs[i]) = reading;
>> +               ptrs[i]->value = reading;
>> +               ptrs[i]->armed = true;
>>                 key += num_cpu;
>>         }
>>
>> @@ -76,16 +83,18 @@ int BPF_PROG(fentry_XXX)
>>  static inline void
>>  fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
>>  {
>> -       struct bpf_perf_event_value___local *before, diff;
>> +       struct profile_reading *before;
>> +       struct bpf_perf_event_value___local diff;
>>
>>         before = bpf_map_lookup_elem(&fentry_readings, &id);
>>         /* only account samples with a valid fentry_reading */
>> -       if (before && before->counter) {
>> +       if (before && before->armed) {
> 
> this is such an unlikely situation that I wouldn't even bother
> "fixing" it, tbh. alternatively we can check enabled or running for
> zero, I don't think realistically enabled can be zero if we actually
> captured it an fentry

Here we check before->counter for 0, substituting by enabled or running
will still have the same risk of dropping the first sample.

I could reproduce it by: 
1. make PMU busy with 4 events to force multiplexing
2. then quick profile for 1 second
Result: first sample gets dropped, because measurement
at fentry is 0. This is not a huge deal by itself, but the fix is simple enough,
in my opinion, to make it worth. For a pocket change we get clearer flow:
fentry arms the counter, fexit disarms it. 
We have the same code in perf, I thought it would be nice to have this change 
there (more choice of sparse events, subsecond timeouts possible)
> 
> pw-bot: cr
> 
> 
>>                 struct bpf_perf_event_value___local *accum;
>>
>> -               diff.counter = after->counter - before->counter;
>> -               diff.enabled = after->enabled - before->enabled;
>> -               diff.running = after->running - before->running;
>> +               before->armed = false;
>> +               diff.counter = after->counter - before->value.counter;
>> +               diff.enabled = after->enabled - before->value.enabled;
>> +               diff.running = after->running - before->value.running;
>>
>>                 accum = bpf_map_lookup_elem(&accum_readings, &id);
>>                 if (accum) {
>>
>> --
>> 2.53.0-Meta
>>

Re: [PATCH bpf-next v3 1/3] bpftool: Track perf counter snapshot state

[Andrii Nakryiko]

On Wed, Sep 9, 2026 at 2:19 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> On 9/9/26 12:56 AM, Andrii Nakryiko wrote:
> > On Tue, Sep 8, 2026 at 7:27 AM Mykyta Yatsenko
> > <mykyta.yatsenko5@gmail.com> wrote:
> >>
> >> From: Mykyta Yatsenko <yatsenko@meta.com>
> >>
> >> A perf counter can be zero at fentry. PMU multiplexing can schedule the
> >> event during the BPF program. The old counter check then drops a valid
> >> sample.
> >>
> >> Use an armed flag to track each successful fentry snapshot. Reset all
> >> flags before new reads. The fexit path clears each flag when it consumes
> >> the snapshot.
> >>
> >> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> >> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> >> ---
> >>  tools/bpf/bpftool/skeleton/profiler.bpf.c | 25 +++++++++++++++++--------
> >>  1 file changed, 17 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
> >> index f48c783cb9f7..6c654bd9b346 100644
> >> --- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
> >> +++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
> >> @@ -10,6 +10,11 @@ struct bpf_perf_event_value___local {
> >>         __u64 running;
> >>  } __attribute__((preserve_access_index));
> >>
> >> +struct profile_reading {
> >> +       struct bpf_perf_event_value___local value;
> >> +       bool armed;
> >> +};
> >> +
> >>  /* map of perf event fds, num_cpu * num_metric entries */
> >>  struct {
> >>         __uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
> >> @@ -21,7 +26,7 @@ struct {
> >>  struct {
> >>         __uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
> >>         __uint(key_size, sizeof(u32));
> >> -       __uint(value_size, sizeof(struct bpf_perf_event_value___local));
> >
> > this is not a bug, but... ;)
> >
> >
> > struct bpf_perf_event_value is UAPI, why do we need CO-RE-relocatable
> > ___local variant?...
> >
> >
> >> +       __uint(value_size, sizeof(struct profile_reading));
> >>  } fentry_readings SEC(".maps");
> >>
> >>  /* accumulated readings */
> >> @@ -45,7 +50,7 @@ const volatile __u32 num_metric = 1;
> >>  SEC("fentry/XXX")
> >>  int BPF_PROG(fentry_XXX)
> >>  {
> >> -       struct bpf_perf_event_value___local *ptrs[MAX_NUM_METRICS];
> >> +       struct profile_reading *ptrs[MAX_NUM_METRICS];
> >>         u32 key = bpf_get_smp_processor_id();
> >>         u32 i;
> >>
> >> @@ -56,6 +61,7 @@ int BPF_PROG(fentry_XXX)
> >>                 ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
> >>                 if (!ptrs[i])
> >>                         return 0;
> >> +               ptrs[i]->armed = false;
> >>         }
> >
> > this is preexisting, but why do we have two separate loops: first
> > lookup up fentry_readings pointers, and then separately a) reading
> > perf counters into local variable just to b) immediately copy it into
> > map_value.
> >
> > can you try simplifying this and doing bpf_perf_event_read_value()
> > into ptrs[i] directly? all within the same loop?
> >
> > it might have been some verifier issue, not sure, but I think this
> > should work just fine

you haven't answered this

> >
> >>
> >>         for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
> >> @@ -66,7 +72,8 @@ int BPF_PROG(fentry_XXX)
> >>                                                 sizeof(reading));
> >>                 if (err)
> >>                         return 0;
> >> -               *(ptrs[i]) = reading;
> >> +               ptrs[i]->value = reading;
> >> +               ptrs[i]->armed = true;
> >>                 key += num_cpu;
> >>         }
> >>
> >> @@ -76,16 +83,18 @@ int BPF_PROG(fentry_XXX)
> >>  static inline void
> >>  fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
> >>  {
> >> -       struct bpf_perf_event_value___local *before, diff;
> >> +       struct profile_reading *before;
> >> +       struct bpf_perf_event_value___local diff;
> >>
> >>         before = bpf_map_lookup_elem(&fentry_readings, &id);
> >>         /* only account samples with a valid fentry_reading */
> >> -       if (before && before->counter) {
> >> +       if (before && before->armed) {
> >
> > this is such an unlikely situation that I wouldn't even bother
> > "fixing" it, tbh. alternatively we can check enabled or running for
> > zero, I don't think realistically enabled can be zero if we actually
> > captured it an fentry
>
> Here we check before->counter for 0, substituting by enabled or running
> will still have the same risk of dropping the first sample.
>
> I could reproduce it by:
> 1. make PMU busy with 4 events to force multiplexing
> 2. then quick profile for 1 second
> Result: first sample gets dropped, because measurement
> at fentry is 0. This is not a huge deal by itself, but the fix is simple enough,
> in my opinion, to make it worth. For a pocket change we get clearer flow:
> fentry arms the counter, fexit disarms it.
> We have the same code in perf, I thought it would be nice to have this change
> there (more choice of sparse events, subsecond timeouts possible)
> >

are you saying that you get zero even for before->enabled?

> > pw-bot: cr
> >
> >
> >>                 struct bpf_perf_event_value___local *accum;
> >>
> >> -               diff.counter = after->counter - before->counter;
> >> -               diff.enabled = after->enabled - before->enabled;
> >> -               diff.running = after->running - before->running;
> >> +               before->armed = false;
> >> +               diff.counter = after->counter - before->value.counter;
> >> +               diff.enabled = after->enabled - before->value.enabled;
> >> +               diff.running = after->running - before->value.running;
> >>
> >>                 accum = bpf_map_lookup_elem(&accum_readings, &id);
> >>                 if (accum) {
> >>
> >> --
> >> 2.53.0-Meta
> >>
>

[PATCH bpf-next v3 2/3] perf bpf_counter: Track valid BPF counter snapshots

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

The perf profiler uses a zero counter value to reject an invalid entry
snapshot. However, zero is valid when the PMU has not scheduled the event.

To reproduce the issue, fill the PMU with other events and profile a
long-running BPF program for iTLB misses. In this test, the entry read
returned 0.

Use an armed flag to track successful entry reads. Reset the flag before
each new read. The fexit path clears the flag when it consumes the
snapshot.

Fixes: fa853c4b839e ("perf stat: Enable counting events for BPF programs")
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c | 26 ++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c b/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
index 97037d3b3d9f..7bc0db26e005 100644
--- a/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
+++ b/tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c
@@ -4,6 +4,11 @@
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_tracing.h>
 
+struct profile_reading {
+	struct bpf_perf_event_value value;
+	bool armed;
+};
+
 /* map of perf event fds, num_cpu * num_metric entries */
 struct {
 	__uint(type, BPF_MAP_TYPE_PERF_EVENT_ARRAY);
@@ -15,7 +20,7 @@ struct {
 struct {
 	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
 	__uint(key_size, sizeof(__u32));
-	__uint(value_size, sizeof(struct bpf_perf_event_value));
+	__uint(value_size, sizeof(struct profile_reading));
 	__uint(max_entries, 1);
 } fentry_readings SEC(".maps");
 
@@ -33,7 +38,7 @@ SEC("fentry/XXX")
 int BPF_PROG(fentry_XXX)
 {
 	__u32 key = bpf_get_smp_processor_id();
-	struct bpf_perf_event_value *ptr;
+	struct profile_reading *ptr;
 	__u32 zero = 0;
 	long err;
 
@@ -42,9 +47,12 @@ int BPF_PROG(fentry_XXX)
 	if (!ptr)
 		return 0;
 
-	err = bpf_perf_event_read_value(&events, key, ptr, sizeof(*ptr));
+	ptr->armed = false;
+	err = bpf_perf_event_read_value(&events, key, &ptr->value,
+					sizeof(ptr->value));
 	if (err)
 		return 0;
+	ptr->armed = true;
 
 	return 0;
 }
@@ -52,17 +60,19 @@ int BPF_PROG(fentry_XXX)
 static inline void
 fexit_update_maps(struct bpf_perf_event_value *after)
 {
-	struct bpf_perf_event_value *before, diff;
+	struct profile_reading *before;
+	struct bpf_perf_event_value diff;
 	__u32 zero = 0;
 
 	before = bpf_map_lookup_elem(&fentry_readings, &zero);
 	/* only account samples with a valid fentry_reading */
-	if (before && before->counter) {
+	if (before && before->armed) {
 		struct bpf_perf_event_value *accum;
 
-		diff.counter = after->counter - before->counter;
-		diff.enabled = after->enabled - before->enabled;
-		diff.running = after->running - before->running;
+		before->armed = false;
+		diff.counter = after->counter - before->value.counter;
+		diff.enabled = after->enabled - before->value.enabled;
+		diff.running = after->running - before->value.running;
 
 		accum = bpf_map_lookup_elem(&accum_readings, &zero);
 		if (accum) {

-- 
2.53.0-Meta

Re: [PATCH bpf-next v3 2/3] perf bpf_counter: Track valid BPF counter snapshots

[sashiko-bot]

> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> The perf profiler uses a zero counter value to reject an invalid entry
> snapshot. However, zero is valid when the PMU has not scheduled the event.
> 
> To reproduce the issue, fill the PMU with other events and profile a
> long-running BPF program for iTLB misses. In this test, the entry read
> returned 0.
> 
> Use an armed flag to track successful entry reads. Reset the flag before
> each new read. The fexit path clears the flag when it consumes the
> snapshot.
> 
> Fixes: fa853c4b839e ("perf stat: Enable counting events for BPF programs")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-bpftool_cyles_per_run-v3-0-60e86f325c35@meta.com?part=2

[PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Perf counters can report too few events when the PMU multiplexes them.
Scale each per-CPU value before aggregation.

Use the same CPU set for derived ratios. Report cycles per included
program run, and preserve the total run count in JSON.

Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
 tools/bpf/bpftool/prog.c                         | 159 +++++++++++++++++------
 2 files changed, 133 insertions(+), 39 deletions(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 90fa2a48cc26..0108a1c8be4d 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -217,7 +217,16 @@ bpftool prog run *PROG* data_in *FILE* [data_out *FILE* [data_size_out *L*]] [ct
 bpftool prog profile *PROG* [duration *DURATION*] *METRICs*
     Profile *METRICs* for bpf program *PROG* for *DURATION* seconds or until
     user hits <Ctrl+C>. *DURATION* is optional. If *DURATION* is not specified,
-    the profiling will run up to **UINT_MAX** seconds.
+    the profiling will run up to **UINT_MAX** seconds. Plain output scales each
+    per-CPU metric value to correct for perf event multiplexing. When
+    **cycles** is selected, it also reports cycles per included program run. If
+    a selected metric was not scheduled on a CPU, all metric values exclude
+    that CPU so that ratios use a consistent CPU set. The **run_cnt** value
+    still includes all recorded runs.
+
+    In JSON output, **value** is raw, **value_scaled** is scaled, **run_cnt**
+    includes all runs, and **run_cnt_valid** includes only runs used for metric
+    values.
 
 bpftool prog help
     Print short help message.
@@ -360,7 +369,7 @@ EXAMPLES
 ::
 
          51397 run_cnt
-      40176203 cycles                                                 (83.05%)
+      40176203 cycles          # 781.68 cycles per run                (83.05%)
       42518139 instructions    #   1.06 insns per cycle               (83.39%)
            123 llc_misses      #   2.89 LLC misses per million insns  (83.15%)
 
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 8c2f9255b36d..9e126f3823ad 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2062,37 +2062,52 @@ static int do_profile(int argc, char **argv)
 
 #include "profiler.skel.h"
 
+enum ratio_metric {
+	METRIC_NONE = -2,
+	METRIC_RUN_CNT = -1,
+	METRIC_CYCLES = 0,
+	METRIC_INSTRUCTIONS = 1,
+	METRIC_L1D_LOADS = 2,
+	METRIC_LLC_MISSES = 3,
+	METRIC_ITLB_MISSES = 4,
+	METRIC_DTLB_MISSES = 5,
+};
+
 struct profile_metric {
 	const char *name;
 	struct bpf_perf_event_value val;
+	__u64 scaled_val;
 	struct perf_event_attr attr;
 	bool selected;
 
 	/* calculate ratios like instructions per cycle */
-	const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
+	const enum ratio_metric ratio_metric;
 	const char *ratio_desc;
 	const float ratio_mul;
 } metrics[] = {
-	{
+	[METRIC_CYCLES] = {
 		.name = "cycles",
 		.attr = {
 			.type = PERF_TYPE_HARDWARE,
 			.config = PERF_COUNT_HW_CPU_CYCLES,
 			.exclude_user = 1,
 		},
+		.ratio_metric = METRIC_RUN_CNT,
+		.ratio_desc = "cycles per run",
+		.ratio_mul = 1.0,
 	},
-	{
+	[METRIC_INSTRUCTIONS] = {
 		.name = "instructions",
 		.attr = {
 			.type = PERF_TYPE_HARDWARE,
 			.config = PERF_COUNT_HW_INSTRUCTIONS,
 			.exclude_user = 1,
 		},
-		.ratio_metric = 1,
+		.ratio_metric = METRIC_CYCLES,
 		.ratio_desc = "insns per cycle",
 		.ratio_mul = 1.0,
 	},
-	{
+	[METRIC_L1D_LOADS] = {
 		.name = "l1d_loads",
 		.attr = {
 			.type = PERF_TYPE_HW_CACHE,
@@ -2102,8 +2117,9 @@ struct profile_metric {
 				(PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
 			.exclude_user = 1,
 		},
+		.ratio_metric = METRIC_NONE,
 	},
-	{
+	[METRIC_LLC_MISSES] = {
 		.name = "llc_misses",
 		.attr = {
 			.type = PERF_TYPE_HW_CACHE,
@@ -2113,11 +2129,11 @@ struct profile_metric {
 				(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
 			.exclude_user = 1
 		},
-		.ratio_metric = 2,
+		.ratio_metric = METRIC_INSTRUCTIONS,
 		.ratio_desc = "LLC misses per million insns",
 		.ratio_mul = 1e6,
 	},
-	{
+	[METRIC_ITLB_MISSES] = {
 		.name = "itlb_misses",
 		.attr = {
 			.type = PERF_TYPE_HW_CACHE,
@@ -2127,11 +2143,11 @@ struct profile_metric {
 				(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
 			.exclude_user = 1
 		},
-		.ratio_metric = 2,
+		.ratio_metric = METRIC_INSTRUCTIONS,
 		.ratio_desc = "itlb misses per million insns",
 		.ratio_mul = 1e6,
 	},
-	{
+	[METRIC_DTLB_MISSES] = {
 		.name = "dtlb_misses",
 		.attr = {
 			.type = PERF_TYPE_HW_CACHE,
@@ -2141,13 +2157,14 @@ struct profile_metric {
 				(PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
 			.exclude_user = 1
 		},
-		.ratio_metric = 2,
+		.ratio_metric = METRIC_INSTRUCTIONS,
 		.ratio_desc = "dtlb misses per million insns",
 		.ratio_mul = 1e6,
 	},
 };
 
 static __u64 profile_total_count;
+static __u64 profile_valid_count;
 
 #define MAX_NUM_PROFILE_METRICS 4
 
@@ -2182,9 +2199,39 @@ static int profile_parse_metrics(int argc, char **argv)
 	return selected_cnt;
 }
 
-static void profile_read_values(struct profiler_bpf *obj)
+/*
+ * Filter out CPUs that have any selected metric not scheduled for them. This makes sure all
+ * metrics are using the same CPU set, as a result ratio metrics are consistent.
+ */
+static void profile_filter_cpus(__u32 num_cpu,
+				struct bpf_perf_event_value vals[MAX_NUM_PROFILE_METRICS][num_cpu],
+				__u64 *counts)
+{
+	__u32 m, cpu, key = 0;
+
+	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+		if (!metrics[m].selected)
+			continue;
+
+		for (cpu = 0; cpu < num_cpu; cpu++) {
+			/*
+			 * CPU has hits, but this metric never scheduled, set counts[cpu] to 0
+			 * so other metrics ignore this CPU too
+			 */
+			if (counts[cpu] && !vals[key][cpu].running) {
+				p_info("%s not scheduled on CPU %u; excluding %llu runs from all metrics",
+				       metrics[m].name, cpu, counts[cpu]);
+				counts[cpu] = 0;
+			}
+		}
+		key++;
+	}
+}
+
+static int profile_read_values(struct profiler_bpf *obj)
 {
 	__u32 m, cpu, num_cpu = obj->rodata->num_cpu;
+	struct bpf_perf_event_value values[MAX_NUM_PROFILE_METRICS][num_cpu], *val;
 	int reading_map_fd, count_map_fd;
 	__u64 counts[num_cpu];
 	__u32 key = 0;
@@ -2194,38 +2241,61 @@ static void profile_read_values(struct profiler_bpf *obj)
 	count_map_fd = bpf_map__fd(obj->maps.counts);
 	if (reading_map_fd < 0 || count_map_fd < 0) {
 		p_err("failed to get fd for map");
-		return;
+		return min(reading_map_fd, count_map_fd);
 	}
 
 	err = bpf_map_lookup_elem(count_map_fd, &key, counts);
 	if (err) {
 		p_err("failed to read count_map: %s", strerror(errno));
-		return;
+		return err;
+	}
+
+	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+		if (!metrics[m].selected)
+			continue;
+
+		err = bpf_map_lookup_elem(reading_map_fd, &key, values[key]);
+		if (err) {
+			p_err("failed to read reading_map: %s", strerror(errno));
+			return err;
+		}
+		key++;
 	}
 
 	profile_total_count = 0;
 	for (cpu = 0; cpu < num_cpu; cpu++)
 		profile_total_count += counts[cpu];
 
+	profile_filter_cpus(num_cpu, values, counts);
+
+	profile_valid_count = 0;
+	for (cpu = 0; cpu < num_cpu; cpu++)
+		profile_valid_count += counts[cpu];
+
+	key = 0;
 	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
-		struct bpf_perf_event_value values[num_cpu];
+		double scale;
 
 		if (!metrics[m].selected)
 			continue;
 
-		err = bpf_map_lookup_elem(reading_map_fd, &key, values);
-		if (err) {
-			p_err("failed to read reading_map: %s",
-			      strerror(errno));
-			return;
-		}
 		for (cpu = 0; cpu < num_cpu; cpu++) {
-			metrics[m].val.counter += values[cpu].counter;
-			metrics[m].val.enabled += values[cpu].enabled;
-			metrics[m].val.running += values[cpu].running;
+			/* Skip CPUs with no runs or with an unscheduled metric. */
+			if (!counts[cpu])
+				continue;
+
+			val = &values[key][cpu];
+
+			metrics[m].val.enabled += val->enabled;
+			metrics[m].val.running += val->running;
+			metrics[m].val.counter += val->counter;
+			/* Scale counter values to account for perf event multiplexing. */
+			scale = (double)val->enabled / val->running;
+			metrics[m].scaled_val += val->counter * scale;
 		}
 		key++;
 	}
+	return 0;
 }
 
 static void profile_print_readings_json(void)
@@ -2239,9 +2309,11 @@ static void profile_print_readings_json(void)
 		jsonw_start_object(json_wtr);
 		jsonw_string_field(json_wtr, "metric", metrics[m].name);
 		jsonw_lluint_field(json_wtr, "run_cnt", profile_total_count);
+		jsonw_lluint_field(json_wtr, "run_cnt_valid", profile_valid_count);
 		jsonw_lluint_field(json_wtr, "value", metrics[m].val.counter);
 		jsonw_lluint_field(json_wtr, "enabled", metrics[m].val.enabled);
 		jsonw_lluint_field(json_wtr, "running", metrics[m].val.running);
+		jsonw_lluint_field(json_wtr, "value_scaled", metrics[m].scaled_val);
 
 		jsonw_end_object(json_wtr);
 	}
@@ -2250,24 +2322,34 @@ static void profile_print_readings_json(void)
 
 static void profile_print_readings_plain(void)
 {
-	__u32 m;
+	__u32 i;
 
 	printf("\n%18llu %-20s\n", profile_total_count, "run_cnt");
-	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
-		struct bpf_perf_event_value *val = &metrics[m].val;
+	for (i = 0; i < ARRAY_SIZE(metrics); i++) {
+		struct profile_metric *m = &metrics[i];
+		struct bpf_perf_event_value *val = &m->val;
 		int r;
+		__u64 ratio;
 
-		if (!metrics[m].selected)
+		if (!m->selected)
 			continue;
-		printf("%18llu %-20s", val->counter, metrics[m].name);
+		printf("%18llu %-20s", m->scaled_val, m->name);
 
-		r = metrics[m].ratio_metric - 1;
-		if (r >= 0 && metrics[r].selected &&
-		    metrics[r].val.counter > 0) {
+		r = m->ratio_metric;
+		switch (r) {
+		case METRIC_RUN_CNT:
+			ratio = profile_valid_count;
+			break;
+		case METRIC_NONE:
+			ratio = 0;
+			break;
+		default:
+			ratio = metrics[r].scaled_val;
+		}
+		if (ratio) {
 			printf("# %8.2f %-30s",
-			       val->counter * metrics[m].ratio_mul /
-			       metrics[r].val.counter,
-			       metrics[m].ratio_desc);
+			       m->scaled_val * m->ratio_mul / ratio,
+			       m->ratio_desc);
 		} else {
 			printf("%-41s", "");
 		}
@@ -2423,9 +2505,12 @@ static int profile_open_perf_events(struct profiler_bpf *obj)
 
 static void profile_print_and_cleanup(void)
 {
+	int err;
+
 	profile_close_perf_events(profile_obj);
-	profile_read_values(profile_obj);
-	profile_print_readings();
+	err = profile_read_values(profile_obj);
+	if (!err)
+		profile_print_readings();
 	profiler_bpf__destroy(profile_obj);
 
 	close(profile_tgt_fd);

-- 
2.53.0-Meta

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[sashiko-bot]

> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Perf counters can report too few events when the PMU multiplexes them.
> Scale each per-CPU value before aggregation.
> 
> Use the same CPU set for derived ratios. Report cycles per included
> program run, and preserve the total run count in JSON.
> 
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-bpftool_cyles_per_run-v3-0-60e86f325c35@meta.com?part=3

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 7699 bytes --]

> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 8c2f9255b36d..9e126f3823ad 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c

[ ... ]

> @@ -2182,9 +2199,39 @@ static int profile_parse_metrics(int argc, char **argv)
>  	return selected_cnt;
>  }
>
> +/*
> + * Filter out CPUs that have any selected metric not scheduled for them. This makes sure all
> + * metrics are using the same CPU set, as a result ratio metrics are consistent.
> + */
> +static void profile_filter_cpus(__u32 num_cpu,
> +				struct bpf_perf_event_value vals[MAX_NUM_PROFILE_METRICS][num_cpu],
> +				__u64 *counts)
> +{
> +	__u32 m, cpu, key = 0;
> +
> +	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> +		if (!metrics[m].selected)
> +			continue;
> +
> +		for (cpu = 0; cpu < num_cpu; cpu++) {
> +			/*
> +			 * CPU has hits, but this metric never scheduled, set counts[cpu] to 0
> +			 * so other metrics ignore this CPU too
> +			 */
> +			if (counts[cpu] && !vals[key][cpu].running) {
> +				p_info("%s not scheduled on CPU %u; excluding %llu runs from all metrics",
> +				       metrics[m].name, cpu, counts[cpu]);
> +				counts[cpu] = 0;
> +			}
> +		}
> +		key++;
> +	}
> +}

[ ... ]

> @@ -2194,38 +2241,61 @@ static void profile_read_values(struct profiler_bpf *obj)
>  	count_map_fd = bpf_map__fd(obj->maps.counts);
>  	if (reading_map_fd < 0 || count_map_fd < 0) {
>  		p_err("failed to get fd for map");
> -		return;
> +		return min(reading_map_fd, count_map_fd);
>  	}
>
>  	err = bpf_map_lookup_elem(count_map_fd, &key, counts);
>  	if (err) {
>  		p_err("failed to read count_map: %s", strerror(errno));
> -		return;
> +		return err;
> +	}
> +
> +	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> +		if (!metrics[m].selected)
> +			continue;
> +
> +		err = bpf_map_lookup_elem(reading_map_fd, &key, values[key]);
> +		if (err) {
> +			p_err("failed to read reading_map: %s", strerror(errno));
> +			return err;
> +		}
> +		key++;
>  	}
>
>  	profile_total_count = 0;
>  	for (cpu = 0; cpu < num_cpu; cpu++)
>  		profile_total_count += counts[cpu];
>
> +	profile_filter_cpus(num_cpu, values, counts);
> +
> +	profile_valid_count = 0;
> +	for (cpu = 0; cpu < num_cpu; cpu++)
> +		profile_valid_count += counts[cpu];
> +
> +	key = 0;
>  	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> -		struct bpf_perf_event_value values[num_cpu];
> +		double scale;
>
>  		if (!metrics[m].selected)
>  			continue;
>
> -		err = bpf_map_lookup_elem(reading_map_fd, &key, values);
> -		if (err) {
> -			p_err("failed to read reading_map: %s",
> -			      strerror(errno));
> -			return;
> -		}
>  		for (cpu = 0; cpu < num_cpu; cpu++) {
> -			metrics[m].val.counter += values[cpu].counter;
> -			metrics[m].val.enabled += values[cpu].enabled;
> -			metrics[m].val.running += values[cpu].running;
> +			/* Skip CPUs with no runs or with an unscheduled metric. */
> +			if (!counts[cpu])
> +				continue;
> +
> +			val = &values[key][cpu];
> +
> +			metrics[m].val.enabled += val->enabled;
> +			metrics[m].val.running += val->running;
> +			metrics[m].val.counter += val->counter;
> +			/* Scale counter values to account for perf event multiplexing. */
> +			scale = (double)val->enabled / val->running;
> +			metrics[m].scaled_val += val->counter * scale;
>  		}
>  		key++;
>  	}
> +	return 0;
>  }

Does this correctly handle the case where one metric is never scheduled
on any CPU?

The CPU-set filter in profile_filter_cpus() zeroes counts[cpu] for every
CPU where any selected metric has running == 0:

    if (counts[cpu] && !vals[key][cpu].running) {
        p_info("%s not scheduled on CPU %u; ...");
        counts[cpu] = 0;
    }

The aggregation loop then skips those CPUs for all metrics:

    for (cpu = 0; cpu < num_cpu; cpu++) {
        if (!counts[cpu])
            continue;
        ...
        metrics[m].val.counter += val->counter;
        ...
        metrics[m].scaled_val += val->counter * scale;
    }

If a selected metric is never scheduled on any CPU (the scenario this
series targets - commit 44f919dd1fbc describes profiling iTLB misses on
a long-running program with a full PMU), then every CPU with counts[cpu]
!= 0 gets zeroed.

For example, `bpftool prog profile <prog> cycles itlb_misses`:

  * profile_valid_count becomes 0
  * cycles' val.counter and scaled_val stay 0 even though the cycles
    event was measured correctly on every CPU
  * plain output prints "run_cnt 51397" followed by "0 cycles" and
    "0 itlb_misses" with no ratio column

Before this patch the same run reported the real cycles count and 0 for
itlb_misses, so the healthy metric's data is lost.

In JSON mode there is no diagnostic at all. The p_info() call in
profile_filter_cpus() returns immediately when json_output is set, so
a consumer sees run_cnt=51397 with value=0, value_scaled=0, enabled=0,
running=0, and only run_cnt_valid=0 as a hint.

The commit message scopes the change to ratios ("Use the same CPU set
for derived ratios"), and the .rst says "all metric values exclude that
CPU so that ratios use a consistent CPU set" but neither documents that
a single unscheduled metric collapses the whole report to zeros, and
JSON gives no warning.

Would it make sense to exclude only the affected metric, or keep the raw
per-metric aggregation intact and restrict only the ratio denominators?

> @@ -2250,24 +2322,34 @@ static void profile_print_readings_json(void)
>
>  static void profile_print_readings_plain(void)
>  {
> -	__u32 m;
> +	__u32 i;
>
>  	printf("\n%18llu %-20s\n", profile_total_count, "run_cnt");
> -	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> -		struct bpf_perf_event_value *val = &metrics[m].val;
> +	for (i = 0; i < ARRAY_SIZE(metrics); i++) {
> +		struct profile_metric *m = &metrics[i];
> +		struct bpf_perf_event_value *val = &m->val;
>  		int r;
> +		__u64 ratio;
>
> -		if (!metrics[m].selected)
> +		if (!m->selected)
>  			continue;
> -		printf("%18llu %-20s", val->counter, metrics[m].name);
> +		printf("%18llu %-20s", m->scaled_val, m->name);
>
> -		r = metrics[m].ratio_metric - 1;
> -		if (r >= 0 && metrics[r].selected &&
> -		    metrics[r].val.counter > 0) {
> +		r = m->ratio_metric;
> +		switch (r) {
> +		case METRIC_RUN_CNT:
> +			ratio = profile_valid_count;
> +			break;
> +		case METRIC_NONE:
> +			ratio = 0;
> +			break;
> +		default:
> +			ratio = metrics[r].scaled_val;
> +		}
> +		if (ratio) {
>  			printf("# %8.2f %-30s",
> -			       val->counter * metrics[m].ratio_mul /
> -			       metrics[r].val.counter,
> -			       metrics[m].ratio_desc);
> +			       m->scaled_val * m->ratio_mul / ratio,
> +			       m->ratio_desc);
>  		} else {
>  			printf("%-41s", "");
>  		}

Plain output prints profile_total_count as "run_cnt" but the new "cycles
per run" column divides by profile_valid_count, and profile_valid_count
is never printed in plain mode (it is only exposed as the JSON
run_cnt_valid field).

Whenever profile_filter_cpus() excludes at least one CPU the two differ,
so the displayed numbers cannot be reconciled. The user sees run_cnt and
a scaled cycles value, and "# <x> cycles per run" where
x != cycles/run_cnt, with no way to see the denominator that was
actually used.

The only signal is the p_info() lines on stderr, which are easy to miss
when stdout is redirected and are suppressed entirely under --json.

Would it make sense to print the valid run count in plain mode (as JSON
now does) or label the ratio with the CPU set it was computed over?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34246064212

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[Mykyta Yatsenko]

On 9/8/26 5:21 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> index 8c2f9255b36d..9e126f3823ad 100644
>> --- a/tools/bpf/bpftool/prog.c
>> +++ b/tools/bpf/bpftool/prog.c
> 
> [ ... ]
> 
>> @@ -2182,9 +2199,39 @@ static int profile_parse_metrics(int argc, char **argv)
>>  	return selected_cnt;
>>  }
>>
>> +/*
>> + * Filter out CPUs that have any selected metric not scheduled for them. This makes sure all
>> + * metrics are using the same CPU set, as a result ratio metrics are consistent.
>> + */
>> +static void profile_filter_cpus(__u32 num_cpu,
>> +				struct bpf_perf_event_value vals[MAX_NUM_PROFILE_METRICS][num_cpu],
>> +				__u64 *counts)
>> +{
>> +	__u32 m, cpu, key = 0;
>> +
>> +	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> +		if (!metrics[m].selected)
>> +			continue;
>> +
>> +		for (cpu = 0; cpu < num_cpu; cpu++) {
>> +			/*
>> +			 * CPU has hits, but this metric never scheduled, set counts[cpu] to 0
>> +			 * so other metrics ignore this CPU too
>> +			 */
>> +			if (counts[cpu] && !vals[key][cpu].running) {
>> +				p_info("%s not scheduled on CPU %u; excluding %llu runs from all metrics",
>> +				       metrics[m].name, cpu, counts[cpu]);
>> +				counts[cpu] = 0;
>> +			}
>> +		}
>> +		key++;
>> +	}
>> +}
> 
> [ ... ]
> 
>> @@ -2194,38 +2241,61 @@ static void profile_read_values(struct profiler_bpf *obj)
>>  	count_map_fd = bpf_map__fd(obj->maps.counts);
>>  	if (reading_map_fd < 0 || count_map_fd < 0) {
>>  		p_err("failed to get fd for map");
>> -		return;
>> +		return min(reading_map_fd, count_map_fd);
>>  	}
>>
>>  	err = bpf_map_lookup_elem(count_map_fd, &key, counts);
>>  	if (err) {
>>  		p_err("failed to read count_map: %s", strerror(errno));
>> -		return;
>> +		return err;
>> +	}
>> +
>> +	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> +		if (!metrics[m].selected)
>> +			continue;
>> +
>> +		err = bpf_map_lookup_elem(reading_map_fd, &key, values[key]);
>> +		if (err) {
>> +			p_err("failed to read reading_map: %s", strerror(errno));
>> +			return err;
>> +		}
>> +		key++;
>>  	}
>>
>>  	profile_total_count = 0;
>>  	for (cpu = 0; cpu < num_cpu; cpu++)
>>  		profile_total_count += counts[cpu];
>>
>> +	profile_filter_cpus(num_cpu, values, counts);
>> +
>> +	profile_valid_count = 0;
>> +	for (cpu = 0; cpu < num_cpu; cpu++)
>> +		profile_valid_count += counts[cpu];
>> +
>> +	key = 0;
>>  	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> -		struct bpf_perf_event_value values[num_cpu];
>> +		double scale;
>>
>>  		if (!metrics[m].selected)
>>  			continue;
>>
>> -		err = bpf_map_lookup_elem(reading_map_fd, &key, values);
>> -		if (err) {
>> -			p_err("failed to read reading_map: %s",
>> -			      strerror(errno));
>> -			return;
>> -		}
>>  		for (cpu = 0; cpu < num_cpu; cpu++) {
>> -			metrics[m].val.counter += values[cpu].counter;
>> -			metrics[m].val.enabled += values[cpu].enabled;
>> -			metrics[m].val.running += values[cpu].running;
>> +			/* Skip CPUs with no runs or with an unscheduled metric. */
>> +			if (!counts[cpu])
>> +				continue;
>> +
>> +			val = &values[key][cpu];
>> +
>> +			metrics[m].val.enabled += val->enabled;
>> +			metrics[m].val.running += val->running;
>> +			metrics[m].val.counter += val->counter;
>> +			/* Scale counter values to account for perf event multiplexing. */
>> +			scale = (double)val->enabled / val->running;
>> +			metrics[m].scaled_val += val->counter * scale;
>>  		}
>>  		key++;
>>  	}
>> +	return 0;
>>  }
> 
> Does this correctly handle the case where one metric is never scheduled
> on any CPU?
> 
> The CPU-set filter in profile_filter_cpus() zeroes counts[cpu] for every
> CPU where any selected metric has running == 0:

This would be an unusual edge case, perf counter normally get 
scheduled for some fraction of time, even when multiplexed.

>     if (counts[cpu] && !vals[key][cpu].running) {
>         p_info("%s not scheduled on CPU %u; ...");
>         counts[cpu] = 0;
>     }
> 
> The aggregation loop then skips those CPUs for all metrics:
> 
>     for (cpu = 0; cpu < num_cpu; cpu++) {
>         if (!counts[cpu])
>             continue;
>         ...
>         metrics[m].val.counter += val->counter;
>         ...
>         metrics[m].scaled_val += val->counter * scale;
>     }
> 
> If a selected metric is never scheduled on any CPU (the scenario this
> series targets - commit 44f919dd1fbc describes profiling iTLB misses on
> a long-running program with a full PMU), then every CPU with counts[cpu]
> != 0 gets zeroed.
> 
> For example, `bpftool prog profile <prog> cycles itlb_misses`:
> 
>   * profile_valid_count becomes 0
>   * cycles' val.counter and scaled_val stay 0 even though the cycles
>     event was measured correctly on every CPU
>   * plain output prints "run_cnt 51397" followed by "0 cycles" and
>     "0 itlb_misses" with no ratio column
> 
> Before this patch the same run reported the real cycles count and 0 for
> itlb_misses, so the healthy metric's data is lost.
> 
> In JSON mode there is no diagnostic at all. The p_info() call in
> profile_filter_cpus() returns immediately when json_output is set, so
> a consumer sees run_cnt=51397 with value=0, value_scaled=0, enabled=0,
> running=0, and only run_cnt_valid=0 as a hint.
> 
> The commit message scopes the change to ratios ("Use the same CPU set
> for derived ratios"), and the .rst says "all metric values exclude that
> CPU so that ratios use a consistent CPU set" but neither documents that
> a single unscheduled metric collapses the whole report to zeros, and
> JSON gives no warning.
> 
> Would it make sense to exclude only the affected metric, or keep the raw
> per-metric aggregation intact and restrict only the ratio denominators?
> 
>> @@ -2250,24 +2322,34 @@ static void profile_print_readings_json(void)
>>
>>  static void profile_print_readings_plain(void)
>>  {
>> -	__u32 m;
>> +	__u32 i;
>>
>>  	printf("\n%18llu %-20s\n", profile_total_count, "run_cnt");
>> -	for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> -		struct bpf_perf_event_value *val = &metrics[m].val;
>> +	for (i = 0; i < ARRAY_SIZE(metrics); i++) {
>> +		struct profile_metric *m = &metrics[i];
>> +		struct bpf_perf_event_value *val = &m->val;
>>  		int r;
>> +		__u64 ratio;
>>
>> -		if (!metrics[m].selected)
>> +		if (!m->selected)
>>  			continue;
>> -		printf("%18llu %-20s", val->counter, metrics[m].name);
>> +		printf("%18llu %-20s", m->scaled_val, m->name);
>>
>> -		r = metrics[m].ratio_metric - 1;
>> -		if (r >= 0 && metrics[r].selected &&
>> -		    metrics[r].val.counter > 0) {
>> +		r = m->ratio_metric;
>> +		switch (r) {
>> +		case METRIC_RUN_CNT:
>> +			ratio = profile_valid_count;
>> +			break;
>> +		case METRIC_NONE:
>> +			ratio = 0;
>> +			break;
>> +		default:
>> +			ratio = metrics[r].scaled_val;
>> +		}
>> +		if (ratio) {
>>  			printf("# %8.2f %-30s",
>> -			       val->counter * metrics[m].ratio_mul /
>> -			       metrics[r].val.counter,
>> -			       metrics[m].ratio_desc);
>> +			       m->scaled_val * m->ratio_mul / ratio,
>> +			       m->ratio_desc);
>>  		} else {
>>  			printf("%-41s", "");
>>  		}
> 
> Plain output prints profile_total_count as "run_cnt" but the new "cycles
> per run" column divides by profile_valid_count, and profile_valid_count
> is never printed in plain mode (it is only exposed as the JSON
> run_cnt_valid field).
> 
> Whenever profile_filter_cpus() excludes at least one CPU the two differ,
> so the displayed numbers cannot be reconciled. The user sees run_cnt and
> a scaled cycles value, and "# <x> cycles per run" where
> x != cycles/run_cnt, with no way to see the denominator that was
> actually used.
> 
> The only signal is the p_info() lines on stderr, which are easy to miss
> when stdout is redirected and are suppressed entirely under --json.
> 
> Would it make sense to print the valid run count in plain mode (as JSON
> now does) or label the ratio with the CPU set it was computed over?
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/34246064212

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[Quentin Monnet]

2026-09-08 07:27 UTC-0700 ~ Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>
> From: Mykyta Yatsenko <yatsenko@meta.com>
> 
> Perf counters can report too few events when the PMU multiplexes them.
> Scale each per-CPU value before aggregation.
> 
> Use the same CPU set for derived ratios. Report cycles per included
> program run, and preserve the total run count in JSON.
> 
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>

Thank you for this!

Acked-by: Quentin Monnet <qmo@kernel.org>

Apologies for missing the review (and your question) on the previous
versions of the patchset.

Quentin

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[Andrii Nakryiko]

On Tue, Sep 8, 2026 at 7:27 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Perf counters can report too few events when the PMU multiplexes them.
> Scale each per-CPU value before aggregation.
>
> Use the same CPU set for derived ratios. Report cycles per included
> program run, and preserve the total run count in JSON.
>
> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
>  tools/bpf/bpftool/prog.c                         | 159 +++++++++++++++++------
>  2 files changed, 133 insertions(+), 39 deletions(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 90fa2a48cc26..0108a1c8be4d 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -217,7 +217,16 @@ bpftool prog run *PROG* data_in *FILE* [data_out *FILE* [data_size_out *L*]] [ct
>  bpftool prog profile *PROG* [duration *DURATION*] *METRICs*
>      Profile *METRICs* for bpf program *PROG* for *DURATION* seconds or until
>      user hits <Ctrl+C>. *DURATION* is optional. If *DURATION* is not specified,
> -    the profiling will run up to **UINT_MAX** seconds.
> +    the profiling will run up to **UINT_MAX** seconds. Plain output scales each
> +    per-CPU metric value to correct for perf event multiplexing. When
> +    **cycles** is selected, it also reports cycles per included program run. If
> +    a selected metric was not scheduled on a CPU, all metric values exclude
> +    that CPU so that ratios use a consistent CPU set. The **run_cnt** value
> +    still includes all recorded runs.
> +
> +    In JSON output, **value** is raw, **value_scaled** is scaled, **run_cnt**
> +    includes all runs, and **run_cnt_valid** includes only runs used for metric
> +    values.
>
>  bpftool prog help
>      Print short help message.
> @@ -360,7 +369,7 @@ EXAMPLES
>  ::
>
>           51397 run_cnt
> -      40176203 cycles                                                 (83.05%)
> +      40176203 cycles          # 781.68 cycles per run                (83.05%)
>        42518139 instructions    #   1.06 insns per cycle               (83.39%)
>             123 llc_misses      #   2.89 LLC misses per million insns  (83.15%)
>
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index 8c2f9255b36d..9e126f3823ad 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -2062,37 +2062,52 @@ static int do_profile(int argc, char **argv)
>
>  #include "profiler.skel.h"
>
> +enum ratio_metric {
> +       METRIC_NONE = -2,
> +       METRIC_RUN_CNT = -1,
> +       METRIC_CYCLES = 0,
> +       METRIC_INSTRUCTIONS = 1,
> +       METRIC_L1D_LOADS = 2,
> +       METRIC_LLC_MISSES = 3,
> +       METRIC_ITLB_MISSES = 4,
> +       METRIC_DTLB_MISSES = 5,
> +};
> +
>  struct profile_metric {
>         const char *name;
>         struct bpf_perf_event_value val;
> +       __u64 scaled_val;
>         struct perf_event_attr attr;
>         bool selected;
>
>         /* calculate ratios like instructions per cycle */
> -       const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
> +       const enum ratio_metric ratio_metric;
>         const char *ratio_desc;
>         const float ratio_mul;
>  } metrics[] = {
> -       {
> +       [METRIC_CYCLES] = {
>                 .name = "cycles",
>                 .attr = {
>                         .type = PERF_TYPE_HARDWARE,
>                         .config = PERF_COUNT_HW_CPU_CYCLES,
>                         .exclude_user = 1,
>                 },
> +               .ratio_metric = METRIC_RUN_CNT,
> +               .ratio_desc = "cycles per run",
> +               .ratio_mul = 1.0,
>         },
> -       {
> +       [METRIC_INSTRUCTIONS] = {
>                 .name = "instructions",
>                 .attr = {
>                         .type = PERF_TYPE_HARDWARE,
>                         .config = PERF_COUNT_HW_INSTRUCTIONS,
>                         .exclude_user = 1,
>                 },
> -               .ratio_metric = 1,
> +               .ratio_metric = METRIC_CYCLES,
>                 .ratio_desc = "insns per cycle",
>                 .ratio_mul = 1.0,
>         },
> -       {
> +       [METRIC_L1D_LOADS] = {
>                 .name = "l1d_loads",
>                 .attr = {
>                         .type = PERF_TYPE_HW_CACHE,
> @@ -2102,8 +2117,9 @@ struct profile_metric {
>                                 (PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
>                         .exclude_user = 1,
>                 },
> +               .ratio_metric = METRIC_NONE,
>         },
> -       {
> +       [METRIC_LLC_MISSES] = {
>                 .name = "llc_misses",
>                 .attr = {
>                         .type = PERF_TYPE_HW_CACHE,
> @@ -2113,11 +2129,11 @@ struct profile_metric {
>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
>                         .exclude_user = 1
>                 },
> -               .ratio_metric = 2,
> +               .ratio_metric = METRIC_INSTRUCTIONS,
>                 .ratio_desc = "LLC misses per million insns",
>                 .ratio_mul = 1e6,
>         },
> -       {
> +       [METRIC_ITLB_MISSES] = {
>                 .name = "itlb_misses",
>                 .attr = {
>                         .type = PERF_TYPE_HW_CACHE,
> @@ -2127,11 +2143,11 @@ struct profile_metric {
>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
>                         .exclude_user = 1
>                 },
> -               .ratio_metric = 2,
> +               .ratio_metric = METRIC_INSTRUCTIONS,
>                 .ratio_desc = "itlb misses per million insns",
>                 .ratio_mul = 1e6,
>         },
> -       {
> +       [METRIC_DTLB_MISSES] = {
>                 .name = "dtlb_misses",
>                 .attr = {
>                         .type = PERF_TYPE_HW_CACHE,
> @@ -2141,13 +2157,14 @@ struct profile_metric {
>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
>                         .exclude_user = 1
>                 },
> -               .ratio_metric = 2,
> +               .ratio_metric = METRIC_INSTRUCTIONS,
>                 .ratio_desc = "dtlb misses per million insns",
>                 .ratio_mul = 1e6,
>         },
>  };
>
>  static __u64 profile_total_count;
> +static __u64 profile_valid_count;
>
>  #define MAX_NUM_PROFILE_METRICS 4
>
> @@ -2182,9 +2199,39 @@ static int profile_parse_metrics(int argc, char **argv)
>         return selected_cnt;
>  }
>
> -static void profile_read_values(struct profiler_bpf *obj)
> +/*
> + * Filter out CPUs that have any selected metric not scheduled for them. This makes sure all
> + * metrics are using the same CPU set, as a result ratio metrics are consistent.
> + */
> +static void profile_filter_cpus(__u32 num_cpu,
> +                               struct bpf_perf_event_value vals[MAX_NUM_PROFILE_METRICS][num_cpu],
> +                               __u64 *counts)
> +{
> +       __u32 m, cpu, key = 0;
> +
> +       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> +               if (!metrics[m].selected)
> +                       continue;
> +
> +               for (cpu = 0; cpu < num_cpu; cpu++) {
> +                       /*
> +                        * CPU has hits, but this metric never scheduled, set counts[cpu] to 0
> +                        * so other metrics ignore this CPU too
> +                        */
> +                       if (counts[cpu] && !vals[key][cpu].running) {
> +                               p_info("%s not scheduled on CPU %u; excluding %llu runs from all metrics",
> +                                      metrics[m].name, cpu, counts[cpu]);
> +                               counts[cpu] = 0;
> +                       }
> +               }
> +               key++;
> +       }
> +}

was this suggested by AI or we actually ran into issues due to this in
practice? this looks perculiar

> +
> +static int profile_read_values(struct profiler_bpf *obj)
>  {
>         __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> +       struct bpf_perf_event_value values[MAX_NUM_PROFILE_METRICS][num_cpu], *val;
>         int reading_map_fd, count_map_fd;
>         __u64 counts[num_cpu];
>         __u32 key = 0;
> @@ -2194,38 +2241,61 @@ static void profile_read_values(struct profiler_bpf *obj)
>         count_map_fd = bpf_map__fd(obj->maps.counts);
>         if (reading_map_fd < 0 || count_map_fd < 0) {

this can't happen if skeleton loaded successfully, just remove this
check instead of weird min() over fds/errors

>                 p_err("failed to get fd for map");
> -               return;
> +               return min(reading_map_fd, count_map_fd);
>         }
>
>         err = bpf_map_lookup_elem(count_map_fd, &key, counts);
>         if (err) {
>                 p_err("failed to read count_map: %s", strerror(errno));
> -               return;
> +               return err;
> +       }
> +
> +       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> +               if (!metrics[m].selected)
> +                       continue;
> +
> +               err = bpf_map_lookup_elem(reading_map_fd, &key, values[key]);
> +               if (err) {
> +                       p_err("failed to read reading_map: %s", strerror(errno));
> +                       return err;
> +               }
> +               key++;
>         }
>
>         profile_total_count = 0;
>         for (cpu = 0; cpu < num_cpu; cpu++)
>                 profile_total_count += counts[cpu];
>
> +       profile_filter_cpus(num_cpu, values, counts);
> +
> +       profile_valid_count = 0;
> +       for (cpu = 0; cpu < num_cpu; cpu++)
> +               profile_valid_count += counts[cpu];
> +
> +       key = 0;
>         for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> -               struct bpf_perf_event_value values[num_cpu];
> +               double scale;
>
>                 if (!metrics[m].selected)
>                         continue;
>
> -               err = bpf_map_lookup_elem(reading_map_fd, &key, values);
> -               if (err) {
> -                       p_err("failed to read reading_map: %s",
> -                             strerror(errno));
> -                       return;
> -               }
>                 for (cpu = 0; cpu < num_cpu; cpu++) {
> -                       metrics[m].val.counter += values[cpu].counter;
> -                       metrics[m].val.enabled += values[cpu].enabled;
> -                       metrics[m].val.running += values[cpu].running;
> +                       /* Skip CPUs with no runs or with an unscheduled metric. */
> +                       if (!counts[cpu])
> +                               continue;
> +
> +                       val = &values[key][cpu];
> +
> +                       metrics[m].val.enabled += val->enabled;
> +                       metrics[m].val.running += val->running;
> +                       metrics[m].val.counter += val->counter;
> +                       /* Scale counter values to account for perf event multiplexing. */
> +                       scale = (double)val->enabled / val->running;
> +                       metrics[m].scaled_val += val->counter * scale;
>                 }
>                 key++;
>         }
> +       return 0;
>  }
>
>  static void profile_print_readings_json(void)
> @@ -2239,9 +2309,11 @@ static void profile_print_readings_json(void)
>                 jsonw_start_object(json_wtr);
>                 jsonw_string_field(json_wtr, "metric", metrics[m].name);
>                 jsonw_lluint_field(json_wtr, "run_cnt", profile_total_count);
> +               jsonw_lluint_field(json_wtr, "run_cnt_valid", profile_valid_count);

aren't we just overcomplicating things for no good reason?..

>                 jsonw_lluint_field(json_wtr, "value", metrics[m].val.counter);
>                 jsonw_lluint_field(json_wtr, "enabled", metrics[m].val.enabled);
>                 jsonw_lluint_field(json_wtr, "running", metrics[m].val.running);
> +               jsonw_lluint_field(json_wtr, "value_scaled", metrics[m].scaled_val);
>
>                 jsonw_end_object(json_wtr);
>         }
> @@ -2250,24 +2322,34 @@ static void profile_print_readings_json(void)
>
>  static void profile_print_readings_plain(void)
>  {
> -       __u32 m;
> +       __u32 i;
>
>         printf("\n%18llu %-20s\n", profile_total_count, "run_cnt");
> -       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> -               struct bpf_perf_event_value *val = &metrics[m].val;
> +       for (i = 0; i < ARRAY_SIZE(metrics); i++) {
> +               struct profile_metric *m = &metrics[i];
> +               struct bpf_perf_event_value *val = &m->val;
>                 int r;
> +               __u64 ratio;
>
> -               if (!metrics[m].selected)
> +               if (!m->selected)
>                         continue;
> -               printf("%18llu %-20s", val->counter, metrics[m].name);
> +               printf("%18llu %-20s", m->scaled_val, m->name);
>
> -               r = metrics[m].ratio_metric - 1;
> -               if (r >= 0 && metrics[r].selected &&
> -                   metrics[r].val.counter > 0) {
> +               r = m->ratio_metric;
> +               switch (r) {
> +               case METRIC_RUN_CNT:
> +                       ratio = profile_valid_count;
> +                       break;
> +               case METRIC_NONE:
> +                       ratio = 0;
> +                       break;
> +               default:
> +                       ratio = metrics[r].scaled_val;
> +               }
> +               if (ratio) {
>                         printf("# %8.2f %-30s",
> -                              val->counter * metrics[m].ratio_mul /
> -                              metrics[r].val.counter,
> -                              metrics[m].ratio_desc);
> +                              m->scaled_val * m->ratio_mul / ratio,
> +                              m->ratio_desc);
>                 } else {
>                         printf("%-41s", "");
>                 }
> @@ -2423,9 +2505,12 @@ static int profile_open_perf_events(struct profiler_bpf *obj)
>
>  static void profile_print_and_cleanup(void)
>  {
> +       int err;
> +
>         profile_close_perf_events(profile_obj);
> -       profile_read_values(profile_obj);
> -       profile_print_readings();
> +       err = profile_read_values(profile_obj);
> +       if (!err)
> +               profile_print_readings();
>         profiler_bpf__destroy(profile_obj);
>
>         close(profile_tgt_fd);
>
> --
> 2.53.0-Meta
>

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[Mykyta Yatsenko]

On 9/9/26 1:02 AM, Andrii Nakryiko wrote:
> On Tue, Sep 8, 2026 at 7:27 AM Mykyta Yatsenko
> <mykyta.yatsenko5@gmail.com> wrote:
>>
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> Perf counters can report too few events when the PMU multiplexes them.
>> Scale each per-CPU value before aggregation.
>>
>> Use the same CPU set for derived ratios. Report cycles per included
>> program run, and preserve the total run count in JSON.
>>
>> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
>>  tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
>>  tools/bpf/bpftool/prog.c                         | 159 +++++++++++++++++------
>>  2 files changed, 133 insertions(+), 39 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>> index 90fa2a48cc26..0108a1c8be4d 100644
>> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>> @@ -217,7 +217,16 @@ bpftool prog run *PROG* data_in *FILE* [data_out *FILE* [data_size_out *L*]] [ct
>>  bpftool prog profile *PROG* [duration *DURATION*] *METRICs*
>>      Profile *METRICs* for bpf program *PROG* for *DURATION* seconds or until
>>      user hits <Ctrl+C>. *DURATION* is optional. If *DURATION* is not specified,
>> -    the profiling will run up to **UINT_MAX** seconds.
>> +    the profiling will run up to **UINT_MAX** seconds. Plain output scales each
>> +    per-CPU metric value to correct for perf event multiplexing. When
>> +    **cycles** is selected, it also reports cycles per included program run. If
>> +    a selected metric was not scheduled on a CPU, all metric values exclude
>> +    that CPU so that ratios use a consistent CPU set. The **run_cnt** value
>> +    still includes all recorded runs.
>> +
>> +    In JSON output, **value** is raw, **value_scaled** is scaled, **run_cnt**
>> +    includes all runs, and **run_cnt_valid** includes only runs used for metric
>> +    values.
>>
>>  bpftool prog help
>>      Print short help message.
>> @@ -360,7 +369,7 @@ EXAMPLES
>>  ::
>>
>>           51397 run_cnt
>> -      40176203 cycles                                                 (83.05%)
>> +      40176203 cycles          # 781.68 cycles per run                (83.05%)
>>        42518139 instructions    #   1.06 insns per cycle               (83.39%)
>>             123 llc_misses      #   2.89 LLC misses per million insns  (83.15%)
>>
>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> index 8c2f9255b36d..9e126f3823ad 100644
>> --- a/tools/bpf/bpftool/prog.c
>> +++ b/tools/bpf/bpftool/prog.c
>> @@ -2062,37 +2062,52 @@ static int do_profile(int argc, char **argv)
>>
>>  #include "profiler.skel.h"
>>
>> +enum ratio_metric {
>> +       METRIC_NONE = -2,
>> +       METRIC_RUN_CNT = -1,
>> +       METRIC_CYCLES = 0,
>> +       METRIC_INSTRUCTIONS = 1,
>> +       METRIC_L1D_LOADS = 2,
>> +       METRIC_LLC_MISSES = 3,
>> +       METRIC_ITLB_MISSES = 4,
>> +       METRIC_DTLB_MISSES = 5,
>> +};
>> +
>>  struct profile_metric {
>>         const char *name;
>>         struct bpf_perf_event_value val;
>> +       __u64 scaled_val;
>>         struct perf_event_attr attr;
>>         bool selected;
>>
>>         /* calculate ratios like instructions per cycle */
>> -       const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
>> +       const enum ratio_metric ratio_metric;
>>         const char *ratio_desc;
>>         const float ratio_mul;
>>  } metrics[] = {
>> -       {
>> +       [METRIC_CYCLES] = {
>>                 .name = "cycles",
>>                 .attr = {
>>                         .type = PERF_TYPE_HARDWARE,
>>                         .config = PERF_COUNT_HW_CPU_CYCLES,
>>                         .exclude_user = 1,
>>                 },
>> +               .ratio_metric = METRIC_RUN_CNT,
>> +               .ratio_desc = "cycles per run",
>> +               .ratio_mul = 1.0,
>>         },
>> -       {
>> +       [METRIC_INSTRUCTIONS] = {
>>                 .name = "instructions",
>>                 .attr = {
>>                         .type = PERF_TYPE_HARDWARE,
>>                         .config = PERF_COUNT_HW_INSTRUCTIONS,
>>                         .exclude_user = 1,
>>                 },
>> -               .ratio_metric = 1,
>> +               .ratio_metric = METRIC_CYCLES,
>>                 .ratio_desc = "insns per cycle",
>>                 .ratio_mul = 1.0,
>>         },
>> -       {
>> +       [METRIC_L1D_LOADS] = {
>>                 .name = "l1d_loads",
>>                 .attr = {
>>                         .type = PERF_TYPE_HW_CACHE,
>> @@ -2102,8 +2117,9 @@ struct profile_metric {
>>                                 (PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
>>                         .exclude_user = 1,
>>                 },
>> +               .ratio_metric = METRIC_NONE,
>>         },
>> -       {
>> +       [METRIC_LLC_MISSES] = {
>>                 .name = "llc_misses",
>>                 .attr = {
>>                         .type = PERF_TYPE_HW_CACHE,
>> @@ -2113,11 +2129,11 @@ struct profile_metric {
>>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
>>                         .exclude_user = 1
>>                 },
>> -               .ratio_metric = 2,
>> +               .ratio_metric = METRIC_INSTRUCTIONS,
>>                 .ratio_desc = "LLC misses per million insns",
>>                 .ratio_mul = 1e6,
>>         },
>> -       {
>> +       [METRIC_ITLB_MISSES] = {
>>                 .name = "itlb_misses",
>>                 .attr = {
>>                         .type = PERF_TYPE_HW_CACHE,
>> @@ -2127,11 +2143,11 @@ struct profile_metric {
>>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
>>                         .exclude_user = 1
>>                 },
>> -               .ratio_metric = 2,
>> +               .ratio_metric = METRIC_INSTRUCTIONS,
>>                 .ratio_desc = "itlb misses per million insns",
>>                 .ratio_mul = 1e6,
>>         },
>> -       {
>> +       [METRIC_DTLB_MISSES] = {
>>                 .name = "dtlb_misses",
>>                 .attr = {
>>                         .type = PERF_TYPE_HW_CACHE,
>> @@ -2141,13 +2157,14 @@ struct profile_metric {
>>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
>>                         .exclude_user = 1
>>                 },
>> -               .ratio_metric = 2,
>> +               .ratio_metric = METRIC_INSTRUCTIONS,
>>                 .ratio_desc = "dtlb misses per million insns",
>>                 .ratio_mul = 1e6,
>>         },
>>  };
>>
>>  static __u64 profile_total_count;
>> +static __u64 profile_valid_count;
>>
>>  #define MAX_NUM_PROFILE_METRICS 4
>>
>> @@ -2182,9 +2199,39 @@ static int profile_parse_metrics(int argc, char **argv)
>>         return selected_cnt;
>>  }
>>
>> -static void profile_read_values(struct profiler_bpf *obj)
>> +/*
>> + * Filter out CPUs that have any selected metric not scheduled for them. This makes sure all
>> + * metrics are using the same CPU set, as a result ratio metrics are consistent.
>> + */
>> +static void profile_filter_cpus(__u32 num_cpu,
>> +                               struct bpf_perf_event_value vals[MAX_NUM_PROFILE_METRICS][num_cpu],
>> +                               __u64 *counts)
>> +{
>> +       __u32 m, cpu, key = 0;
>> +
>> +       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> +               if (!metrics[m].selected)
>> +                       continue;
>> +
>> +               for (cpu = 0; cpu < num_cpu; cpu++) {
>> +                       /*
>> +                        * CPU has hits, but this metric never scheduled, set counts[cpu] to 0
>> +                        * so other metrics ignore this CPU too
>> +                        */
>> +                       if (counts[cpu] && !vals[key][cpu].running) {
>> +                               p_info("%s not scheduled on CPU %u; excluding %llu runs from all metrics",
>> +                                      metrics[m].name, cpu, counts[cpu]);
>> +                               counts[cpu] = 0;
>> +                       }
>> +               }
>> +               key++;
>> +       }
>> +}
> 
> was this suggested by AI or we actually ran into issues due to this in
> practice? this looks perculiar
> 

Suggested by AI is a bit of a simplification:
The initial problem was scaling counters, the counters are collected 
and accounted per CPU, so it sounds like a right thing to scale per
CPU as well(?) Because we divide by running, we need to check it for 0.
Now it is possible that some counters have running 0 on some CPU, but
others dont, will it make ratios biased?

this is easy to repro:
in tab 1 run:
```
sudo ./build/tools/bpf/bpftool/bpftool prog profile name myprog duration 80 itlb_misses dtlb_misses cycles instructions
```
in tab 2 run:
```
sudo ./build/tools/bpf/bpftool/bpftool prog profile name myprog duration 1 itlb_misses dtlb_misses cycles instructions
cycles not scheduled on CPU 36; excluding 1 runs from all metrics
instructions not scheduled on CPU 6; excluding 4 runs from all metrics
instructions not scheduled on CPU 39; excluding 1 runs from all metrics
instructions not scheduled on CPU 72; excluding 1 runs from all metrics
itlb_misses not scheduled on CPU 50; excluding 1 runs from all metrics

               330 run_cnt
           3601259 cycles              # 11184.03 cycles per run                (73.57%)
            682448 instructions        #     0.19 insns per cycle               (80.85%)
               132 itlb_misses         #   193.42 itlb misses per million insns (73.66%)
              2189 dtlb_misses         #  3207.57 dtlb misses per million insns (72.24%)
```

>> +
>> +static int profile_read_values(struct profiler_bpf *obj)
>>  {
>>         __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>> +       struct bpf_perf_event_value values[MAX_NUM_PROFILE_METRICS][num_cpu], *val;
>>         int reading_map_fd, count_map_fd;
>>         __u64 counts[num_cpu];
>>         __u32 key = 0;
>> @@ -2194,38 +2241,61 @@ static void profile_read_values(struct profiler_bpf *obj)
>>         count_map_fd = bpf_map__fd(obj->maps.counts);
>>         if (reading_map_fd < 0 || count_map_fd < 0) {
> 
> this can't happen if skeleton loaded successfully, just remove this
> check instead of weird min() over fds/errors
> 
>>                 p_err("failed to get fd for map");
>> -               return;
>> +               return min(reading_map_fd, count_map_fd);
>>         }
>>
>>         err = bpf_map_lookup_elem(count_map_fd, &key, counts);
>>         if (err) {
>>                 p_err("failed to read count_map: %s", strerror(errno));
>> -               return;
>> +               return err;
>> +       }
>> +
>> +       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> +               if (!metrics[m].selected)
>> +                       continue;
>> +
>> +               err = bpf_map_lookup_elem(reading_map_fd, &key, values[key]);
>> +               if (err) {
>> +                       p_err("failed to read reading_map: %s", strerror(errno));
>> +                       return err;
>> +               }
>> +               key++;
>>         }
>>
>>         profile_total_count = 0;
>>         for (cpu = 0; cpu < num_cpu; cpu++)
>>                 profile_total_count += counts[cpu];
>>
>> +       profile_filter_cpus(num_cpu, values, counts);
>> +
>> +       profile_valid_count = 0;
>> +       for (cpu = 0; cpu < num_cpu; cpu++)
>> +               profile_valid_count += counts[cpu];
>> +
>> +       key = 0;
>>         for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> -               struct bpf_perf_event_value values[num_cpu];
>> +               double scale;
>>
>>                 if (!metrics[m].selected)
>>                         continue;
>>
>> -               err = bpf_map_lookup_elem(reading_map_fd, &key, values);
>> -               if (err) {
>> -                       p_err("failed to read reading_map: %s",
>> -                             strerror(errno));
>> -                       return;
>> -               }
>>                 for (cpu = 0; cpu < num_cpu; cpu++) {
>> -                       metrics[m].val.counter += values[cpu].counter;
>> -                       metrics[m].val.enabled += values[cpu].enabled;
>> -                       metrics[m].val.running += values[cpu].running;
>> +                       /* Skip CPUs with no runs or with an unscheduled metric. */
>> +                       if (!counts[cpu])
>> +                               continue;
>> +
>> +                       val = &values[key][cpu];
>> +
>> +                       metrics[m].val.enabled += val->enabled;
>> +                       metrics[m].val.running += val->running;
>> +                       metrics[m].val.counter += val->counter;
>> +                       /* Scale counter values to account for perf event multiplexing. */
>> +                       scale = (double)val->enabled / val->running;
>> +                       metrics[m].scaled_val += val->counter * scale;
>>                 }
>>                 key++;
>>         }
>> +       return 0;
>>  }
>>
>>  static void profile_print_readings_json(void)
>> @@ -2239,9 +2309,11 @@ static void profile_print_readings_json(void)
>>                 jsonw_start_object(json_wtr);
>>                 jsonw_string_field(json_wtr, "metric", metrics[m].name);
>>                 jsonw_lluint_field(json_wtr, "run_cnt", profile_total_count);
>> +               jsonw_lluint_field(json_wtr, "run_cnt_valid", profile_valid_count);
> 
> aren't we just overcomplicating things for no good reason?..
> 
>>                 jsonw_lluint_field(json_wtr, "value", metrics[m].val.counter);
>>                 jsonw_lluint_field(json_wtr, "enabled", metrics[m].val.enabled);
>>                 jsonw_lluint_field(json_wtr, "running", metrics[m].val.running);
>> +               jsonw_lluint_field(json_wtr, "value_scaled", metrics[m].scaled_val);
>>
>>                 jsonw_end_object(json_wtr);
>>         }
>> @@ -2250,24 +2322,34 @@ static void profile_print_readings_json(void)
>>
>>  static void profile_print_readings_plain(void)
>>  {
>> -       __u32 m;
>> +       __u32 i;
>>
>>         printf("\n%18llu %-20s\n", profile_total_count, "run_cnt");
>> -       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> -               struct bpf_perf_event_value *val = &metrics[m].val;
>> +       for (i = 0; i < ARRAY_SIZE(metrics); i++) {
>> +               struct profile_metric *m = &metrics[i];
>> +               struct bpf_perf_event_value *val = &m->val;
>>                 int r;
>> +               __u64 ratio;
>>
>> -               if (!metrics[m].selected)
>> +               if (!m->selected)
>>                         continue;
>> -               printf("%18llu %-20s", val->counter, metrics[m].name);
>> +               printf("%18llu %-20s", m->scaled_val, m->name);
>>
>> -               r = metrics[m].ratio_metric - 1;
>> -               if (r >= 0 && metrics[r].selected &&
>> -                   metrics[r].val.counter > 0) {
>> +               r = m->ratio_metric;
>> +               switch (r) {
>> +               case METRIC_RUN_CNT:
>> +                       ratio = profile_valid_count;
>> +                       break;
>> +               case METRIC_NONE:
>> +                       ratio = 0;
>> +                       break;
>> +               default:
>> +                       ratio = metrics[r].scaled_val;
>> +               }
>> +               if (ratio) {
>>                         printf("# %8.2f %-30s",
>> -                              val->counter * metrics[m].ratio_mul /
>> -                              metrics[r].val.counter,
>> -                              metrics[m].ratio_desc);
>> +                              m->scaled_val * m->ratio_mul / ratio,
>> +                              m->ratio_desc);
>>                 } else {
>>                         printf("%-41s", "");
>>                 }
>> @@ -2423,9 +2505,12 @@ static int profile_open_perf_events(struct profiler_bpf *obj)
>>
>>  static void profile_print_and_cleanup(void)
>>  {
>> +       int err;
>> +
>>         profile_close_perf_events(profile_obj);
>> -       profile_read_values(profile_obj);
>> -       profile_print_readings();
>> +       err = profile_read_values(profile_obj);
>> +       if (!err)
>> +               profile_print_readings();
>>         profiler_bpf__destroy(profile_obj);
>>
>>         close(profile_tgt_fd);
>>
>> --
>> 2.53.0-Meta
>>

Re: [PATCH bpf-next v3 3/3] bpftool: Scale counters and report cycles per run

[Andrii Nakryiko]

On Wed, Sep 9, 2026 at 2:49 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> On 9/9/26 1:02 AM, Andrii Nakryiko wrote:
> > On Tue, Sep 8, 2026 at 7:27 AM Mykyta Yatsenko
> > <mykyta.yatsenko5@gmail.com> wrote:
> >>
> >> From: Mykyta Yatsenko <yatsenko@meta.com>
> >>
> >> Perf counters can report too few events when the PMU multiplexes them.
> >> Scale each per-CPU value before aggregation.
> >>
> >> Use the same CPU set for derived ratios. Report cycles per included
> >> program run, and preserve the total run count in JSON.
> >>
> >> Fixes: 47c09d6a9f67 ("bpftool: Introduce "prog profile" command")
> >> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> >> ---
> >>  tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
> >>  tools/bpf/bpftool/prog.c                         | 159 +++++++++++++++++------
> >>  2 files changed, 133 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> >> index 90fa2a48cc26..0108a1c8be4d 100644
> >> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> >> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> >> @@ -217,7 +217,16 @@ bpftool prog run *PROG* data_in *FILE* [data_out *FILE* [data_size_out *L*]] [ct
> >>  bpftool prog profile *PROG* [duration *DURATION*] *METRICs*
> >>      Profile *METRICs* for bpf program *PROG* for *DURATION* seconds or until
> >>      user hits <Ctrl+C>. *DURATION* is optional. If *DURATION* is not specified,
> >> -    the profiling will run up to **UINT_MAX** seconds.
> >> +    the profiling will run up to **UINT_MAX** seconds. Plain output scales each
> >> +    per-CPU metric value to correct for perf event multiplexing. When
> >> +    **cycles** is selected, it also reports cycles per included program run. If
> >> +    a selected metric was not scheduled on a CPU, all metric values exclude
> >> +    that CPU so that ratios use a consistent CPU set. The **run_cnt** value
> >> +    still includes all recorded runs.
> >> +
> >> +    In JSON output, **value** is raw, **value_scaled** is scaled, **run_cnt**
> >> +    includes all runs, and **run_cnt_valid** includes only runs used for metric
> >> +    values.
> >>
> >>  bpftool prog help
> >>      Print short help message.
> >> @@ -360,7 +369,7 @@ EXAMPLES
> >>  ::
> >>
> >>           51397 run_cnt
> >> -      40176203 cycles                                                 (83.05%)
> >> +      40176203 cycles          # 781.68 cycles per run                (83.05%)
> >>        42518139 instructions    #   1.06 insns per cycle               (83.39%)
> >>             123 llc_misses      #   2.89 LLC misses per million insns  (83.15%)
> >>
> >> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> >> index 8c2f9255b36d..9e126f3823ad 100644
> >> --- a/tools/bpf/bpftool/prog.c
> >> +++ b/tools/bpf/bpftool/prog.c
> >> @@ -2062,37 +2062,52 @@ static int do_profile(int argc, char **argv)
> >>
> >>  #include "profiler.skel.h"
> >>
> >> +enum ratio_metric {
> >> +       METRIC_NONE = -2,
> >> +       METRIC_RUN_CNT = -1,
> >> +       METRIC_CYCLES = 0,
> >> +       METRIC_INSTRUCTIONS = 1,
> >> +       METRIC_L1D_LOADS = 2,
> >> +       METRIC_LLC_MISSES = 3,
> >> +       METRIC_ITLB_MISSES = 4,
> >> +       METRIC_DTLB_MISSES = 5,
> >> +};
> >> +
> >>  struct profile_metric {
> >>         const char *name;
> >>         struct bpf_perf_event_value val;
> >> +       __u64 scaled_val;
> >>         struct perf_event_attr attr;
> >>         bool selected;
> >>
> >>         /* calculate ratios like instructions per cycle */
> >> -       const int ratio_metric; /* 0 for N/A, 1 for index 0 (cycles) */
> >> +       const enum ratio_metric ratio_metric;
> >>         const char *ratio_desc;
> >>         const float ratio_mul;
> >>  } metrics[] = {
> >> -       {
> >> +       [METRIC_CYCLES] = {
> >>                 .name = "cycles",
> >>                 .attr = {
> >>                         .type = PERF_TYPE_HARDWARE,
> >>                         .config = PERF_COUNT_HW_CPU_CYCLES,
> >>                         .exclude_user = 1,
> >>                 },
> >> +               .ratio_metric = METRIC_RUN_CNT,
> >> +               .ratio_desc = "cycles per run",
> >> +               .ratio_mul = 1.0,
> >>         },
> >> -       {
> >> +       [METRIC_INSTRUCTIONS] = {
> >>                 .name = "instructions",
> >>                 .attr = {
> >>                         .type = PERF_TYPE_HARDWARE,
> >>                         .config = PERF_COUNT_HW_INSTRUCTIONS,
> >>                         .exclude_user = 1,
> >>                 },
> >> -               .ratio_metric = 1,
> >> +               .ratio_metric = METRIC_CYCLES,
> >>                 .ratio_desc = "insns per cycle",
> >>                 .ratio_mul = 1.0,
> >>         },
> >> -       {
> >> +       [METRIC_L1D_LOADS] = {
> >>                 .name = "l1d_loads",
> >>                 .attr = {
> >>                         .type = PERF_TYPE_HW_CACHE,
> >> @@ -2102,8 +2117,9 @@ struct profile_metric {
> >>                                 (PERF_COUNT_HW_CACHE_RESULT_ACCESS << 16),
> >>                         .exclude_user = 1,
> >>                 },
> >> +               .ratio_metric = METRIC_NONE,
> >>         },
> >> -       {
> >> +       [METRIC_LLC_MISSES] = {
> >>                 .name = "llc_misses",
> >>                 .attr = {
> >>                         .type = PERF_TYPE_HW_CACHE,
> >> @@ -2113,11 +2129,11 @@ struct profile_metric {
> >>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
> >>                         .exclude_user = 1
> >>                 },
> >> -               .ratio_metric = 2,
> >> +               .ratio_metric = METRIC_INSTRUCTIONS,
> >>                 .ratio_desc = "LLC misses per million insns",
> >>                 .ratio_mul = 1e6,
> >>         },
> >> -       {
> >> +       [METRIC_ITLB_MISSES] = {
> >>                 .name = "itlb_misses",
> >>                 .attr = {
> >>                         .type = PERF_TYPE_HW_CACHE,
> >> @@ -2127,11 +2143,11 @@ struct profile_metric {
> >>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
> >>                         .exclude_user = 1
> >>                 },
> >> -               .ratio_metric = 2,
> >> +               .ratio_metric = METRIC_INSTRUCTIONS,
> >>                 .ratio_desc = "itlb misses per million insns",
> >>                 .ratio_mul = 1e6,
> >>         },
> >> -       {
> >> +       [METRIC_DTLB_MISSES] = {
> >>                 .name = "dtlb_misses",
> >>                 .attr = {
> >>                         .type = PERF_TYPE_HW_CACHE,
> >> @@ -2141,13 +2157,14 @@ struct profile_metric {
> >>                                 (PERF_COUNT_HW_CACHE_RESULT_MISS << 16),
> >>                         .exclude_user = 1
> >>                 },
> >> -               .ratio_metric = 2,
> >> +               .ratio_metric = METRIC_INSTRUCTIONS,
> >>                 .ratio_desc = "dtlb misses per million insns",
> >>                 .ratio_mul = 1e6,
> >>         },
> >>  };
> >>
> >>  static __u64 profile_total_count;
> >> +static __u64 profile_valid_count;
> >>
> >>  #define MAX_NUM_PROFILE_METRICS 4
> >>
> >> @@ -2182,9 +2199,39 @@ static int profile_parse_metrics(int argc, char **argv)
> >>         return selected_cnt;
> >>  }
> >>
> >> -static void profile_read_values(struct profiler_bpf *obj)
> >> +/*
> >> + * Filter out CPUs that have any selected metric not scheduled for them. This makes sure all
> >> + * metrics are using the same CPU set, as a result ratio metrics are consistent.
> >> + */
> >> +static void profile_filter_cpus(__u32 num_cpu,
> >> +                               struct bpf_perf_event_value vals[MAX_NUM_PROFILE_METRICS][num_cpu],
> >> +                               __u64 *counts)
> >> +{
> >> +       __u32 m, cpu, key = 0;
> >> +
> >> +       for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> >> +               if (!metrics[m].selected)
> >> +                       continue;
> >> +
> >> +               for (cpu = 0; cpu < num_cpu; cpu++) {
> >> +                       /*
> >> +                        * CPU has hits, but this metric never scheduled, set counts[cpu] to 0
> >> +                        * so other metrics ignore this CPU too
> >> +                        */
> >> +                       if (counts[cpu] && !vals[key][cpu].running) {
> >> +                               p_info("%s not scheduled on CPU %u; excluding %llu runs from all metrics",
> >> +                                      metrics[m].name, cpu, counts[cpu]);
> >> +                               counts[cpu] = 0;
> >> +                       }
> >> +               }
> >> +               key++;
> >> +       }
> >> +}
> >
> > was this suggested by AI or we actually ran into issues due to this in
> > practice? this looks perculiar
> >
>
> Suggested by AI is a bit of a simplification:
> The initial problem was scaling counters, the counters are collected
> and accounted per CPU, so it sounds like a right thing to scale per
> CPU as well(?) Because we divide by running, we need to check it for 0.
> Now it is possible that some counters have running 0 on some CPU, but
> others dont, will it make ratios biased?
>

So you are talking about IPC (insns per cycle) calculation, for
example, right? Where we need two PMUs to both be present on a given
CPU. I think a bit more proper solution for this is PMU groups, so
that they are either both scheduled or both not scheduled. In this
case seems like we'll want 4 PMUs in a group, we'd have to try and see
if that's a problem in practice, but I'd try PMU group first as a
solution for this.


[...]

Re: [PATCH bpf-next v3 0/3] bpftool: Improve perf counter reporting

[Ihor Solodrai]

On 9/8/26 7:27 AM, Mykyta Yatsenko wrote:
> Track valid snapshots that have a zero counter value. Scale each per-CPU
> counter before aggregation. Report cycles per included program run.
> 
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
> Changes in v3:
> - Keep the JSON value field raw. Add value_scaled and run_cnt_valid.
> - Use the same CPU set for all metrics.
> - Move snapshot state tracking to a separate patch.
> - Link to v2: https://patch.msgid.link/20260902-bpftool_cyles_per_run-v2-1-bc5d14ad0ce9@meta.com
> 
> Changes in v2:
> - Scale all counters.
> - Use an enum for metric indexes.
> - Link to v1: https://patch.msgid.link/20260827-bpftool_cyles_per_run-v1-1-77d7bfc3c065@meta.com
> 
> ---
> Mykyta Yatsenko (3):
>       bpftool: Track perf counter snapshot state
>       perf bpf_counter: Track valid BPF counter snapshots
>       bpftool: Scale counters and report cycles per run

For the series:

Acked-by: Ihor Solodrai <ihor.solodrai@linux.dev>

> 
>  tools/bpf/bpftool/Documentation/bpftool-prog.rst |  13 +-
>  tools/bpf/bpftool/prog.c                         | 159 +++++++++++++++++------
>  tools/bpf/bpftool/skeleton/profiler.bpf.c        |  25 ++--
>  tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c |  26 ++--
>  4 files changed, 168 insertions(+), 55 deletions(-)
> ---
> base-commit: 3fa3128887a4df6057c543aae7c5c7fd2ed41f12
> change-id: 20260827-bpftool_cyles_per_run-93169fcb30b7
> 
> Best regards,
> --  
> Mykyta Yatsenko <yatsenko@meta.com>
>