diff mbox series

[6/9] bpf: Compile bpfwl tool at kernel compilation start

Message ID 20200506132946.2164578-7-jolsa@kernel.org
State RFC
Delegated to: BPF Maintainers
Headers show
Series bpf: Add d_path helper | expand

Commit Message

Jiri Olsa May 6, 2020, 1:29 p.m. UTC
The bpfwl tool will be used during the vmlinux linking,
so it's necessary it's ready.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 Makefile           | 21 +++++++++++++++++----
 tools/Makefile     |  3 +++
 tools/bpf/Makefile |  5 ++++-
 3 files changed, 24 insertions(+), 5 deletions(-)

Comments

Andrii Nakryiko May 14, 2020, 10:38 p.m. UTC | #1
On Wed, May 6, 2020 at 6:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> The bpfwl tool will be used during the vmlinux linking,
> so it's necessary it's ready.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  Makefile           | 21 +++++++++++++++++----
>  tools/Makefile     |  3 +++
>  tools/bpf/Makefile |  5 ++++-
>  3 files changed, 24 insertions(+), 5 deletions(-)
>

[...]

>
> +prepare-bpfwl: $(bpfwl_target)
> +ifeq ($(SKIP_BTF_WHITELIST_GENERATION),1)
> +       @echo "warning: Cannot use BTF whitelist checks, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
> +endif

When we added BTF dedup and generation first time, we also made pahole
unavailability or any error during deduplication process an error. It
actually was very confusing to users and they often missed that BTF
generation didn't happen, but they would notice it only at runtime
(after a confusing debugging session).

So I wonder if it's better to make this an error instead? Just guard
whitelist generation on whether CONFIG_DEBUG_INFO_BTF is enabled or
not?

>  # Generate some files
>  # ---------------------------------------------------------------------------
>
> diff --git a/tools/Makefile b/tools/Makefile
> index bd778812e915..85af6ebbce91 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -67,6 +67,9 @@ cpupower: FORCE
>  cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
>         $(call descend,$@)
>
> +bpf/%: FORCE
> +       $(call descend,$@)
> +
>  liblockdep: FORCE
>         $(call descend,lib/lockdep)
>
> diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> index f897eeeb0b4f..d4ea2b5a2e58 100644
> --- a/tools/bpf/Makefile
> +++ b/tools/bpf/Makefile
> @@ -124,5 +124,8 @@ runqslower_install:
>  runqslower_clean:
>         $(call descend,runqslower,clean)
>
> +bpfwl:
> +       $(call descend,bpfwl)
> +
>  .PHONY: all install clean bpftool bpftool_install bpftool_clean \
> -       runqslower runqslower_install runqslower_clean
> +       runqslower runqslower_install runqslower_clean bpfwl

what about install/clean subcommands? At least clean seems like a good idea?

> --
> 2.25.4
>
Jiri Olsa May 15, 2020, 2:57 p.m. UTC | #2
On Thu, May 14, 2020 at 03:38:57PM -0700, Andrii Nakryiko wrote:
> On Wed, May 6, 2020 at 6:31 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > The bpfwl tool will be used during the vmlinux linking,
> > so it's necessary it's ready.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  Makefile           | 21 +++++++++++++++++----
> >  tools/Makefile     |  3 +++
> >  tools/bpf/Makefile |  5 ++++-
> >  3 files changed, 24 insertions(+), 5 deletions(-)
> >
> 
> [...]
> 
> >
> > +prepare-bpfwl: $(bpfwl_target)
> > +ifeq ($(SKIP_BTF_WHITELIST_GENERATION),1)
> > +       @echo "warning: Cannot use BTF whitelist checks, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
> > +endif
> 
> When we added BTF dedup and generation first time, we also made pahole
> unavailability or any error during deduplication process an error. It
> actually was very confusing to users and they often missed that BTF
> generation didn't happen, but they would notice it only at runtime
> (after a confusing debugging session).
> 
> So I wonder if it's better to make this an error instead? Just guard
> whitelist generation on whether CONFIG_DEBUG_INFO_BTF is enabled or
> not?

ok, makes sense.. I'll let it fail if there's CONFIG_DEBUG_INFO_BTF
enabled and we'are missing libelf

> 
> >  # Generate some files
> >  # ---------------------------------------------------------------------------
> >
> > diff --git a/tools/Makefile b/tools/Makefile
> > index bd778812e915..85af6ebbce91 100644
> > --- a/tools/Makefile
> > +++ b/tools/Makefile
> > @@ -67,6 +67,9 @@ cpupower: FORCE
> >  cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
> >         $(call descend,$@)
> >
> > +bpf/%: FORCE
> > +       $(call descend,$@)
> > +
> >  liblockdep: FORCE
> >         $(call descend,lib/lockdep)
> >
> > diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
> > index f897eeeb0b4f..d4ea2b5a2e58 100644
> > --- a/tools/bpf/Makefile
> > +++ b/tools/bpf/Makefile
> > @@ -124,5 +124,8 @@ runqslower_install:
> >  runqslower_clean:
> >         $(call descend,runqslower,clean)
> >
> > +bpfwl:
> > +       $(call descend,bpfwl)
> > +
> >  .PHONY: all install clean bpftool bpftool_install bpftool_clean \
> > -       runqslower runqslower_install runqslower_clean
> > +       runqslower runqslower_install runqslower_clean bpfwl
> 
> what about install/clean subcommands? At least clean seems like a good idea?

not sure about install, does not seem necessary for this tool,
but I'll add propagation of clean (it's defined in bpfwl already)

thanks,
jirka
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 49b2709ff44e..b0537af523dc 100644
--- a/Makefile
+++ b/Makefile
@@ -1017,9 +1017,10 @@  export mod_sign_cmd
 
 HOST_LIBELF_LIBS = $(shell pkg-config libelf --libs 2>/dev/null || echo -lelf)
 
+has_libelf = $(call try-run,\
+               echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
+
 ifdef CONFIG_STACK_VALIDATION
-  has_libelf := $(call try-run,\
-		echo "int main() {}" | $(HOSTCC) -xc -o /dev/null $(HOST_LIBELF_LIBS) -,1,0)
   ifeq ($(has_libelf),1)
     objtool_target := tools/objtool FORCE
   else
@@ -1028,6 +1029,14 @@  ifdef CONFIG_STACK_VALIDATION
   endif
 endif
 
+ifdef CONFIG_DEBUG_INFO_BTF
+  ifeq ($(has_libelf),1)
+    bpfwl_target := tools/bpf/bpfwl FORCE
+  else
+    SKIP_BTF_WHITELIST_GENERATION := 1
+  endif
+endif
+
 PHONY += prepare0
 
 export MODORDER := $(extmod-prefix)modules.order
@@ -1141,7 +1150,7 @@  prepare0: archprepare
 	$(Q)$(MAKE) $(build)=.
 
 # All the preparing..
-prepare: prepare0 prepare-objtool
+prepare: prepare0 prepare-objtool prepare-bpfwl
 
 # Support for using generic headers in asm-generic
 asm-generic := -f $(srctree)/scripts/Makefile.asm-generic obj
@@ -1154,7 +1163,7 @@  uapi-asm-generic:
 	$(Q)$(MAKE) $(asm-generic)=arch/$(SRCARCH)/include/generated/uapi/asm \
 	generic=include/uapi/asm-generic
 
-PHONY += prepare-objtool
+PHONY += prepare-objtool prepare-bpfwl
 prepare-objtool: $(objtool_target)
 ifeq ($(SKIP_STACK_VALIDATION),1)
 ifdef CONFIG_UNWINDER_ORC
@@ -1165,6 +1174,10 @@  else
 endif
 endif
 
+prepare-bpfwl: $(bpfwl_target)
+ifeq ($(SKIP_BTF_WHITELIST_GENERATION),1)
+	@echo "warning: Cannot use BTF whitelist checks, please install libelf-dev, libelf-devel or elfutils-libelf-devel" >&2
+endif
 # Generate some files
 # ---------------------------------------------------------------------------
 
diff --git a/tools/Makefile b/tools/Makefile
index bd778812e915..85af6ebbce91 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -67,6 +67,9 @@  cpupower: FORCE
 cgroup firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging: FORCE
 	$(call descend,$@)
 
+bpf/%: FORCE
+	$(call descend,$@)
+
 liblockdep: FORCE
 	$(call descend,lib/lockdep)
 
diff --git a/tools/bpf/Makefile b/tools/bpf/Makefile
index f897eeeb0b4f..d4ea2b5a2e58 100644
--- a/tools/bpf/Makefile
+++ b/tools/bpf/Makefile
@@ -124,5 +124,8 @@  runqslower_install:
 runqslower_clean:
 	$(call descend,runqslower,clean)
 
+bpfwl:
+	$(call descend,bpfwl)
+
 .PHONY: all install clean bpftool bpftool_install bpftool_clean \
-	runqslower runqslower_install runqslower_clean
+	runqslower runqslower_install runqslower_clean bpfwl