All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf lock contention: Fix struct rq lock access
@ 2023-04-27 23:48 Namhyung Kim
  2023-04-27 23:48 ` [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE Namhyung Kim
  2023-04-28  0:32 ` [PATCH 1/2] perf lock contention: Fix struct rq lock access Ian Rogers
  0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2023-04-27 23:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users, bpf, Andrii Nakryiko, Hao Luo, Song Liu

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 <namhyung@kernel.org>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE
  2023-04-27 23:48 [PATCH 1/2] perf lock contention: Fix struct rq lock access Namhyung Kim
@ 2023-04-27 23:48 ` Namhyung Kim
  2023-04-28  0:32   ` Ian Rogers
  2023-04-28  0:32 ` [PATCH 1/2] perf lock contention: Fix struct rq lock access Ian Rogers
  1 sibling, 1 reply; 5+ messages in thread
From: Namhyung Kim @ 2023-04-27 23:48 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jiri Olsa
  Cc: Ian Rogers, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users, bpf, Andrii Nakryiko, Hao Luo, Song Liu,
	Andrii Nakryiko

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 <andrii.nakryiko@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] perf lock contention: Fix struct rq lock access
  2023-04-27 23:48 [PATCH 1/2] perf lock contention: Fix struct rq lock access Namhyung Kim
  2023-04-27 23:48 ` [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE Namhyung Kim
@ 2023-04-28  0:32 ` Ian Rogers
  2023-04-29  1:27   ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 5+ messages in thread
From: Ian Rogers @ 2023-04-28  0:32 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, bpf,
	Andrii Nakryiko, Hao Luo, Song Liu

On Thu, Apr 27, 2023 at 4:48 PM Namhyung Kim <namhyung@kernel.org> 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 <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE
  2023-04-27 23:48 ` [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE Namhyung Kim
@ 2023-04-28  0:32   ` Ian Rogers
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Rogers @ 2023-04-28  0:32 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, bpf,
	Andrii Nakryiko, Hao Luo, Song Liu, Andrii Nakryiko

On Thu, Apr 27, 2023 at 4:48 PM Namhyung Kim <namhyung@kernel.org> 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 <andrii.nakryiko@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.com>

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
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] perf lock contention: Fix struct rq lock access
  2023-04-28  0:32 ` [PATCH 1/2] perf lock contention: Fix struct rq lock access Ian Rogers
@ 2023-04-29  1:27   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-29  1:27 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Namhyung Kim, Jiri Olsa, Adrian Hunter, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users, bpf, Andrii Nakryiko,
	Hao Luo, Song Liu

Em Thu, Apr 27, 2023 at 05:32:08PM -0700, Ian Rogers escreveu:
> On Thu, Apr 27, 2023 at 4:48 PM Namhyung Kim <namhyung@kernel.org> 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 <namhyung@kernel.org>
> 
> Acked-by: Ian Rogers <irogers@google.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-04-29  1:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-27 23:48 [PATCH 1/2] perf lock contention: Fix struct rq lock access Namhyung Kim
2023-04-27 23:48 ` [PATCH 2/2] perf lock contention: Rework offset calculation with BPF CO-RE Namhyung Kim
2023-04-28  0:32   ` Ian Rogers
2023-04-28  0:32 ` [PATCH 1/2] perf lock contention: Fix struct rq lock access Ian Rogers
2023-04-29  1:27   ` Arnaldo Carvalho de Melo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.