| From 2ada9c8840d7ae65185512d1a24973df7181a1a0 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 1 Apr 2024 21:40:10 -0600 |
| Subject: bpf: Improve program stats run-time calculation |
| |
| From: Jose Fernandez <josef@netflix.com> |
| |
| [ Upstream commit ce09cbdd988887662546a1175bcfdfc6c8fdd150 ] |
| |
| 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: Jose Fernandez <josef@netflix.com> |
| Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> |
| Acked-by: Daniel Borkmann <daniel@iogearbox.net> |
| Link: https://lore.kernel.org/bpf/20240402034010.25060-1-josef@netflix.com |
| Stable-dep-of: 7dc211c1159d ("bpf: Fix invalid prog->stats access when update_effective_progs fails") |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| include/linux/filter.h | 6 ++++-- |
| kernel/bpf/trampoline.c | 3 ++- |
| 2 files changed, 6 insertions(+), 3 deletions(-) |
| |
| diff --git a/include/linux/filter.h b/include/linux/filter.h |
| index ad5a3d68b5552..f9fb83be935d4 100644 |
| --- a/include/linux/filter.h |
| +++ b/include/linux/filter.h |
| @@ -599,14 +599,16 @@ static __always_inline u32 __bpf_prog_run(const struct bpf_prog *prog, |
| cant_migrate(); |
| if (static_branch_unlikely(&bpf_stats_enabled_key)) { |
| struct bpf_prog_stats *stats; |
| - u64 start = sched_clock(); |
| + u64 duration, start = sched_clock(); |
| unsigned long flags; |
| |
| ret = dfunc(ctx, prog->insnsi, prog->bpf_func); |
| + |
| + duration = sched_clock() - start; |
| stats = this_cpu_ptr(prog->stats); |
| flags = u64_stats_update_begin_irqsave(&stats->syncp); |
| 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); |
| } else { |
| ret = dfunc(ctx, prog->insnsi, prog->bpf_func); |
| diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c |
| index f8a58b4130701..e48791442acc5 100644 |
| --- a/kernel/bpf/trampoline.c |
| +++ b/kernel/bpf/trampoline.c |
| @@ -870,12 +870,13 @@ static void notrace update_prog_stats(struct bpf_prog *prog, |
| * Hence check that 'start' is valid. |
| */ |
| start > NO_START_TIME) { |
| + u64 duration = sched_clock() - start; |
| unsigned long flags; |
| |
| stats = this_cpu_ptr(prog->stats); |
| flags = u64_stats_update_begin_irqsave(&stats->syncp); |
| 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); |
| } |
| } |
| -- |
| 2.51.0 |
| |