On Sat, Nov 15, 2025 at 4:46 AM Andrii Nakryiko <[email protected]> wrote: > > On Mon, Nov 10, 2025 at 5:49 AM Menglong Dong <[email protected]> > wrote: > > > > Remove the "trigger_count" in trigger_bench.c and reuse trigger_driver() > > instead for trigger_kernel_count_setup(). > > > > With the calling to bpf_get_numa_node_id(), the result for "kernel_count" > > will become a little more accurate. > > "more accurate" is a bit misleading here. I think you meant that it > will do same amount of helper calls as fentry and other benchmarks, > and in that sense will be closer as a baseline comparison, is that > right? Can you clarify that in the next revision, please?
Yeah, this is what I mean. The call to "bpf_get_numa_node_id" should be considered as the baseline comparison. > > > > > It will also easier if we want to test the performance of livepatch, just > > hook the bpf_get_numa_node_id() and run the "kernel_count" bench trigger. > > > > Signed-off-by: Menglong Dong <[email protected]> > > --- > > .../selftests/bpf/benchs/bench_trigger.c | 5 +---- > > .../testing/selftests/bpf/progs/trigger_bench.c | 17 +++++------------ > > 2 files changed, 6 insertions(+), 16 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/benchs/bench_trigger.c > > b/tools/testing/selftests/bpf/benchs/bench_trigger.c > > index 1e2aff007c2a..34fd8fa3b803 100644 > > --- a/tools/testing/selftests/bpf/benchs/bench_trigger.c > > +++ b/tools/testing/selftests/bpf/benchs/bench_trigger.c > > @@ -179,11 +179,8 @@ static void trigger_syscall_count_setup(void) > > static void trigger_kernel_count_setup(void) > > { > > setup_ctx(); > > - bpf_program__set_autoload(ctx.skel->progs.trigger_driver, false); > > - bpf_program__set_autoload(ctx.skel->progs.trigger_count, true); > > + ctx.skel->rodata->kernel_count = 1; > > load_ctx(); > > - /* override driver program */ > > - ctx.driver_prog_fd = bpf_program__fd(ctx.skel->progs.trigger_count); > > } > > > > static void trigger_kprobe_setup(void) > > diff --git a/tools/testing/selftests/bpf/progs/trigger_bench.c > > b/tools/testing/selftests/bpf/progs/trigger_bench.c > > index 3d5f30c29ae3..6564d1909c7b 100644 > > --- a/tools/testing/selftests/bpf/progs/trigger_bench.c > > +++ b/tools/testing/selftests/bpf/progs/trigger_bench.c > > @@ -39,26 +39,19 @@ int bench_trigger_uprobe_multi(void *ctx) > > return 0; > > } > > > > +const volatile int kernel_count = 0; > > nit: use bool? it's not a counter, no need to use int here > > > const volatile int batch_iters = 0; > > > > -SEC("?raw_tp") > > -int trigger_count(void *ctx) > > -{ > > - int i; > > - > > - for (i = 0; i < batch_iters; i++) > > - inc_counter(); > > - > > - return 0; > > -} > > - > > SEC("?raw_tp") > > int trigger_driver(void *ctx) > > { > > int i; > > > > - for (i = 0; i < batch_iters; i++) > > + for (i = 0; i < batch_iters; i++) { > > (void)bpf_get_numa_node_id(); /* attach point for > > benchmarking */ > > + if (kernel_count) > > + inc_counter(); > > + } > > > tbh, I wouldn't touch trigger_driver() adding unnecessary if > conditions to it. It's fine, IMO, to have bpf_get_numa_node_id() call > in trigger_count() for being closer in terms of actual work being > done, but I'd keep trigger_driver and trigger_count separate (maybe > renaming trigger_count to trigger_kernel_count would help, I don't > know) Ah, OK! I'll add the call to bpf_get_numa_node_id() in trigger_count() instead. I think the "trigger_kernel_count" makes more sense to me. Thanks! Menglong Dong > > pw-bot: cr > > > > > return 0; > > } > > -- > > 2.51.2 > >
