diff mbox series

[bpf-next,03/12] bpf: Count stats for kprobe_multi programs

Message ID 20230828075537.194192-4-jolsa@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series bpf: Add missed stats for kprobes | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on s390x with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1346 this patch: 1346
netdev/cc_maintainers warning 7 maintainers not CCed: linux-trace-kernel@vger.kernel.org kpsingh@kernel.org martin.lau@linux.dev mhiramat@kernel.org song@kernel.org yonghong.song@linux.dev rostedt@goodmis.org
netdev/build_clang success Errors and warnings before: 1353 this patch: 1353
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1369 this patch: 1369
netdev/checkpatch warning CHECK: Please don't use multiple blank lines
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Jiri Olsa Aug. 28, 2023, 7:55 a.m. UTC 
Adding support to gather stats for kprobe_multi programs.

We now count:
  - missed stats due to bpf_prog_active protection (always)
  - cnt/nsec of the bpf program execution (if kernel.bpf_stats_enabled=1)

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/bpf_trace.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Hou Tao Sept. 4, 2023, 1:30 p.m. UTC | #1 
On 8/28/2023 3:55 PM, Jiri Olsa wrote:
> Adding support to gather stats for kprobe_multi programs.
>
> We now count:
>   - missed stats due to bpf_prog_active protection (always)
>   - cnt/nsec of the bpf program execution (if kernel.bpf_stats_enabled=1)
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

Acked-by: Hou Tao <houtao1@huawei.com>

With one nit below.

> ---
>  kernel/trace/bpf_trace.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index a7264b2c17ad..0a8685fc1eee 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2706,18 +2706,24 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
>  		.link = link,
>  		.entry_ip = entry_ip,
>  	};
> +	struct bpf_prog *prog = link->link.prog;
>  	struct bpf_run_ctx *old_run_ctx;
> +	u64 start;
>  	int err;
>  
>  	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> +		bpf_prog_inc_misses_counter(prog);
>  		err = 0;
>  		goto out;
>  	}
>  
> +

The extra empty line is not needed here.
Hou Tao Sept. 5, 2023, 6:15 a.m. UTC | #2 
Hi,

On 8/28/2023 3:55 PM, Jiri Olsa wrote:
> Adding support to gather stats for kprobe_multi programs.
>
> We now count:
>   - missed stats due to bpf_prog_active protection (always)
>   - cnt/nsec of the bpf program execution (if kernel.bpf_stats_enabled=1)
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  kernel/trace/bpf_trace.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index a7264b2c17ad..0a8685fc1eee 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -2706,18 +2706,24 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
>  		.link = link,
>  		.entry_ip = entry_ip,
>  	};
> +	struct bpf_prog *prog = link->link.prog;
>  	struct bpf_run_ctx *old_run_ctx;
> +	u64 start;
>  	int err;
>  
>  	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> +		bpf_prog_inc_misses_counter(prog);
>  		err = 0;
>  		goto out;
>  	}
>  
> +
>  	migrate_disable();
>  	rcu_read_lock();
>  	old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> -	err = bpf_prog_run(link->link.prog, regs);
> +	start = bpf_prog_start_time();
> +	err = bpf_prog_run(prog, regs);
> +	bpf_prog_update_prog_stats(prog, start);

Oops, I missed the bpf_prog_run() here. It seems that bpf_prog_run() has
already done the accounting thing, so there is no need for double
accounting.
>  	bpf_reset_run_ctx(old_run_ctx);
>  	rcu_read_unlock();
>  	migrate_enable();
Jiri Olsa Sept. 5, 2023, 7:19 a.m. UTC | #3 
On Tue, Sep 05, 2023 at 02:15:49PM +0800, Hou Tao wrote:
> Hi,
> 
> On 8/28/2023 3:55 PM, Jiri Olsa wrote:
> > Adding support to gather stats for kprobe_multi programs.
> >
> > We now count:
> >   - missed stats due to bpf_prog_active protection (always)
> >   - cnt/nsec of the bpf program execution (if kernel.bpf_stats_enabled=1)
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  kernel/trace/bpf_trace.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index a7264b2c17ad..0a8685fc1eee 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -2706,18 +2706,24 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
> >  		.link = link,
> >  		.entry_ip = entry_ip,
> >  	};
> > +	struct bpf_prog *prog = link->link.prog;
> >  	struct bpf_run_ctx *old_run_ctx;
> > +	u64 start;
> >  	int err;
> >  
> >  	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
> > +		bpf_prog_inc_misses_counter(prog);
> >  		err = 0;
> >  		goto out;
> >  	}
> >  
> > +
> >  	migrate_disable();
> >  	rcu_read_lock();
> >  	old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
> > -	err = bpf_prog_run(link->link.prog, regs);
> > +	start = bpf_prog_start_time();
> > +	err = bpf_prog_run(prog, regs);
> > +	bpf_prog_update_prog_stats(prog, start);
> 
> Oops, I missed the bpf_prog_run() here. It seems that bpf_prog_run() has
> already done the accounting thing, so there is no need for double
> accounting.

right, same as the other change, thanks

jirka

> >  	bpf_reset_run_ctx(old_run_ctx);
> >  	rcu_read_unlock();
> >  	migrate_enable();
>
diff mbox series

Patch

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index a7264b2c17ad..0a8685fc1eee 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2706,18 +2706,24 @@  kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 		.link = link,
 		.entry_ip = entry_ip,
 	};
+	struct bpf_prog *prog = link->link.prog;
 	struct bpf_run_ctx *old_run_ctx;
+	u64 start;
 	int err;
 
 	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
+		bpf_prog_inc_misses_counter(prog);
 		err = 0;
 		goto out;
 	}
 
+
 	migrate_disable();
 	rcu_read_lock();
 	old_run_ctx = bpf_set_run_ctx(&run_ctx.run_ctx);
-	err = bpf_prog_run(link->link.prog, regs);
+	start = bpf_prog_start_time();
+	err = bpf_prog_run(prog, regs);
+	bpf_prog_update_prog_stats(prog, start);
 	bpf_reset_run_ctx(old_run_ctx);
 	rcu_read_unlock();
 	migrate_enable();