Hi Andrii, On Thu, 25 Apr 2024 13:31:53 -0700 Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > Hey Masami, > > I can't really review most of that code as I'm completely unfamiliar > with all those inner workings of fprobe/ftrace/function_graph. I left > a few comments where there were somewhat more obvious BPF-related > pieces. > > But I also did run our BPF benchmarks on probes/for-next as a baseline > and then with your series applied on top. Just to see if there are any > regressions. I think it will be a useful data point for you. Thanks for testing! > > You should be already familiar with the bench tool we have in BPF > selftests (I used it on some other patches for your tree). What patches we need? > > BASELINE > ======== > kprobe : 24.634 ± 0.205M/s > kprobe-multi : 28.898 ± 0.531M/s > kretprobe : 10.478 ± 0.015M/s > kretprobe-multi: 11.012 ± 0.063M/s > > THIS PATCH SET ON TOP > ===================== > kprobe : 25.144 ± 0.027M/s (+2%) > kprobe-multi : 28.909 ± 0.074M/s > kretprobe : 9.482 ± 0.008M/s (-9.5%) > kretprobe-multi: 13.688 ± 0.027M/s (+24%) This looks good. Kretprobe should also use kretprobe-multi (fprobe) eventually because it should be a single callback version of kretprobe-multi. > > These numbers are pretty stable and look to be more or less representative. > > As you can see, kprobes got a bit faster, kprobe-multi seems to be > about the same, though. > > Then (I suppose they are "legacy") kretprobes got quite noticeably > slower, almost by 10%. Not sure why, but looks real after re-running > benchmarks a bunch of times and getting stable results. Hmm, kretprobe on x86 should use ftrace + rethook even with my series. So nothing should be changed. Maybe cache access pattern has been changed? I'll check it with tracefs (to remove the effect from bpf related changes) > > On the other hand, multi-kretprobes got significantly faster (+24%!). > Again, I don't know if it is expected or not, but it's a nice > improvement. Thanks! > > If you have any idea why kretprobes would get so much slower, it would > be nice to look into that and see if you can mitigate the regression > somehow. Thanks! OK, let me check it. Thank you! > > > > 51 files changed, 2325 insertions(+), 882 deletions(-) > > create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc > > > > -- > > Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> > > -- Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
