All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] perf build: Fix check for btf__load_from_kernel_by_id() in libbpf
@ 2022-05-20  0:40 Arnaldo Carvalho de Melo
  2022-05-20 12:48 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-05-20  0:40 UTC (permalink / raw)
  To: Avi Kivity
  Cc: linux-perf-users, Jiri Olsa, Ian Rogers, Namhyung Kim,
	Adrian Hunter, Linux Kernel Mailing List

Avi, can you please try with this patch applied?

Thanks,

- Arnaldo

---

Avi Kivity reported a problem where the __weak
btf__load_from_kernel_by_id() in tools/perf/util/bpf-event.c was being
used and it called btf__get_from_id() in tools/lib/bpf/btf.c that in
turn called back to btf__load_from_kernel_by_id(), resulting in an
endless loop.

Fix this by adding a feature test to check if
btf__load_from_kernel_by_id() is available when building perf with
LIBBPF_DYNAMIC=1, and if not then provide the fallback to the old
btf__get_from_id(), that doesn't call back to btf__load_from_kernel_by_id()
since at that time it didn't exist at all.

Tested on Fedora 35 where we have libbpf-devel 0.4.0 with LIBBPF_DYNAMIC
where we don't have btf__load_from_kernel_by_id() and thus its feature
test fail, not defining HAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID:

  $ cat /tmp/build/perf-urgent/feature/test-libbpf-btf__load_from_kernel_by_id.make.output
  test-libbpf-btf__load_from_kernel_by_id.c: In function ‘main’:
  test-libbpf-btf__load_from_kernel_by_id.c:6:16: error: implicit declaration of function ‘btf__load_from_kernel_by_id’ [-Werror=implicit-function-declaration]
      6 |         return btf__load_from_kernel_by_id(20151128, NULL);
        |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  cc1: all warnings being treated as errors
  $

  $ nm /tmp/build/perf-urgent/perf | grep btf__load_from_kernel_by_id
  00000000005ba180 T btf__load_from_kernel_by_id
  $

  $ objdump --disassemble=btf__load_from_kernel_by_id -S /tmp/build/perf-urgent/perf

  /tmp/build/perf-urgent/perf:     file format elf64-x86-64
  <SNIP>
  00000000005ba180 <btf__load_from_kernel_by_id>:
  #include "record.h"
  #include "util/synthetic-events.h"

  #ifndef HAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
  struct btf *btf__load_from_kernel_by_id(__u32 id)
  {
    5ba180:	55                   	push   %rbp
    5ba181:	48 89 e5             	mov    %rsp,%rbp
    5ba184:	48 83 ec 10          	sub    $0x10,%rsp
    5ba188:	64 48 8b 04 25 28 00 	mov    %fs:0x28,%rax
    5ba18f:	00 00
    5ba191:	48 89 45 f8          	mov    %rax,-0x8(%rbp)
    5ba195:	31 c0                	xor    %eax,%eax
         struct btf *btf;
  #pragma GCC diagnostic push
  #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
         int err = btf__get_from_id(id, &btf);
    5ba197:	48 8d 75 f0          	lea    -0x10(%rbp),%rsi
    5ba19b:	e8 a0 57 e5 ff       	call   40f940 <btf__get_from_id@plt>
    5ba1a0:	89 c2                	mov    %eax,%edx
  #pragma GCC diagnostic pop

         return err ? ERR_PTR(err) : btf;
    5ba1a2:	48 98                	cltq
    5ba1a4:	85 d2                	test   %edx,%edx
    5ba1a6:	48 0f 44 45 f0       	cmove  -0x10(%rbp),%rax
  }
  <SNIP>

Fixes: 218e7b775d368f38 ("perf bpf: Provide a weak btf__load_from_kernel_by_id() for older libbpf versions")
Link: https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/build/Makefile.feature                               | 1 +
 tools/build/feature/Makefile                               | 4 ++++
 .../feature/test-libbpf-btf__load_from_kernel_by_id.c      | 7 +++++++
 tools/perf/Makefile.config                                 | 7 +++++++
 tools/perf/util/bpf-event.c                                | 4 +++-
 5 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c

diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
index ae61f464043a11fb..c6a48d0ef9ff0648 100644
--- a/tools/build/Makefile.feature
+++ b/tools/build/Makefile.feature
@@ -98,6 +98,7 @@ FEATURE_TESTS_EXTRA :=                  \
          llvm-version                   \
          clang                          \
          libbpf                         \
+         libbpf-btf__load_from_kernel_by_id \
          libpfm4                        \
          libdebuginfod			\
          clang-bpf-co-re
diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
index de66e1cc073481c5..cb4a2a4fa2e48ebf 100644
--- a/tools/build/feature/Makefile
+++ b/tools/build/feature/Makefile
@@ -57,6 +57,7 @@ FILES=                                          \
          test-lzma.bin                          \
          test-bpf.bin                           \
          test-libbpf.bin                        \
+         test-libbpf-btf__load_from_kernel_by_id.bin	\
          test-get_cpuid.bin                     \
          test-sdt.bin                           \
          test-cxx.bin                           \
@@ -287,6 +288,9 @@ $(OUTPUT)test-bpf.bin:
 $(OUTPUT)test-libbpf.bin:
 	$(BUILD) -lbpf
 
+$(OUTPUT)test-libbpf-btf__load_from_kernel_by_id.bin:
+	$(BUILD) -lbpf
+
 $(OUTPUT)test-sdt.bin:
 	$(BUILD)
 
diff --git a/tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c b/tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c
new file mode 100644
index 0000000000000000..f7c084428735a73d
--- /dev/null
+++ b/tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c
@@ -0,0 +1,7 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <bpf/libbpf.h>
+
+int main(void)
+{
+	return btf__load_from_kernel_by_id(20151128, NULL);
+}
diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
index f3bf9297bcc03c5e..1bd64e7404b9fbd4 100644
--- a/tools/perf/Makefile.config
+++ b/tools/perf/Makefile.config
@@ -553,9 +553,16 @@ ifndef NO_LIBELF
         ifeq ($(feature-libbpf), 1)
           EXTLIBS += -lbpf
           $(call detected,CONFIG_LIBBPF_DYNAMIC)
+
+          $(call feature_check,libbpf-btf__load_from_kernel_by_id)
+          ifeq ($(feature-libbpf-btf__load_from_kernel_by_id), 1)
+            CFLAGS += -DHAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
+          endif
         else
           dummy := $(error Error: No libbpf devel library found, please install libbpf-devel);
         endif
+      else
+	CFLAGS += -DHAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
       endif
     endif
 
diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
index 94624733af7e225a..8271ab764eb56c7b 100644
--- a/tools/perf/util/bpf-event.c
+++ b/tools/perf/util/bpf-event.c
@@ -22,7 +22,8 @@
 #include "record.h"
 #include "util/synthetic-events.h"
 
-struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
+#ifndef HAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
+struct btf *btf__load_from_kernel_by_id(__u32 id)
 {
        struct btf *btf;
 #pragma GCC diagnostic push
@@ -32,6 +33,7 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
 
        return err ? ERR_PTR(err) : btf;
 }
+#endif
 
 int __weak bpf_prog_load(enum bpf_prog_type prog_type,
 			 const char *prog_name __maybe_unused,
-- 
2.35.1


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

* Re: [PATCH 1/1] perf build: Fix check for btf__load_from_kernel_by_id() in libbpf
  2022-05-20  0:40 [PATCH 1/1] perf build: Fix check for btf__load_from_kernel_by_id() in libbpf Arnaldo Carvalho de Melo
@ 2022-05-20 12:48 ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-05-20 12:48 UTC (permalink / raw)
  To: Avi Kivity
  Cc: linux-perf-users, Jiri Olsa, Ian Rogers, Namhyung Kim,
	Adrian Hunter, Linux Kernel Mailing List

Em Thu, May 19, 2022 at 09:40:44PM -0300, Arnaldo Carvalho de Melo escreveu:
> Avi, can you please try with this patch applied?
> 

Forgot to add:

Reported-by: Avi Kivity <avi@scylladb.com>

Fixed locally, if you managed to test this, please ack :-)

- Arnaldo
> 
> ---
> 
> Avi Kivity reported a problem where the __weak
> btf__load_from_kernel_by_id() in tools/perf/util/bpf-event.c was being
> used and it called btf__get_from_id() in tools/lib/bpf/btf.c that in
> turn called back to btf__load_from_kernel_by_id(), resulting in an
> endless loop.
> 
> Fix this by adding a feature test to check if
> btf__load_from_kernel_by_id() is available when building perf with
> LIBBPF_DYNAMIC=1, and if not then provide the fallback to the old
> btf__get_from_id(), that doesn't call back to btf__load_from_kernel_by_id()
> since at that time it didn't exist at all.
> 
> Tested on Fedora 35 where we have libbpf-devel 0.4.0 with LIBBPF_DYNAMIC
> where we don't have btf__load_from_kernel_by_id() and thus its feature
> test fail, not defining HAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID:
> 
>   $ cat /tmp/build/perf-urgent/feature/test-libbpf-btf__load_from_kernel_by_id.make.output
>   test-libbpf-btf__load_from_kernel_by_id.c: In function ‘main’:
>   test-libbpf-btf__load_from_kernel_by_id.c:6:16: error: implicit declaration of function ‘btf__load_from_kernel_by_id’ [-Werror=implicit-function-declaration]
>       6 |         return btf__load_from_kernel_by_id(20151128, NULL);
>         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>   cc1: all warnings being treated as errors
>   $
> 
>   $ nm /tmp/build/perf-urgent/perf | grep btf__load_from_kernel_by_id
>   00000000005ba180 T btf__load_from_kernel_by_id
>   $
> 
>   $ objdump --disassemble=btf__load_from_kernel_by_id -S /tmp/build/perf-urgent/perf
> 
>   /tmp/build/perf-urgent/perf:     file format elf64-x86-64
>   <SNIP>
>   00000000005ba180 <btf__load_from_kernel_by_id>:
>   #include "record.h"
>   #include "util/synthetic-events.h"
> 
>   #ifndef HAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
>   struct btf *btf__load_from_kernel_by_id(__u32 id)
>   {
>     5ba180:	55                   	push   %rbp
>     5ba181:	48 89 e5             	mov    %rsp,%rbp
>     5ba184:	48 83 ec 10          	sub    $0x10,%rsp
>     5ba188:	64 48 8b 04 25 28 00 	mov    %fs:0x28,%rax
>     5ba18f:	00 00
>     5ba191:	48 89 45 f8          	mov    %rax,-0x8(%rbp)
>     5ba195:	31 c0                	xor    %eax,%eax
>          struct btf *btf;
>   #pragma GCC diagnostic push
>   #pragma GCC diagnostic ignored "-Wdeprecated-declarations"
>          int err = btf__get_from_id(id, &btf);
>     5ba197:	48 8d 75 f0          	lea    -0x10(%rbp),%rsi
>     5ba19b:	e8 a0 57 e5 ff       	call   40f940 <btf__get_from_id@plt>
>     5ba1a0:	89 c2                	mov    %eax,%edx
>   #pragma GCC diagnostic pop
> 
>          return err ? ERR_PTR(err) : btf;
>     5ba1a2:	48 98                	cltq
>     5ba1a4:	85 d2                	test   %edx,%edx
>     5ba1a6:	48 0f 44 45 f0       	cmove  -0x10(%rbp),%rax
>   }
>   <SNIP>
> 
> Fixes: 218e7b775d368f38 ("perf bpf: Provide a weak btf__load_from_kernel_by_id() for older libbpf versions")
> Link: https://lore.kernel.org/linux-perf-users/f0add43b-3de5-20c5-22c4-70aff4af959f@scylladb.com
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/build/Makefile.feature                               | 1 +
>  tools/build/feature/Makefile                               | 4 ++++
>  .../feature/test-libbpf-btf__load_from_kernel_by_id.c      | 7 +++++++
>  tools/perf/Makefile.config                                 | 7 +++++++
>  tools/perf/util/bpf-event.c                                | 4 +++-
>  5 files changed, 22 insertions(+), 1 deletion(-)
>  create mode 100644 tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c
> 
> diff --git a/tools/build/Makefile.feature b/tools/build/Makefile.feature
> index ae61f464043a11fb..c6a48d0ef9ff0648 100644
> --- a/tools/build/Makefile.feature
> +++ b/tools/build/Makefile.feature
> @@ -98,6 +98,7 @@ FEATURE_TESTS_EXTRA :=                  \
>           llvm-version                   \
>           clang                          \
>           libbpf                         \
> +         libbpf-btf__load_from_kernel_by_id \
>           libpfm4                        \
>           libdebuginfod			\
>           clang-bpf-co-re
> diff --git a/tools/build/feature/Makefile b/tools/build/feature/Makefile
> index de66e1cc073481c5..cb4a2a4fa2e48ebf 100644
> --- a/tools/build/feature/Makefile
> +++ b/tools/build/feature/Makefile
> @@ -57,6 +57,7 @@ FILES=                                          \
>           test-lzma.bin                          \
>           test-bpf.bin                           \
>           test-libbpf.bin                        \
> +         test-libbpf-btf__load_from_kernel_by_id.bin	\
>           test-get_cpuid.bin                     \
>           test-sdt.bin                           \
>           test-cxx.bin                           \
> @@ -287,6 +288,9 @@ $(OUTPUT)test-bpf.bin:
>  $(OUTPUT)test-libbpf.bin:
>  	$(BUILD) -lbpf
>  
> +$(OUTPUT)test-libbpf-btf__load_from_kernel_by_id.bin:
> +	$(BUILD) -lbpf
> +
>  $(OUTPUT)test-sdt.bin:
>  	$(BUILD)
>  
> diff --git a/tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c b/tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c
> new file mode 100644
> index 0000000000000000..f7c084428735a73d
> --- /dev/null
> +++ b/tools/build/feature/test-libbpf-btf__load_from_kernel_by_id.c
> @@ -0,0 +1,7 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <bpf/libbpf.h>
> +
> +int main(void)
> +{
> +	return btf__load_from_kernel_by_id(20151128, NULL);
> +}
> diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> index f3bf9297bcc03c5e..1bd64e7404b9fbd4 100644
> --- a/tools/perf/Makefile.config
> +++ b/tools/perf/Makefile.config
> @@ -553,9 +553,16 @@ ifndef NO_LIBELF
>          ifeq ($(feature-libbpf), 1)
>            EXTLIBS += -lbpf
>            $(call detected,CONFIG_LIBBPF_DYNAMIC)
> +
> +          $(call feature_check,libbpf-btf__load_from_kernel_by_id)
> +          ifeq ($(feature-libbpf-btf__load_from_kernel_by_id), 1)
> +            CFLAGS += -DHAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
> +          endif
>          else
>            dummy := $(error Error: No libbpf devel library found, please install libbpf-devel);
>          endif
> +      else
> +	CFLAGS += -DHAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
>        endif
>      endif
>  
> diff --git a/tools/perf/util/bpf-event.c b/tools/perf/util/bpf-event.c
> index 94624733af7e225a..8271ab764eb56c7b 100644
> --- a/tools/perf/util/bpf-event.c
> +++ b/tools/perf/util/bpf-event.c
> @@ -22,7 +22,8 @@
>  #include "record.h"
>  #include "util/synthetic-events.h"
>  
> -struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
> +#ifndef HAVE_LIBBPF_BTF__LOAD_FROM_KERNEL_BY_ID
> +struct btf *btf__load_from_kernel_by_id(__u32 id)
>  {
>         struct btf *btf;
>  #pragma GCC diagnostic push
> @@ -32,6 +33,7 @@ struct btf * __weak btf__load_from_kernel_by_id(__u32 id)
>  
>         return err ? ERR_PTR(err) : btf;
>  }
> +#endif
>  
>  int __weak bpf_prog_load(enum bpf_prog_type prog_type,
>  			 const char *prog_name __maybe_unused,
> -- 
> 2.35.1

-- 

- Arnaldo

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

end of thread, other threads:[~2022-05-20 12:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-20  0:40 [PATCH 1/1] perf build: Fix check for btf__load_from_kernel_by_id() in libbpf Arnaldo Carvalho de Melo
2022-05-20 12:48 ` 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.