2024-04-06 04:09:29

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v1 1/2] perf bench uprobe: Remove lib64 from libc.so.6 binary path

bpf_program__attach_uprobe_opts will search LD_LIBRARY_PATH and so
specifying `/lib64` is unnecessary and causes failures for libc.so.6
paths like `/lib/x86_64-linux-gnu/libc.so.6`.

Fixes: 7b47623b8cae ("perf bench uprobe trace_printk: Add entry attaching an BPF program that does a trace_printk")
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/bench/uprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/bench/uprobe.c b/tools/perf/bench/uprobe.c
index 5c71fdc419dd..b722ff88fe7d 100644
--- a/tools/perf/bench/uprobe.c
+++ b/tools/perf/bench/uprobe.c
@@ -47,7 +47,7 @@ static const char * const bench_uprobe_usage[] = {
#define bench_uprobe__attach_uprobe(prog) \
skel->links.prog = bpf_program__attach_uprobe_opts(/*prog=*/skel->progs.prog, \
/*pid=*/-1, \
- /*binary_path=*/"/lib64/libc.so.6", \
+ /*binary_path=*/"libc.so.6", \
/*func_offset=*/0, \
/*opts=*/&uprobe_opts); \
if (!skel->links.prog) { \
--
2.44.0.478.gd926399ef9-goog



2024-04-06 04:09:47

by Ian Rogers

[permalink] [raw]
Subject: [PATCH v1 2/2] perf bench uprobe: Add uretprobe variant of uprobe benchmarks

Name benchmarks with _ret at the end to avoid creating a new set of
benchmarks.

Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/bench/bench.h | 2 ++
tools/perf/bench/uprobe.c | 20 +++++++++++++++++---
tools/perf/builtin-bench.c | 2 ++
tools/perf/util/bpf_skel/bench_uprobe.bpf.c | 16 ++++++++++++++++
4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/tools/perf/bench/bench.h b/tools/perf/bench/bench.h
index faa18e6d2467..9f736423af53 100644
--- a/tools/perf/bench/bench.h
+++ b/tools/perf/bench/bench.h
@@ -46,6 +46,8 @@ int bench_breakpoint_enable(int argc, const char **argv);
int bench_uprobe_baseline(int argc, const char **argv);
int bench_uprobe_empty(int argc, const char **argv);
int bench_uprobe_trace_printk(int argc, const char **argv);
+int bench_uprobe_empty_ret(int argc, const char **argv);
+int bench_uprobe_trace_printk_ret(int argc, const char **argv);
int bench_pmu_scan(int argc, const char **argv);

#define BENCH_FORMAT_DEFAULT_STR "default"
diff --git a/tools/perf/bench/uprobe.c b/tools/perf/bench/uprobe.c
index b722ff88fe7d..0b90275862e1 100644
--- a/tools/perf/bench/uprobe.c
+++ b/tools/perf/bench/uprobe.c
@@ -26,9 +26,11 @@
static int loops = LOOPS_DEFAULT;

enum bench_uprobe {
- BENCH_UPROBE__BASELINE,
- BENCH_UPROBE__EMPTY,
- BENCH_UPROBE__TRACE_PRINTK,
+ BENCH_UPROBE__BASELINE,
+ BENCH_UPROBE__EMPTY,
+ BENCH_UPROBE__TRACE_PRINTK,
+ BENCH_UPROBE__EMPTY_RET,
+ BENCH_UPROBE__TRACE_PRINTK_RET,
};

static const struct option options[] = {
@@ -81,6 +83,8 @@ static int bench_uprobe__setup_bpf_skel(enum bench_uprobe bench)
case BENCH_UPROBE__BASELINE: break;
case BENCH_UPROBE__EMPTY: bench_uprobe__attach_uprobe(empty); break;
case BENCH_UPROBE__TRACE_PRINTK: bench_uprobe__attach_uprobe(trace_printk); break;
+ case BENCH_UPROBE__EMPTY_RET: bench_uprobe__attach_uprobe(empty_ret); break;
+ case BENCH_UPROBE__TRACE_PRINTK_RET: bench_uprobe__attach_uprobe(trace_printk_ret); break;
default:
fprintf(stderr, "Invalid bench: %d\n", bench);
goto cleanup;
@@ -197,3 +201,13 @@ int bench_uprobe_trace_printk(int argc, const char **argv)
{
return bench_uprobe(argc, argv, BENCH_UPROBE__TRACE_PRINTK);
}
+
+int bench_uprobe_empty_ret(int argc, const char **argv)
+{
+ return bench_uprobe(argc, argv, BENCH_UPROBE__EMPTY_RET);
+}
+
+int bench_uprobe_trace_printk_ret(int argc, const char **argv)
+{
+ return bench_uprobe(argc, argv, BENCH_UPROBE__TRACE_PRINTK_RET);
+}
diff --git a/tools/perf/builtin-bench.c b/tools/perf/builtin-bench.c
index 1a8898d5b560..2c1a9f3d847a 100644
--- a/tools/perf/builtin-bench.c
+++ b/tools/perf/builtin-bench.c
@@ -109,6 +109,8 @@ static struct bench uprobe_benchmarks[] = {
{ "baseline", "Baseline libc usleep(1000) call", bench_uprobe_baseline, },
{ "empty", "Attach empty BPF prog to uprobe on usleep, system wide", bench_uprobe_empty, },
{ "trace_printk", "Attach trace_printk BPF prog to uprobe on usleep syswide", bench_uprobe_trace_printk, },
+ { "empty_ret", "Attach empty BPF prog to uretprobe on usleep, system wide", bench_uprobe_empty_ret, },
+ { "trace_printk_ret", "Attach trace_printk BPF prog to uretprobe on usleep syswide", bench_uprobe_trace_printk_ret,},
{ NULL, NULL, NULL },
};

diff --git a/tools/perf/util/bpf_skel/bench_uprobe.bpf.c b/tools/perf/util/bpf_skel/bench_uprobe.bpf.c
index 2c55896bb33c..a01c7f791fcd 100644
--- a/tools/perf/util/bpf_skel/bench_uprobe.bpf.c
+++ b/tools/perf/util/bpf_skel/bench_uprobe.bpf.c
@@ -4,6 +4,7 @@
#include <bpf/bpf_tracing.h>

unsigned int nr_uprobes;
+unsigned int nr_uretprobes;

SEC("uprobe")
int BPF_UPROBE(empty)
@@ -20,4 +21,19 @@ int BPF_UPROBE(trace_printk)
return 0;
}

+SEC("uretprobe")
+int BPF_URETPROBE(empty_ret)
+{
+ return 0;
+}
+
+SEC("uretprobe")
+int BPF_URETPROBE(trace_printk_ret)
+{
+ char fmt[] = "perf bench uretprobe %u";
+
+ bpf_trace_printk(fmt, sizeof(fmt), ++nr_uretprobes);
+ return 0;
+}
+
char LICENSE[] SEC("license") = "Dual BSD/GPL";
--
2.44.0.478.gd926399ef9-goog


2024-04-10 07:19:08

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] perf bench uprobe: Remove lib64 from libc.so.6 binary path

On Fri, Apr 05, 2024 at 09:09:10PM -0700, Ian Rogers wrote:
> bpf_program__attach_uprobe_opts will search LD_LIBRARY_PATH and so
> specifying `/lib64` is unnecessary and causes failures for libc.so.6
> paths like `/lib/x86_64-linux-gnu/libc.so.6`.
>
> Fixes: 7b47623b8cae ("perf bench uprobe trace_printk: Add entry attaching an BPF program that does a trace_printk")
> Signed-off-by: Ian Rogers <[email protected]>

patchset lgtm

Acked-by: Jiri Olsa <[email protected]>

jirka

> ---
> tools/perf/bench/uprobe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/bench/uprobe.c b/tools/perf/bench/uprobe.c
> index 5c71fdc419dd..b722ff88fe7d 100644
> --- a/tools/perf/bench/uprobe.c
> +++ b/tools/perf/bench/uprobe.c
> @@ -47,7 +47,7 @@ static const char * const bench_uprobe_usage[] = {
> #define bench_uprobe__attach_uprobe(prog) \
> skel->links.prog = bpf_program__attach_uprobe_opts(/*prog=*/skel->progs.prog, \
> /*pid=*/-1, \
> - /*binary_path=*/"/lib64/libc.so.6", \
> + /*binary_path=*/"libc.so.6", \
> /*func_offset=*/0, \
> /*opts=*/&uprobe_opts); \
> if (!skel->links.prog) { \
> --
> 2.44.0.478.gd926399ef9-goog
>

2024-04-12 20:52:35

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] perf bench uprobe: Remove lib64 from libc.so.6 binary path

On Wed, Apr 10, 2024 at 09:18:44AM +0200, Jiri Olsa wrote:
> On Fri, Apr 05, 2024 at 09:09:10PM -0700, Ian Rogers wrote:
> > bpf_program__attach_uprobe_opts will search LD_LIBRARY_PATH and so
> > specifying `/lib64` is unnecessary and causes failures for libc.so.6
> > paths like `/lib/x86_64-linux-gnu/libc.so.6`.
> >
> > Fixes: 7b47623b8cae ("perf bench uprobe trace_printk: Add entry attaching an BPF program that does a trace_printk")
> > Signed-off-by: Ian Rogers <[email protected]>
>
> patchset lgtm
>
> Acked-by: Jiri Olsa <[email protected]>

Thanks, applied it to the series, b4 picked it just for this patch.

I tried to convince Konstantin to look for "patchset lgtm", "for the
series", but for now we need to do it manually :-)

- Arnaldo