2023-04-28 00:21:35

by Namhyung Kim

Subject: [PATCH 1/2] perf lock contention: Fix struct rq lock access

The BPF CO-RE's ignore suffix rule requires three underscores.
Otherwise it'd fail like below:

$ sudo perf lock contention -ab
libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument
libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG --
reg type unsupported for arg#0 function collect_lock_syms#380
; int BPF_PROG(collect_lock_syms)
0: (b7) r6 = 0 ; R6_w=0
1: (b7) r7 = 0 ; R7_w=0
2: (b7) r9 = 1 ; R9_w=1
3: <invalid CO-RE relocation>
failed to resolve CO-RE relocation <byte_off> [381] struct rq__new.__lock (0:0 @ offset 0)

Fixes: 0c1228486bef ("perf lock contention: Support pre-5.14 kernels")
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/bpf_skel/lock_contention.bpf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 8911e2a077d8..30c193078bdb 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -418,11 +418,11 @@ int contention_end(u64 *ctx)

extern struct rq runqueues __ksym;

-struct rq__old {
+struct rq___old {
raw_spinlock_t lock;
} __attribute__((preserve_access_index));

-struct rq__new {
+struct rq___new {
raw_spinlock_t __lock;
} __attribute__((preserve_access_index));

@@ -434,8 +434,8 @@ int BPF_PROG(collect_lock_syms)

for (int i = 0; i < MAX_CPUS; i++) {
struct rq *rq = bpf_per_cpu_ptr(&runqueues, i);
- struct rq__new *rq_new = (void *)rq;
- struct rq__old *rq_old = (void *)rq;
+ struct rq___new *rq_new = (void *)rq;
+ struct rq___old *rq_old = (void *)rq;

if (rq == NULL)
break;
--
2.40.1.495.gc816e09b53d-goog

2023-04-28 00:21:57

by Namhyung Kim

Subject: [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE

It seems BPF CO-RE reloc doesn't work well with the pattern that gets
the field-offset only. Use offsetof() to make it explicit so that
the compiler would generate the correct code.

Fixes: 0c1228486bef ("perf lock contention: Support pre-5.14 kernels")
Co-developed-by: Andrii Nakryiko <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/bpf_skel/lock_contention.bpf.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
index 30c193078bdb..8d3cfbb3cc65 100644
--- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
+++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
@@ -429,21 +429,21 @@ struct rq___new {
SEC("raw_tp/bpf_test_finish")
int BPF_PROG(collect_lock_syms)
{
- __u64 lock_addr;
+ __u64 lock_addr, lock_off;
__u32 lock_flag;

+ if (bpf_core_field_exists(struct rq___new, __lock))
+ lock_off = offsetof(struct rq___new, __lock);
+ else
+ lock_off = offsetof(struct rq___old, lock);
+
for (int i = 0; i < MAX_CPUS; i++) {
struct rq *rq = bpf_per_cpu_ptr(&runqueues, i);
- struct rq___new *rq_new = (void *)rq;
- struct rq___old *rq_old = (void *)rq;

if (rq == NULL)
break;

- if (bpf_core_field_exists(rq_new->__lock))
- lock_addr = (__u64)&rq_new->__lock;
- else
- lock_addr = (__u64)&rq_old->lock;
+ lock_addr = (__u64)(void *)rq + lock_off;
lock_flag = LOCK_CLASS_RQLOCK;
bpf_map_update_elem(&lock_syms, &lock_addr, &lock_flag, BPF_ANY);
}
--
2.40.1.495.gc816e09b53d-goog

2023-04-28 00:33:24

by Ian Rogers

Subject: Re: [PATCH 1/2] perf lock contention: Fix struct rq lock access

On Thu, Apr 27, 2023 at 4:48 PM Namhyung Kim <[email protected]> wrote:
>
> The BPF CO-RE's ignore suffix rule requires three underscores.
> Otherwise it'd fail like below:
>
> $ sudo perf lock contention -ab
> libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument
> libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG --
> reg type unsupported for arg#0 function collect_lock_syms#380
> ; int BPF_PROG(collect_lock_syms)
> 0: (b7) r6 = 0 ; R6_w=0
> 1: (b7) r7 = 0 ; R7_w=0
> 2: (b7) r9 = 1 ; R9_w=1
> 3: <invalid CO-RE relocation>
> failed to resolve CO-RE relocation <byte_off> [381] struct rq__new.__lock (0:0 @ offset 0)
>
> Fixes: 0c1228486bef ("perf lock contention: Support pre-5.14 kernels")
> Signed-off-by: Namhyung Kim <[email protected]>

Acked-by: Ian Rogers <[email protected]>

Thanks,
Ian

> ---
> tools/perf/util/bpf_skel/lock_contention.bpf.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 8911e2a077d8..30c193078bdb 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> @@ -418,11 +418,11 @@ int contention_end(u64 *ctx)
>
> extern struct rq runqueues __ksym;
>
> -struct rq__old {
> +struct rq___old {
> raw_spinlock_t lock;
> } __attribute__((preserve_access_index));
>
> -struct rq__new {
> +struct rq___new {
> raw_spinlock_t __lock;
> } __attribute__((preserve_access_index));
>
> @@ -434,8 +434,8 @@ int BPF_PROG(collect_lock_syms)
>
> for (int i = 0; i < MAX_CPUS; i++) {
> struct rq *rq = bpf_per_cpu_ptr(&runqueues, i);
> - struct rq__new *rq_new = (void *)rq;
> - struct rq__old *rq_old = (void *)rq;
> + struct rq___new *rq_new = (void *)rq;
> + struct rq___old *rq_old = (void *)rq;
>
> if (rq == NULL)
> break;
> --
> 2.40.1.495.gc816e09b53d-goog
>

2023-04-28 00:33:34

by Ian Rogers

Subject: Re: [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE

On Thu, Apr 27, 2023 at 4:48 PM Namhyung Kim <[email protected]> wrote:
>
> It seems BPF CO-RE reloc doesn't work well with the pattern that gets
> the field-offset only. Use offsetof() to make it explicit so that
> the compiler would generate the correct code.
>
> Fixes: 0c1228486bef ("perf lock contention: Support pre-5.14 kernels")
> Co-developed-by: Andrii Nakryiko <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>

Acked-by: Ian Rogers <[email protected]>

Thanks,
Ian

> ---
> tools/perf/util/bpf_skel/lock_contention.bpf.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> index 30c193078bdb..8d3cfbb3cc65 100644
> --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> @@ -429,21 +429,21 @@ struct rq___new {
> SEC("raw_tp/bpf_test_finish")
> int BPF_PROG(collect_lock_syms)
> {
> - __u64 lock_addr;
> + __u64 lock_addr, lock_off;
> __u32 lock_flag;
>
> + if (bpf_core_field_exists(struct rq___new, __lock))
> + lock_off = offsetof(struct rq___new, __lock);
> + else
> + lock_off = offsetof(struct rq___old, lock);
> +
> for (int i = 0; i < MAX_CPUS; i++) {
> struct rq *rq = bpf_per_cpu_ptr(&runqueues, i);
> - struct rq___new *rq_new = (void *)rq;
> - struct rq___old *rq_old = (void *)rq;
>
> if (rq == NULL)
> break;
>
> - if (bpf_core_field_exists(rq_new->__lock))
> - lock_addr = (__u64)&rq_new->__lock;
> - else
> - lock_addr = (__u64)&rq_old->lock;
> + lock_addr = (__u64)(void *)rq + lock_off;
> lock_flag = LOCK_CLASS_RQLOCK;
> bpf_map_update_elem(&lock_syms, &lock_addr, &lock_flag, BPF_ANY);
> }
> --
> 2.40.1.495.gc816e09b53d-goog
>

2023-04-29 02:00:33

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH 1/2] perf lock contention: Fix struct rq lock access

Em Thu, Apr 27, 2023 at 05:32:08PM -0700, Ian Rogers escreveu:
> On Thu, Apr 27, 2023 at 4:48 PM Namhyung Kim <[email protected]> wrote:
> >
> > The BPF CO-RE's ignore suffix rule requires three underscores.
> > Otherwise it'd fail like below:
> >
> > $ sudo perf lock contention -ab
> > libbpf: prog 'collect_lock_syms': BPF program load failed: Invalid argument
> > libbpf: prog 'collect_lock_syms': -- BEGIN PROG LOAD LOG --
> > reg type unsupported for arg#0 function collect_lock_syms#380
> > ; int BPF_PROG(collect_lock_syms)
> > 0: (b7) r6 = 0 ; R6_w=0
> > 1: (b7) r7 = 0 ; R7_w=0
> > 2: (b7) r9 = 1 ; R9_w=1
> > 3: <invalid CO-RE relocation>
> > failed to resolve CO-RE relocation <byte_off> [381] struct rq__new.__lock (0:0 @ offset 0)
> >
> > Fixes: 0c1228486bef ("perf lock contention: Support pre-5.14 kernels")
> > Signed-off-by: Namhyung Kim <[email protected]>
>
> Acked-by: Ian Rogers <[email protected]>

Thanks, applied the series.

- Arnaldo


> Thanks,
> Ian
>
> > ---
> > tools/perf/util/bpf_skel/lock_contention.bpf.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/bpf_skel/lock_contention.bpf.c b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > index 8911e2a077d8..30c193078bdb 100644
> > --- a/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > +++ b/tools/perf/util/bpf_skel/lock_contention.bpf.c
> > @@ -418,11 +418,11 @@ int contention_end(u64 *ctx)
> >
> > extern struct rq runqueues __ksym;
> >
> > -struct rq__old {
> > +struct rq___old {
> > raw_spinlock_t lock;
> > } __attribute__((preserve_access_index));
> >
> > -struct rq__new {
> > +struct rq___new {
> > raw_spinlock_t __lock;
> > } __attribute__((preserve_access_index));
> >
> > @@ -434,8 +434,8 @@ int BPF_PROG(collect_lock_syms)
> >
> > for (int i = 0; i < MAX_CPUS; i++) {
> > struct rq *rq = bpf_per_cpu_ptr(&runqueues, i);
> > - struct rq__new *rq_new = (void *)rq;
> > - struct rq__old *rq_old = (void *)rq;
> > + struct rq___new *rq_new = (void *)rq;
> > + struct rq___old *rq_old = (void *)rq;
> >
> > if (rq == NULL)
> > break;
> > --
> > 2.40.1.495.gc816e09b53d-goog
> >

--

- Arnaldo