* [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters
@ 2026-09-02 13:53 Mykyta Yatsenko
2026-09-02 15:20 ` bot+bpf-ci
0 siblings, 1 reply; 5+ messages in thread
From: Mykyta Yatsenko @ 2026-09-02 13:53 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor, qmo
Cc: Mykyta Yatsenko
From: Mykyta Yatsenko <yatsenko@meta.com>
Perf counters values may be underreported when PMU multiplexing
takes place. Scale resulting values similar to how perf does it:
scaled_count = raw_count*time_enabled/time_running;
Report cycles per run to expose the per-invocation cost.
Example:
sudo ./bpftool prog profile name mprog duration 15 cycles instructions
423256 run_cnt
947413975 cycles # 2238.39 cycles per run
333965846 instructions # 0.35 insns per cycle
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
Changes in v2:
- Added scaling for all counters to address PMU multiplexing (Suchit, Andrii)
- Introduce an enum for metric indexes (Andrii)
- Link to v1: https://patch.msgid.link/20260827-bpftool_cyles_per_run-v1-1-77d7bfc3c065@meta.com
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 6 +-
tools/bpf/bpftool/prog.c | 77 ++++++++++++++++++------
2 files changed, 62 insertions(+), 21 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 90fa2a48cc26..2280dc4492c0 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -217,7 +217,9 @@ 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. When **cycles** is
+ selected, plain output also reports the average number of cycles per
+ program run.
bpftool prog help
Print short help message.
@@ -360,7 +362,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 a9f730d407a9..ad6d33f838e3 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2062,6 +2062,17 @@ 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;
@@ -2069,30 +2080,33 @@ struct profile_metric {
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 +2116,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 +2128,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 +2142,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,7 +2156,7 @@ 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,
},
@@ -2185,6 +2200,7 @@ static int profile_parse_metrics(int argc, char **argv)
static void profile_read_values(struct profiler_bpf *obj)
{
__u32 m, cpu, num_cpu = obj->rodata->num_cpu;
+ struct bpf_perf_event_value *val;
int reading_map_fd, count_map_fd;
__u64 counts[num_cpu];
__u32 key = 0;
@@ -2209,6 +2225,7 @@ static void profile_read_values(struct profiler_bpf *obj)
for (m = 0; m < ARRAY_SIZE(metrics); m++) {
struct bpf_perf_event_value values[num_cpu];
+ double scale;
if (!metrics[m].selected)
continue;
@@ -2220,9 +2237,22 @@ static void profile_read_values(struct profiler_bpf *obj)
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;
+ val = &values[cpu];
+
+ metrics[m].val.enabled += val->enabled;
+ metrics[m].val.running += val->running;
+
+ /* This counter has never been scheduled */
+ if (!val->running) {
+ if (counts[cpu])
+ p_info("%s not scheduled on CPU %u; results may be inaccurate",
+ metrics[m].name, cpu);
+ continue;
+ }
+
+ /* Scale counter values to account for perf event multiplexing. */
+ scale = (double)val->enabled / val->running;
+ metrics[m].val.counter += val->counter * scale;
}
key++;
}
@@ -2256,17 +2286,26 @@ static void profile_print_readings_plain(void)
for (m = 0; m < ARRAY_SIZE(metrics); m++) {
struct bpf_perf_event_value *val = &metrics[m].val;
int r;
+ __u64 ratio;
if (!metrics[m].selected)
continue;
printf("%18llu %-20s", val->counter, metrics[m].name);
- r = metrics[m].ratio_metric - 1;
- if (r >= 0 && metrics[r].selected &&
- metrics[r].val.counter > 0) {
+ r = metrics[m].ratio_metric;
+ switch (r) {
+ case METRIC_RUN_CNT:
+ ratio = profile_total_count;
+ break;
+ case METRIC_NONE:
+ ratio = 0;
+ break;
+ default:
+ ratio = metrics[r].val.counter;
+ }
+ if (ratio) {
printf("# %8.2f %-30s",
- val->counter * metrics[m].ratio_mul /
- metrics[r].val.counter,
+ val->counter * metrics[m].ratio_mul / ratio,
metrics[m].ratio_desc);
} else {
printf("%-41s", "");
---
base-commit: 23ff631b3b8b1452dfe933ee21f96321a9c5e209
change-id: 20260827-bpftool_cyles_per_run-93169fcb30b7
Best regards,
--
Mykyta Yatsenko <yatsenko@meta.com>
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters
2026-09-02 13:53 [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters Mykyta Yatsenko
@ 2026-09-02 15:20 ` bot+bpf-ci
2026-09-02 18:14 ` Mykyta Yatsenko
0 siblings, 1 reply; 5+ messages in thread
From: bot+bpf-ci @ 2026-09-02 15:20 UTC (permalink / raw)
To: mykyta.yatsenko5, bpf, ast, andrii, daniel, kernel-team, eddyz87,
memxor, qmo
Cc: yatsenko, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song,
mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4357 bytes --]
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 90fa2a48cc26a..2280dc4492c06 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -217,7 +217,9 @@ 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. When **cycles** is
> + selected, plain output also reports the average number of cycles per
> + program run.
[ ... ]
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index a9f730d407a92..ad6d33f838e3c 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
[ ... ]
> @@ -2185,6 +2200,7 @@ static int profile_parse_metrics(int argc, char **argv)
> static void profile_read_values(struct profiler_bpf *obj)
> {
> __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
> + struct bpf_perf_event_value *val;
> int reading_map_fd, count_map_fd;
> __u64 counts[num_cpu];
> __u32 key = 0;
> @@ -2209,6 +2225,7 @@ static void profile_read_values(struct profiler_bpf *obj)
>
> for (m = 0; m < ARRAY_SIZE(metrics); m++) {
> struct bpf_perf_event_value values[num_cpu];
> + double scale;
>
> if (!metrics[m].selected)
> continue;
> @@ -2220,9 +2237,22 @@ static void profile_read_values(struct profiler_bpf *obj)
> 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;
> + val = &values[cpu];
> +
> + metrics[m].val.enabled += val->enabled;
> + metrics[m].val.running += val->running;
> +
> + /* This counter has never been scheduled */
> + if (!val->running) {
> + if (counts[cpu])
> + p_info("%s not scheduled on CPU %u; results may be inaccurate",
> + metrics[m].name, cpu);
> + continue;
> + }
> +
> + /* Scale counter values to account for perf event multiplexing. */
> + scale = (double)val->enabled / val->running;
> + metrics[m].val.counter += val->counter * scale;
> }
> key++;
> }
Does this change the semantics of the JSON output? Looking at
profile_print_readings_json() in prog.c:
profile_print_readings_json() {
...
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);
...
}
Before this commit "value" was the raw counter sum and "enabled"/"running"
let a consumer apply the correction value * enabled / running itself.
After this commit "value" is already scaled (multiplied by enabled/running)
while "enabled" and "running" remain as raw sums. If an existing consumer
of bpftool -j prog profile applies the documented formula - value * enabled
/ running - it would now double-scale the result.
There is no new field and no version marker letting a consumer distinguish
between the two output formats. The changelog says only "Scale resulting
values similar to how perf does it" without mentioning the JSON output,
and the .rst hunk explicitly scopes its wording to plain output ("When
**cycles** is selected, plain output also reports ...").
Should the JSON "value" field stay raw to keep the interface stable, or
should the scaled/raw split be made explicit with a new "scaled_value"
field?
The same asymmetry appears in plain output. profile_print_readings_plain()
still prints the raw multiplex percentage next to a counter that has
already been corrected for that percentage:
if (val->enabled > val->running)
printf("(%4.2f%%)", val->running * 100.0 / val->enabled);
which reads as if the correction still has to be applied by the reader.
---
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/33641892894
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters
2026-09-02 15:20 ` bot+bpf-ci
@ 2026-09-02 18:14 ` Mykyta Yatsenko
2026-09-03 14:33 ` Mykyta Yatsenko
0 siblings, 1 reply; 5+ messages in thread
From: Mykyta Yatsenko @ 2026-09-02 18:14 UTC (permalink / raw)
To: bot+bpf-ci, bpf, ast, andrii, daniel, kernel-team, eddyz87,
memxor, qmo
Cc: yatsenko, martin.lau, yonghong.song, mason, ihor.solodrai
On 9/2/26 4:20 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>> index 90fa2a48cc26a..2280dc4492c06 100644
>> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>> @@ -217,7 +217,9 @@ 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. When **cycles** is
>> + selected, plain output also reports the average number of cycles per
>> + program run.
>
> [ ... ]
>
>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>> index a9f730d407a92..ad6d33f838e3c 100644
>> --- a/tools/bpf/bpftool/prog.c
>> +++ b/tools/bpf/bpftool/prog.c
>
> [ ... ]
>
>> @@ -2185,6 +2200,7 @@ static int profile_parse_metrics(int argc, char **argv)
>> static void profile_read_values(struct profiler_bpf *obj)
>> {
>> __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>> + struct bpf_perf_event_value *val;
>> int reading_map_fd, count_map_fd;
>> __u64 counts[num_cpu];
>> __u32 key = 0;
>> @@ -2209,6 +2225,7 @@ static void profile_read_values(struct profiler_bpf *obj)
>>
>> for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>> struct bpf_perf_event_value values[num_cpu];
>> + double scale;
>>
>> if (!metrics[m].selected)
>> continue;
>> @@ -2220,9 +2237,22 @@ static void profile_read_values(struct profiler_bpf *obj)
>> 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;
>> + val = &values[cpu];
>> +
>> + metrics[m].val.enabled += val->enabled;
>> + metrics[m].val.running += val->running;
>> +
>> + /* This counter has never been scheduled */
>> + if (!val->running) {
>> + if (counts[cpu])
>> + p_info("%s not scheduled on CPU %u; results may be inaccurate",
>> + metrics[m].name, cpu);
>> + continue;
>> + }
>> +
>> + /* Scale counter values to account for perf event multiplexing. */
>> + scale = (double)val->enabled / val->running;
>> + metrics[m].val.counter += val->counter * scale;
>> }
>> key++;
>> }
>
> Does this change the semantics of the JSON output? Looking at
> profile_print_readings_json() in prog.c:
>
> profile_print_readings_json() {
> ...
> 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);
> ...
> }
>
> Before this commit "value" was the raw counter sum and "enabled"/"running"
> let a consumer apply the correction value * enabled / running itself.
>
> After this commit "value" is already scaled (multiplied by enabled/running)
> while "enabled" and "running" remain as raw sums. If an existing consumer
> of bpftool -j prog profile applies the documented formula - value * enabled
> / running - it would now double-scale the result.
>
> There is no new field and no version marker letting a consumer distinguish
> between the two output formats. The changelog says only "Scale resulting
> values similar to how perf does it" without mentioning the JSON output,
> and the .rst hunk explicitly scopes its wording to plain output ("When
> **cycles** is selected, plain output also reports ...").
>
> Should the JSON "value" field stay raw to keep the interface stable, or
> should the scaled/raw split be made explicit with a new "scaled_value"
> field?
>
> The same asymmetry appears in plain output. profile_print_readings_plain()
> still prints the raw multiplex percentage next to a counter that has
> already been corrected for that percentage:
>
> if (val->enabled > val->running)
> printf("(%4.2f%%)", val->running * 100.0 / val->enabled);
>
> which reads as if the correction still has to be applied by the reader.
>
This is a good point. I don't think we should scale for plain output, but not json.
But otherwise we potentially break users.
>
> ---
> 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/33641892894
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters
2026-09-02 18:14 ` Mykyta Yatsenko
@ 2026-09-03 14:33 ` Mykyta Yatsenko
2026-09-03 16:15 ` Ihor Solodrai
0 siblings, 1 reply; 5+ messages in thread
From: Mykyta Yatsenko @ 2026-09-03 14:33 UTC (permalink / raw)
To: bot+bpf-ci, bpf, ast, andrii, daniel, kernel-team, eddyz87,
memxor, qmo
Cc: yatsenko, martin.lau, yonghong.song, mason, ihor.solodrai
On 9/2/26 7:14 PM, Mykyta Yatsenko wrote:
>
>
> On 9/2/26 4:20 PM, bot+bpf-ci@kernel.org wrote:
>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>> index 90fa2a48cc26a..2280dc4492c06 100644
>>> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>> @@ -217,7 +217,9 @@ 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. When **cycles** is
>>> + selected, plain output also reports the average number of cycles per
>>> + program run.
>>
>> [ ... ]
>>
>>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>>> index a9f730d407a92..ad6d33f838e3c 100644
>>> --- a/tools/bpf/bpftool/prog.c
>>> +++ b/tools/bpf/bpftool/prog.c
>>
>> [ ... ]
>>
>>> @@ -2185,6 +2200,7 @@ static int profile_parse_metrics(int argc, char **argv)
>>> static void profile_read_values(struct profiler_bpf *obj)
>>> {
>>> __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>> + struct bpf_perf_event_value *val;
>>> int reading_map_fd, count_map_fd;
>>> __u64 counts[num_cpu];
>>> __u32 key = 0;
>>> @@ -2209,6 +2225,7 @@ static void profile_read_values(struct profiler_bpf *obj)
>>>
>>> for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>>> struct bpf_perf_event_value values[num_cpu];
>>> + double scale;
>>>
>>> if (!metrics[m].selected)
>>> continue;
>>> @@ -2220,9 +2237,22 @@ static void profile_read_values(struct profiler_bpf *obj)
>>> 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;
>>> + val = &values[cpu];
>>> +
>>> + metrics[m].val.enabled += val->enabled;
>>> + metrics[m].val.running += val->running;
>>> +
>>> + /* This counter has never been scheduled */
>>> + if (!val->running) {
>>> + if (counts[cpu])
>>> + p_info("%s not scheduled on CPU %u; results may be inaccurate",
>>> + metrics[m].name, cpu);
>>> + continue;
>>> + }
>>> +
>>> + /* Scale counter values to account for perf event multiplexing. */
>>> + scale = (double)val->enabled / val->running;
>>> + metrics[m].val.counter += val->counter * scale;
>>> }
>>> key++;
>>> }
>>
>> Does this change the semantics of the JSON output? Looking at
>> profile_print_readings_json() in prog.c:
>>
>> profile_print_readings_json() {
>> ...
>> 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);
>> ...
>> }
>>
>> Before this commit "value" was the raw counter sum and "enabled"/"running"
>> let a consumer apply the correction value * enabled / running itself.
>>
>> After this commit "value" is already scaled (multiplied by enabled/running)
>> while "enabled" and "running" remain as raw sums. If an existing consumer
>> of bpftool -j prog profile applies the documented formula - value * enabled
>> / running - it would now double-scale the result.
>>
>> There is no new field and no version marker letting a consumer distinguish
>> between the two output formats. The changelog says only "Scale resulting
>> values similar to how perf does it" without mentioning the JSON output,
>> and the .rst hunk explicitly scopes its wording to plain output ("When
>> **cycles** is selected, plain output also reports ...").
>>
>> Should the JSON "value" field stay raw to keep the interface stable, or
>> should the scaled/raw split be made explicit with a new "scaled_value"
>> field?
>>
>> The same asymmetry appears in plain output. profile_print_readings_plain()
>> still prints the raw multiplex percentage next to a counter that has
>> already been corrected for that percentage:
>>
>> if (val->enabled > val->running)
>> printf("(%4.2f%%)", val->running * 100.0 / val->enabled);
>>
>> which reads as if the correction still has to be applied by the reader.
>>
>
> This is a good point. I don't think we should scale for plain output, but not json.
> But otherwise we potentially break users.
>>
Discussed with Andrii, we'll scale for stdout and keep existing behaviour for the json.
>> ---
>> 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/33641892894
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters
2026-09-03 14:33 ` Mykyta Yatsenko
@ 2026-09-03 16:15 ` Ihor Solodrai
0 siblings, 0 replies; 5+ messages in thread
From: Ihor Solodrai @ 2026-09-03 16:15 UTC (permalink / raw)
To: Mykyta Yatsenko, bot+bpf-ci, bpf, ast, andrii, daniel,
kernel-team, eddyz87, memxor, qmo
Cc: yatsenko, martin.lau, yonghong.song, mason
On 2026-09-03 7:33 a.m., Mykyta Yatsenko wrote:
>
>
> On 9/2/26 7:14 PM, Mykyta Yatsenko wrote:
>>
>>
>> On 9/2/26 4:20 PM, bot+bpf-ci@kernel.org wrote:
>>>> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>>> index 90fa2a48cc26a..2280dc4492c06 100644
>>>> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>>> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
>>>> @@ -217,7 +217,9 @@ 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. When **cycles** is
>>>> + selected, plain output also reports the average number of cycles per
>>>> + program run.
>>>
>>> [ ... ]
>>>
>>>> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
>>>> index a9f730d407a92..ad6d33f838e3c 100644
>>>> --- a/tools/bpf/bpftool/prog.c
>>>> +++ b/tools/bpf/bpftool/prog.c
>>>
>>> [ ... ]
>>>
>>>> @@ -2185,6 +2200,7 @@ static int profile_parse_metrics(int argc, char **argv)
>>>> static void profile_read_values(struct profiler_bpf *obj)
>>>> {
>>>> __u32 m, cpu, num_cpu = obj->rodata->num_cpu;
>>>> + struct bpf_perf_event_value *val;
>>>> int reading_map_fd, count_map_fd;
>>>> __u64 counts[num_cpu];
>>>> __u32 key = 0;
>>>> @@ -2209,6 +2225,7 @@ static void profile_read_values(struct profiler_bpf *obj)
>>>>
>>>> for (m = 0; m < ARRAY_SIZE(metrics); m++) {
>>>> struct bpf_perf_event_value values[num_cpu];
>>>> + double scale;
>>>>
>>>> if (!metrics[m].selected)
>>>> continue;
>>>> @@ -2220,9 +2237,22 @@ static void profile_read_values(struct profiler_bpf *obj)
>>>> 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;
>>>> + val = &values[cpu];
>>>> +
>>>> + metrics[m].val.enabled += val->enabled;
>>>> + metrics[m].val.running += val->running;
>>>> +
>>>> + /* This counter has never been scheduled */
>>>> + if (!val->running) {
>>>> + if (counts[cpu])
>>>> + p_info("%s not scheduled on CPU %u; results may be inaccurate",
>>>> + metrics[m].name, cpu);
>>>> + continue;
>>>> + }
>>>> +
>>>> + /* Scale counter values to account for perf event multiplexing. */
>>>> + scale = (double)val->enabled / val->running;
>>>> + metrics[m].val.counter += val->counter * scale;
>>>> }
>>>> key++;
>>>> }
>>>
>>> Does this change the semantics of the JSON output? Looking at
>>> profile_print_readings_json() in prog.c:
>>>
>>> profile_print_readings_json() {
>>> ...
>>> 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);
>>> ...
>>> }
>>>
>>> Before this commit "value" was the raw counter sum and "enabled"/"running"
>>> let a consumer apply the correction value * enabled / running itself.
>>>
>>> After this commit "value" is already scaled (multiplied by enabled/running)
>>> while "enabled" and "running" remain as raw sums. If an existing consumer
>>> of bpftool -j prog profile applies the documented formula - value * enabled
>>> / running - it would now double-scale the result.
>>>
>>> There is no new field and no version marker letting a consumer distinguish
>>> between the two output formats. The changelog says only "Scale resulting
>>> values similar to how perf does it" without mentioning the JSON output,
>>> and the .rst hunk explicitly scopes its wording to plain output ("When
>>> **cycles** is selected, plain output also reports ...").
>>>
>>> Should the JSON "value" field stay raw to keep the interface stable, or
>>> should the scaled/raw split be made explicit with a new "scaled_value"
>>> field?
>>>
>>> The same asymmetry appears in plain output. profile_print_readings_plain()
>>> still prints the raw multiplex percentage next to a counter that has
>>> already been corrected for that percentage:
>>>
>>> if (val->enabled > val->running)
>>> printf("(%4.2f%%)", val->running * 100.0 / val->enabled);
>>>
>>> which reads as if the correction still has to be applied by the reader.
>>>
>>
>> This is a good point. I don't think we should scale for plain output, but not json.
>> But otherwise we potentially break users.
>>>
>
> Discussed with Andrii, we'll scale for stdout and keep existing behaviour for the json.
Hi Mykyta.
I think for json we can just add new fields, for example "value_scaled".
This way existing consumers are unaffected, while new/updated consumers
have a way to opt-in.
This seems to be an established pattern for json output changes, see:
74fc8801edc2 ("bpftool: Show map IDs along with struct_ops links.")
bf06c9393493 ("bpftool: Dump map id instead of value for map_of_maps
types")
>
>>> ---
>>> 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/33641892894
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-03 16:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02 13:53 [PATCH bpf-next v2] bpftool: Scale multiplexed perf counters Mykyta Yatsenko
2026-09-02 15:20 ` bot+bpf-ci
2026-09-02 18:14 ` Mykyta Yatsenko
2026-09-03 14:33 ` Mykyta Yatsenko
2026-09-03 16:15 ` Ihor Solodrai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox