linux-perf-users.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).