Commit ce09cbdd authored by Jose Fernandez's avatar Jose Fernandez Committed by Daniel Borkmann

bpf: Improve program stats run-time calculation

This patch improves the run-time calculation for program stats by
capturing the duration as soon as possible after the program returns.

Previously, the duration included u64_stats_t operations. While the
instrumentation overhead is part of the total time spent when stats are
enabled, distinguishing between the program's native execution time and
the time spent due to instrumentation is crucial for accurate
performance analysis.

By making this change, the patch facilitates more precise optimization
of BPF programs, enabling users to understand their performance in
environments without stats enabled.

I used a virtualized environment to measure the run-time over one minute
for a basic raw_tracepoint/sys_enter program, which just increments a
local counter. Although the virtualization introduced some performance
degradation that could affect the results, I observed approximately a
16% decrease in average run-time reported by stats with this change
(310 -> 260 nsec).
Signed-off-by: default avatarJose Fernandez <josef@netflix.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20240402034010.25060-1-josef@netflix.com
parent c186ed12
...@@ -654,14 +654,16 @@ static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog, ...@@ -654,14 +654,16 @@ static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog,
cant_migrate(); cant_migrate();
if (static_branch_unlikely(&bpf_stats_enabled_key)) { if (static_branch_unlikely(&bpf_stats_enabled_key)) {
struct bpf_prog_stats *stats; struct bpf_prog_stats *stats;
u64 start = sched_clock(); u64 duration, start = sched_clock();
unsigned long flags; unsigned long flags;
ret = dfunc(ctx, prog->insnsi, prog->bpf_func); ret = dfunc(ctx, prog->insnsi, prog->bpf_func);
duration = sched_clock() - start;
stats = this_cpu_ptr(prog->stats); stats = this_cpu_ptr(prog->stats);
flags = u64_stats_update_begin_irqsave(&stats->syncp); flags = u64_stats_update_begin_irqsave(&stats->syncp);
u64_stats_inc(&stats->cnt); u64_stats_inc(&stats->cnt);
u64_stats_add(&stats->nsecs, sched_clock() - start); u64_stats_add(&stats->nsecs, duration);
u64_stats_update_end_irqrestore(&stats->syncp, flags); u64_stats_update_end_irqrestore(&stats->syncp, flags);
} else { } else {
ret = dfunc(ctx, prog->insnsi, prog->bpf_func); ret = dfunc(ctx, prog->insnsi, prog->bpf_func);
......
...@@ -885,12 +885,13 @@ static void notrace update_prog_stats(struct bpf_prog *prog, ...@@ -885,12 +885,13 @@ static void notrace update_prog_stats(struct bpf_prog *prog,
* Hence check that 'start' is valid. * Hence check that 'start' is valid.
*/ */
start > NO_START_TIME) { start > NO_START_TIME) {
u64 duration = sched_clock() - start;
unsigned long flags; unsigned long flags;
stats = this_cpu_ptr(prog->stats); stats = this_cpu_ptr(prog->stats);
flags = u64_stats_update_begin_irqsave(&stats->syncp); flags = u64_stats_update_begin_irqsave(&stats->syncp);
u64_stats_inc(&stats->cnt); u64_stats_inc(&stats->cnt);
u64_stats_add(&stats->nsecs, sched_clock() - start); u64_stats_add(&stats->nsecs, duration);
u64_stats_update_end_irqrestore(&stats->syncp, flags); u64_stats_update_end_irqrestore(&stats->syncp, flags);
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment