* [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting
@ 2026-09-16 19:26 Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots Mykyta Yatsenko
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-09-16 19:26 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko
Accept valid perf snapshots whose counter is zero. Group selected metrics
per CPU so derived ratios cover the same scheduling intervals. Scale each
per-CPU counter before aggregation and report cycles per program run.
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
Changes in v4:
- Use enabled time to recognize valid zero-counter snapshots.
- Build and enable each CPU's complete perf event group in one pass.
- Keep run_cnt as the total count, drop run_cnt_valid.
- Link to v3: https://patch.msgid.link/20260908-bpftool_cyles_per_run-v3-0-60e86f325c35@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: Accept zero perf counter snapshots
bpftool: Group profile events by CPU
bpftool: Scale counters and report cycles per run
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 11 +-
tools/bpf/bpftool/prog.c | 209 ++++++++++++++---------
tools/bpf/bpftool/skeleton/profiler.bpf.c | 21 +--
3 files changed, 148 insertions(+), 93 deletions(-)
---
base-commit: 847b984213626bee356d550dc1a8df12e683e50f
change-id: 20260827-bpftool_cyles_per_run-93169fcb30b7
Best regards,
--
Mykyta Yatsenko <yatsenko@meta.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots
2026-09-16 19:26 [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting Mykyta Yatsenko
@ 2026-09-16 19:26 ` Mykyta Yatsenko
2026-09-16 20:07 ` bot+bpf-ci
2026-09-16 19:26 ` [PATCH bpf-next v4 2/3] bpftool: Group profile events by CPU Mykyta Yatsenko
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-09-16 19:26 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko
From: Mykyta Yatsenko <yatsenko@meta.com>
A perf counter can be zero at fentry when PMU multiplexing has
not scheduled the event. Do not reject that sample.
Use enabled time to distinguish a successful snapshot. Read
directly into the per-CPU map value.
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
tools/bpf/bpftool/skeleton/profiler.bpf.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
index f48c783cb9f7..6685e7252fb5 100644
--- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
+++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
@@ -45,28 +45,21 @@ const volatile __u32 num_metric = 1;
SEC("fentry/XXX")
int BPF_PROG(fentry_XXX)
{
- struct bpf_perf_event_value___local *ptrs[MAX_NUM_METRICS];
u32 key = bpf_get_smp_processor_id();
u32 i;
- /* look up before reading, to reduce error */
for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
+ struct bpf_perf_event_value___local *reading;
u32 flag = i;
-
- ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
- if (!ptrs[i])
- return 0;
- }
-
- for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
- struct bpf_perf_event_value___local reading;
int err;
- err = bpf_perf_event_read_value(&events, key, (void *)&reading,
- sizeof(reading));
+ reading = bpf_map_lookup_elem(&fentry_readings, &flag);
+ if (!reading)
+ return 0;
+ err = bpf_perf_event_read_value(&events, key, (void *)reading,
+ sizeof(*reading));
if (err)
return 0;
- *(ptrs[i]) = reading;
key += num_cpu;
}
@@ -80,7 +73,7 @@ fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
before = bpf_map_lookup_elem(&fentry_readings, &id);
/* only account samples with a valid fentry_reading */
- if (before && before->counter) {
+ if (before && before->enabled) {
struct bpf_perf_event_value___local *accum;
diff.counter = after->counter - before->counter;
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next v4 2/3] bpftool: Group profile events by CPU
2026-09-16 19:26 [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots Mykyta Yatsenko
@ 2026-09-16 19:26 ` Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 3/3] bpftool: Scale counters and report cycles per run Mykyta Yatsenko
2026-09-17 22:30 ` [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-09-16 19:26 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko
From: Mykyta Yatsenko <yatsenko@meta.com>
Independently scheduled perf events can cover different intervals,
making derived ratios inconsistent.
Open one event group per CPU and enable it only after all selected
metrics have joined.
Fail the profile setup when a selected event cannot join its group.
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
tools/bpf/bpftool/prog.c | 87 +++++++++++++++++++++++++++++-------------------
1 file changed, 52 insertions(+), 35 deletions(-)
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 6ab911e82155..a14a1a9601bd 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -2350,7 +2350,7 @@ static char *profile_tgt_name;
static int *profile_perf_events;
static int profile_perf_event_cnt;
-static void profile_close_perf_events(struct profiler_bpf *obj)
+static void profile_close_perf_events(void)
{
int i;
@@ -2361,61 +2361,78 @@ static void profile_close_perf_events(struct profiler_bpf *obj)
profile_perf_event_cnt = 0;
}
-static int profile_open_perf_event(int mid, int cpu, int map_fd)
+static int profile_open_perf_event(int mid, int cpu,
+ int map_fd, int group_fd, __u32 map_key)
{
+ struct perf_event_attr attr = metrics[mid].attr;
+ bool group_leader = group_fd < 0;
int pmu_fd;
+ int err;
- pmu_fd = syscall(__NR_perf_event_open, &metrics[mid].attr,
- -1 /*pid*/, cpu, -1 /*group_fd*/, 0);
+ attr.disabled = group_leader;
+ pmu_fd = syscall(__NR_perf_event_open, &attr, -1 /* pid */, cpu,
+ group_fd, 0);
if (pmu_fd < 0) {
- if (errno == ENODEV) {
- p_info("cpu %d may be offline, skip %s profiling.",
- cpu, metrics[mid].name);
- profile_perf_event_cnt++;
- return 0;
- }
- return -1;
+ err = -errno;
+ if (errno == ENODEV && group_leader)
+ p_info("cpu %d may be offline, skip profiling.", cpu);
+ return err;
}
- if (bpf_map_update_elem(map_fd,
- &profile_perf_event_cnt,
- &pmu_fd, BPF_ANY) ||
- ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0)) {
+ if (bpf_map_update_elem(map_fd, &map_key, &pmu_fd, BPF_ANY)) {
+ err = -errno;
close(pmu_fd);
- return -1;
+ return err;
}
profile_perf_events[profile_perf_event_cnt++] = pmu_fd;
- return 0;
+ return pmu_fd;
}
static int profile_open_perf_events(struct profiler_bpf *obj)
{
+ __u32 num_cpu = obj->rodata->num_cpu;
+ __u32 map_key;
unsigned int cpu, m;
- int map_fd;
+ int group_fd, map_fd, pmu_fd;
+ int err;
- profile_perf_events = calloc(
- obj->rodata->num_cpu * obj->rodata->num_metric, sizeof(int));
+ profile_perf_events = calloc(num_cpu * obj->rodata->num_metric,
+ sizeof(*profile_perf_events));
if (!profile_perf_events) {
p_err("failed to allocate memory for perf_event array: %s",
strerror(errno));
- return -1;
+ return -ENOMEM;
}
+
map_fd = bpf_map__fd(obj->maps.events);
- if (map_fd < 0) {
- p_err("failed to get fd for events map");
- return -1;
- }
+ for (cpu = 0; cpu < num_cpu; cpu++) {
+ group_fd = -1;
+ map_key = cpu;
+ for (m = 0; m < ARRAY_SIZE(metrics); m++) {
+ if (!metrics[m].selected)
+ continue;
- for (m = 0; m < ARRAY_SIZE(metrics); m++) {
- if (!metrics[m].selected)
- continue;
- for (cpu = 0; cpu < obj->rodata->num_cpu; cpu++) {
- if (profile_open_perf_event(m, cpu, map_fd)) {
- p_err("failed to create event %s on cpu %u",
- metrics[m].name, cpu);
- return -1;
+ pmu_fd = profile_open_perf_event(m, cpu, map_fd, group_fd,
+ map_key);
+ if (pmu_fd == -ENODEV && group_fd < 0)
+ break;
+ if (pmu_fd < 0) {
+ p_err("failed to add event %s to group on CPU %u: %s",
+ metrics[m].name, cpu, strerror(-pmu_fd));
+ return pmu_fd;
}
+ if (group_fd < 0)
+ group_fd = pmu_fd;
+ map_key += num_cpu;
+ }
+ if (group_fd < 0)
+ continue;
+ if (ioctl(group_fd, PERF_EVENT_IOC_ENABLE, PERF_IOC_FLAG_GROUP)) {
+ err = -errno;
+ p_err("failed to enable perf event group on CPU %u: %s",
+ cpu, strerror(-err));
+ return err;
}
}
return 0;
@@ -2423,7 +2440,7 @@ static int profile_open_perf_events(struct profiler_bpf *obj)
static void profile_print_and_cleanup(void)
{
- profile_close_perf_events(profile_obj);
+ profile_close_perf_events();
profile_read_values(profile_obj);
profile_print_readings();
profiler_bpf__destroy(profile_obj);
@@ -2529,7 +2546,7 @@ static int do_profile(int argc, char **argv)
return 0;
out:
- profile_close_perf_events(profile_obj);
+ profile_close_perf_events();
if (profile_obj)
profiler_bpf__destroy(profile_obj);
close(profile_tgt_fd);
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next v4 3/3] bpftool: Scale counters and report cycles per run
2026-09-16 19:26 [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 2/3] bpftool: Group profile events by CPU Mykyta Yatsenko
@ 2026-09-16 19:26 ` Mykyta Yatsenko
2026-09-17 22:30 ` [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Mykyta Yatsenko @ 2026-09-16 19:26 UTC (permalink / raw)
To: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor; +Cc: Mykyta Yatsenko
From: Mykyta Yatsenko <yatsenko@meta.com>
Perf counters undercount when the PMU multiplexes them. Scale each
per-CPU value before aggregation. Per-CPU event groups make PMU ratios
use matching scheduling intervals.
Keep run_cnt as the total number of program executions, independent of
counter scheduling.
Keep JSON value raw, report the scaled value in value_scaled, and
document the output.
Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
tools/bpf/bpftool/Documentation/bpftool-prog.rst | 11 +-
tools/bpf/bpftool/prog.c | 122 +++++++++++++++--------
2 files changed, 89 insertions(+), 44 deletions(-)
diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 90fa2a48cc26..90fe8c61bf42 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -217,7 +217,14 @@ 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. Selected metrics form a
+ perf event group on each CPU so that ratios use counters scheduled over the
+ same intervals. Plain output scales each per-CPU metric value to correct
+ for perf event multiplexing. When **cycles** is selected, it also reports
+ cycles per program run.
+
+ In JSON output, **value** is raw, **value_scaled** is scaled, and
+ **run_cnt** is the total number of program runs.
bpftool prog help
Print short help message.
@@ -360,7 +367,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 a14a1a9601bd..000774835b10 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,7 +2157,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,
},
@@ -2182,7 +2198,7 @@ static int profile_parse_metrics(int argc, char **argv)
return selected_cnt;
}
-static void profile_read_values(struct profiler_bpf *obj)
+static int profile_read_values(struct profiler_bpf *obj)
{
__u32 m, cpu, num_cpu = obj->rodata->num_cpu;
int reading_map_fd, count_map_fd;
@@ -2192,15 +2208,11 @@ static void profile_read_values(struct profiler_bpf *obj)
reading_map_fd = bpf_map__fd(obj->maps.accum_readings);
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;
- }
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;
}
profile_total_count = 0;
@@ -2208,24 +2220,37 @@ static void profile_read_values(struct profiler_bpf *obj)
profile_total_count += counts[cpu];
for (m = 0; m < ARRAY_SIZE(metrics); m++) {
- struct bpf_perf_event_value values[num_cpu];
+ struct bpf_perf_event_value values[num_cpu], *val;
+ 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;
+ p_err("failed to read reading_map: %s", strerror(errno));
+ return err;
}
+
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];
+ if (counts[cpu] && !val->running) {
+ p_err("perf event %s was not counted on CPU %u",
+ metrics[m].name, cpu);
+ return -EAGAIN;
+ }
+ metrics[m].val.enabled += val->enabled;
+ metrics[m].val.running += val->running;
+ metrics[m].val.counter += val->counter;
+ if (val->running) {
+ /* Scale 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)
@@ -2242,6 +2267,7 @@ static void profile_print_readings_json(void)
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 +2276,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_total_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", "");
}
@@ -2438,21 +2474,24 @@ static int profile_open_perf_events(struct profiler_bpf *obj)
return 0;
}
-static void profile_print_and_cleanup(void)
+static int profile_print_and_cleanup(void)
{
+ int err;
+
profile_close_perf_events();
- 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);
free(profile_tgt_name);
+ return err;
}
static void int_exit(int signo)
{
- profile_print_and_cleanup();
- exit(0);
+ exit(!!profile_print_and_cleanup());
}
static int do_profile(int argc, char **argv)
@@ -2542,8 +2581,7 @@ static int do_profile(int argc, char **argv)
signal(SIGINT, int_exit);
sleep(duration);
- profile_print_and_cleanup();
- return 0;
+ return profile_print_and_cleanup();
out:
profile_close_perf_events();
--
2.53.0-Meta
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots
2026-09-16 19:26 ` [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots Mykyta Yatsenko
@ 2026-09-16 20:07 ` bot+bpf-ci
0 siblings, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-09-16 20:07 UTC (permalink / raw)
To: mykyta.yatsenko5, bpf, ast, andrii, daniel, kernel-team, eddyz87,
memxor
Cc: yatsenko, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song,
mason, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 4645 bytes --]
> commit e6ed198f75f0f993908fe2507feab24a456f9c16
> Author: Mykyta Yatsenko <yatsenko@meta.com>
> Date: Wed Sep 16 12:26:40 2026 -0700
>
> bpftool: Accept zero perf counter snapshots
>
> A perf counter can be zero at fentry when PMU multiplexing has
> not scheduled the event. Do not reject that sample.
>
> Use enabled time to distinguish a successful snapshot. Read
> directly into the per-CPU map value.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
This looks like a bug fix for the prog profile command. Should this include
a Fixes: tag pointing to the commit that introduced the original validation
logic?
Fixes: 47c09d6a9f67 ("bpftool: Introduce \"prog profile\" command")
> diff --git a/tools/bpf/bpftool/skeleton/profiler.bpf.c b/tools/bpf/bpftool/skeleton/profiler.bpf.c
> index f48c783cb9f7..6685e7252fb5 100644
> --- a/tools/bpf/bpftool/skeleton/profiler.bpf.c
> +++ b/tools/bpf/bpftool/skeleton/profiler.bpf.c
> @@ -45,28 +45,21 @@ const volatile __u32 num_metric = 1;
> SEC("fentry/XXX")
> int BPF_PROG(fentry_XXX)
> {
> - struct bpf_perf_event_value___local *ptrs[MAX_NUM_METRICS];
> u32 key = bpf_get_smp_processor_id();
> u32 i;
>
> - /* look up before reading, to reduce error */
> for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
> + struct bpf_perf_event_value___local *reading;
> u32 flag = i;
> -
> - ptrs[i] = bpf_map_lookup_elem(&fentry_readings, &flag);
> - if (!ptrs[i])
> - return 0;
> - }
> -
> - for (i = 0; i < num_metric && i < MAX_NUM_METRICS; i++) {
> - struct bpf_perf_event_value___local reading;
> int err;
>
> - err = bpf_perf_event_read_value(&events, key, (void *)&reading,
> - sizeof(reading));
> + reading = bpf_map_lookup_elem(&fentry_readings, &flag);
> + if (!reading)
> + return 0;
> + err = bpf_perf_event_read_value(&events, key, (void *)reading,
> + sizeof(*reading));
> if (err)
> return 0;
> - *(ptrs[i]) = reading;
> key += num_cpu;
> }
Does merging these loops introduce measurement error? The original code
performed all map lookups before any perf event reads, so the
bpf_perf_event_read_value() calls that start each metric's measurement
window ran back to back with no map operations between them.
After this change, each iteration inserts a bpf_map_lookup_elem() call
between successive bpf_perf_event_read_value() calls. The counter for
metric i is computed as after_i - before_i, where before_i is now captured
later than it was. That extra delay from the interleaved lookups gets
counted as part of the profiled program in fexit_update_maps():
tools/bpf/bpftool/skeleton/profiler.bpf.c:fexit_update_maps() {
diff.counter = after->counter - before->counter;
...
accum->counter += diff.counter;
}
This produces a systematic inflation of reported cycles for all but the last
metric. The magnitude is small because percpu-array lookups are inlined, but
the commit message only mentions reading directly into the map value, not
removing the error-reduction ordering.
A comment later in the file still documents this principle:
/* read all events before updating the maps, to reduce error */
Should the pre-lookup pass be kept to preserve measurement accuracy, or
should the commit message and the remaining comment be updated to reflect
the new trade-off?
> @@ -80,7 +73,7 @@ fexit_update_maps(u32 id, struct bpf_perf_event_value___local *after)
>
> before = bpf_map_lookup_elem(&fentry_readings, &id);
> /* only account samples with a valid fentry_reading */
> - if (before && before->counter) {
> + if (before && before->enabled) {
> struct bpf_perf_event_value___local *accum;
>
> diff.counter = after->counter - before->counter;
The switch from checking counter to checking enabled looks correct for this
file. However, there's a second copy of this profiler in the tree at
tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c that still uses the old
test:
tools/perf/util/bpf_skel/bpf_prog_profiler.bpf.c:fexit_XXX() {
...
if (before && before->counter) {
diff.counter = after.counter - before->counter;
...
}
}
That function is a near-verbatim copy of fexit_update_maps() and would drop
samples whenever the fentry counter snapshot is zero, which is the exact bug
described in this commit message.
Should perf's copy be fixed in this series, or is there a reason to leave it
unchanged?
---
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/35141110446
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting
2026-09-16 19:26 [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting Mykyta Yatsenko
` (2 preceding siblings ...)
2026-09-16 19:26 ` [PATCH bpf-next v4 3/3] bpftool: Scale counters and report cycles per run Mykyta Yatsenko
@ 2026-09-17 22:30 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-17 22:30 UTC (permalink / raw)
To: Mykyta Yatsenko
Cc: bpf, ast, andrii, daniel, kernel-team, eddyz87, memxor, yatsenko
Hello:
This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:
On Wed, 16 Sep 2026 12:26:39 -0700 you wrote:
> Accept valid perf snapshots whose counter is zero. Group selected metrics
> per CPU so derived ratios cover the same scheduling intervals. Scale each
> per-CPU counter before aggregation and report cycles per program run.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
> Changes in v4:
> - Use enabled time to recognize valid zero-counter snapshots.
> - Build and enable each CPU's complete perf event group in one pass.
> - Keep run_cnt as the total count, drop run_cnt_valid.
> - Link to v3: https://patch.msgid.link/20260908-bpftool_cyles_per_run-v3-0-60e86f325c35@meta.com
>
> [...]
Here is the summary with links:
- [bpf-next,v4,1/3] bpftool: Accept zero perf counter snapshots
https://git.kernel.org/bpf/bpf-next/c/b2cf39f53478
- [bpf-next,v4,2/3] bpftool: Group profile events by CPU
https://git.kernel.org/bpf/bpf-next/c/6b5fe4b6d5ae
- [bpf-next,v4,3/3] bpftool: Scale counters and report cycles per run
https://git.kernel.org/bpf/bpf-next/c/c2c8bd265627
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-17 22:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-16 19:26 [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 1/3] bpftool: Accept zero perf counter snapshots Mykyta Yatsenko
2026-09-16 20:07 ` bot+bpf-ci
2026-09-16 19:26 ` [PATCH bpf-next v4 2/3] bpftool: Group profile events by CPU Mykyta Yatsenko
2026-09-16 19:26 ` [PATCH bpf-next v4 3/3] bpftool: Scale counters and report cycles per run Mykyta Yatsenko
2026-09-17 22:30 ` [PATCH bpf-next v4 0/3] bpftool: Improve perf counter reporting patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox