Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF is enabled and pahole supports it

2020-11-16 Thread David Ahern 
On 11/9/20 6:19 PM, Andrii Nakryiko wrote:
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index d7a7bc3b6098..1e78faaf20a5 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -274,6 +274,15 @@ config DEBUG_INFO_BTF
> Turning this on expects presence of pahole tool, which will convert
> DWARF type info into equivalent deduplicated BTF type info.
>  
> +config PAHOLE_HAS_SPLIT_BTF
> + def_bool $(success, test `$(PAHOLE) --version | sed -E 
> 's/v([0-9]+)\.([0-9]+)/\1\2/'` -ge "119")
> +
> +config DEBUG_INFO_BTF_MODULES
> + def_bool y
> + depends on DEBUG_INFO_BTF && MODULES && PAHOLE_HAS_SPLIT_BTF
> + help
> +   Generate compact split BTF type information for kernel modules.
> +
>  config GDB_SCRIPTS
>   bool "Provide GDB scripts for kernel debugging"
>   help

Thank you for adding a config option for this feature vs bumping the
required pahole version in scripts/link-vmlinux.sh. This is a much more
friendly way of handling kernel features that require support from build
tools.

Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF is enabled and pahole supports it

2020-11-16 Thread Andrii Nakryiko 
On Mon, Nov 16, 2020 at 1:24 PM Jakub Kicinski  wrote:
>
> On Mon, 16 Nov 2020 12:34:17 -0800 Andrii Nakryiko wrote:
> > > This change, commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if 
> > > BTF is enabled and pahole
> > > supports it") currently in net-next, linux-next, etc. breaks the use-case 
> > > of compiling only a specific
> > > kernel module (both in-tree and out-of-tree, e.g. 'make 
> > > M=drivers/net/ethernet/intel/ice') after
> > > first doing a 'make modules_prepare'.  Previously, that use-case would 
> > > result in a warning noting
> > > "Symbol info of vmlinux is missing. Unresolved symbol check will be 
> > > entirely skipped" but now it
> > > errors out after noting "No rule to make target 'vmlinux', needed by 
> > > '<...>.ko'.  Stop."
> > >
> > > Is that intentional?
> >
> > I wasn't aware of such a use pattern, so definitely not intentional.
> > But vmlinux is absolutely necessary to generate the module BTF. So I'm
> > wondering what's the proper fix here? Leave it as is (that error
> > message is actually surprisingly descriptive, btw)? Force vmlinux
> > build? Or skip BTF generation for that module?
>
> For an external out-of-tree module there is no guarantee that vmlinux
> will even be on the system, no? So only the last option can work in
> that case.


Ok, how about something like the patch below. With that I seem to be
getting the desired behavior:

$ make clean
$ touch drivers/acpi/button.c
$ make M=drivers/acpi
make[1]: Entering directory `/data/users/andriin/linux-build/default-x86_64'
  CC [M]  drivers/acpi/button.o
  MODPOST drivers/acpi/Module.symvers
  LD [M]  drivers/acpi/button.ko
  BTF [M] drivers/acpi/button.ko
Skipping BTF generation for drivers/acpi/button.ko due to
unavailability of vmlinux
make[1]: Leaving directory `/data/users/andriin/linux-build/default-x86_64'
$ readelf -S ~/linux-build/default/drivers/acpi/button.ko | grep BTF -A1
... empty ...

Now with normal build:

$ make all
...
LD [M]  drivers/acpi/button.ko
BTF [M] drivers/acpi/button.ko
...
$ readelf -S ~/linux-build/default/drivers/acpi/button.ko | grep BTF -A1
  [60] .BTF  PROGBITS   00029310
   ab3f     0 0 1



diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 02b892421f7a..d49ec001825d 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -38,7 +38,12 @@ quiet_cmd_ld_ko_o = LD [M]  $@
 $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)

 quiet_cmd_btf_ko = BTF [M] $@
-  cmd_btf_ko = LLVM_OBJCOPY=$(OBJCOPY) $(PAHOLE) -J --btf_base vmlinux $@
+  cmd_btf_ko = \
+if [ -f vmlinux ]; then\
+LLVM_OBJCOPY=$(OBJCOPY) $(PAHOLE) -J --btf_base vmlinux $@; \
+else\
+printf "Skipping BTF generation for %s due to unavailability
of vmlinux\n" $@ 1>&2; \
+fi;

 # Same as newer-prereqs, but allows to exclude specified extra dependencies
 newer_prereqs_except = $(filter-out $(PHONY) $(1),$?)
@@ -49,7 +54,7 @@ if_changed_except = $(if $(call
newer_prereqs_except,$(2))$(cmd-check),  \
 printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)

 # Re-generate module BTFs if either module's .ko or vmlinux changed
-$(modules): %.ko: %.o %.mod.o scripts/module.lds vmlinux FORCE
+$(modules): %.ko: %.o %.mod.o scripts/module.lds $(if
$(KBUILD_BUILTIN),vmlinux) FORCE
 +$(call if_changed_except,ld_ko_o,vmlinux)
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 +$(if $(newer-prereqs),$(call cmd,btf_ko))

Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF is enabled and pahole supports it

2020-11-16 Thread Jakub Kicinski 
On Mon, 16 Nov 2020 12:34:17 -0800 Andrii Nakryiko wrote:
> > This change, commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF 
> > is enabled and pahole
> > supports it") currently in net-next, linux-next, etc. breaks the use-case 
> > of compiling only a specific
> > kernel module (both in-tree and out-of-tree, e.g. 'make 
> > M=drivers/net/ethernet/intel/ice') after
> > first doing a 'make modules_prepare'.  Previously, that use-case would 
> > result in a warning noting
> > "Symbol info of vmlinux is missing. Unresolved symbol check will be 
> > entirely skipped" but now it
> > errors out after noting "No rule to make target 'vmlinux', needed by 
> > '<...>.ko'.  Stop."
> >
> > Is that intentional?  
> 
> I wasn't aware of such a use pattern, so definitely not intentional.
> But vmlinux is absolutely necessary to generate the module BTF. So I'm
> wondering what's the proper fix here? Leave it as is (that error
> message is actually surprisingly descriptive, btw)? Force vmlinux
> build? Or skip BTF generation for that module?

For an external out-of-tree module there is no guarantee that vmlinux
will even be on the system, no? So only the last option can work in
that case.

Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF is enabled and pahole supports it

2020-11-16 Thread Andrii Nakryiko 
On Mon, Nov 16, 2020 at 11:55 AM Allan, Bruce W  wrote:
>
> > -Original Message-
> > From: Song Liu 
> > Sent: Tuesday, November 10, 2020 5:05 PM
> > To: Andrii Nakryiko 
> > Cc: bpf ; Networking ;
> > Starovoitov, Alexei ; Daniel Borkmann ;
> > Kernel Team ; open list  > ker...@vger.kernel.org>; raf...@kernel.org; j...@kernel.org; Arnaldo
> > Carvalho de Melo ; Greg Kroah-Hartman
> > ; Masahiro Yamada
> > 
> > Subject: Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if 
> > BTF is
> > enabled and pahole supports it
> >
> >
> >
> > > On Nov 9, 2020, at 5:19 PM, Andrii Nakryiko  wrote:
> >
> > [...]
> >
> > > SPLIT BTF
> > > =
> > >
> > > $ for f in $(find . -name '*.ko'); do size -A -d $f | grep BTF | awk 
> > > '{print $2}';
> > done | awk '{ s += $1 } END { print s }'
> > > 5194047
> > >
> > > $ for f in $(find . -name '*.ko'); do printf "%s %d\n" $f $(size -A -d $f 
> > > | grep
> > BTF | awk '{print $2}'); done | sort -nr -k2 | head -n10
> > > ./drivers/gpu/drm/i915/i915.ko 293206
> > > ./drivers/gpu/drm/radeon/radeon.ko 282103
> > > ./fs/xfs/xfs.ko 222150
> > > ./drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko 198503
> > > ./drivers/infiniband/hw/mlx5/mlx5_ib.ko 198356
> > > ./drivers/net/ethernet/broadcom/bnx2x/bnx2x.ko 113444
> > > ./fs/cifs/cifs.ko 109379
> > > ./arch/x86/kvm/kvm.ko 100225
> > > ./drivers/gpu/drm/drm.ko 94827
> > > ./drivers/infiniband/core/ib_core.ko 91188
> > >
> > > Cc: Masahiro Yamada 
> > > Signed-off-by: Andrii Nakryiko 
> >
> > Acked-by: Song Liu 
>
> This change, commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is 
> enabled and pahole
> supports it") currently in net-next, linux-next, etc. breaks the use-case of 
> compiling only a specific
> kernel module (both in-tree and out-of-tree, e.g. 'make 
> M=drivers/net/ethernet/intel/ice') after
> first doing a 'make modules_prepare'.  Previously, that use-case would result 
> in a warning noting
> "Symbol info of vmlinux is missing. Unresolved symbol check will be entirely 
> skipped" but now it
> errors out after noting "No rule to make target 'vmlinux', needed by 
> '<...>.ko'.  Stop."
>
> Is that intentional?

I wasn't aware of such a use pattern, so definitely not intentional.
But vmlinux is absolutely necessary to generate the module BTF. So I'm
wondering what's the proper fix here? Leave it as is (that error
message is actually surprisingly descriptive, btw)? Force vmlinux
build? Or skip BTF generation for that module?

>
> Thanks,
> Bruce.

RE: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF is enabled and pahole supports it

2020-11-16 Thread Allan, Bruce W 
> -Original Message-
> From: Song Liu 
> Sent: Tuesday, November 10, 2020 5:05 PM
> To: Andrii Nakryiko 
> Cc: bpf ; Networking ;
> Starovoitov, Alexei ; Daniel Borkmann ;
> Kernel Team ; open list  ker...@vger.kernel.org>; raf...@kernel.org; j...@kernel.org; Arnaldo
> Carvalho de Melo ; Greg Kroah-Hartman
> ; Masahiro Yamada
> 
> Subject: Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF 
> is
> enabled and pahole supports it
> 
> 
> 
> > On Nov 9, 2020, at 5:19 PM, Andrii Nakryiko  wrote:
> 
> [...]
> 
> > SPLIT BTF
> > =
> >
> > $ for f in $(find . -name '*.ko'); do size -A -d $f | grep BTF | awk 
> > '{print $2}';
> done | awk '{ s += $1 } END { print s }'
> > 5194047
> >
> > $ for f in $(find . -name '*.ko'); do printf "%s %d\n" $f $(size -A -d $f | 
> > grep
> BTF | awk '{print $2}'); done | sort -nr -k2 | head -n10
> > ./drivers/gpu/drm/i915/i915.ko 293206
> > ./drivers/gpu/drm/radeon/radeon.ko 282103
> > ./fs/xfs/xfs.ko 222150
> > ./drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko 198503
> > ./drivers/infiniband/hw/mlx5/mlx5_ib.ko 198356
> > ./drivers/net/ethernet/broadcom/bnx2x/bnx2x.ko 113444
> > ./fs/cifs/cifs.ko 109379
> > ./arch/x86/kvm/kvm.ko 100225
> > ./drivers/gpu/drm/drm.ko 94827
> > ./drivers/infiniband/core/ib_core.ko 91188
> >
> > Cc: Masahiro Yamada 
> > Signed-off-by: Andrii Nakryiko 
> 
> Acked-by: Song Liu 

This change, commit 5f9ae91f7c0d ("kbuild: Build kernel module BTFs if BTF is 
enabled and pahole
supports it") currently in net-next, linux-next, etc. breaks the use-case of 
compiling only a specific
kernel module (both in-tree and out-of-tree, e.g. 'make 
M=drivers/net/ethernet/intel/ice') after
first doing a 'make modules_prepare'.  Previously, that use-case would result 
in a warning noting
"Symbol info of vmlinux is missing. Unresolved symbol check will be entirely 
skipped" but now it
errors out after noting "No rule to make target 'vmlinux', needed by 
'<...>.ko'.  Stop."

Is that intentional?

Thanks,
Bruce.

Re: [PATCH v4 bpf-next 3/5] kbuild: build kernel module BTFs if BTF is enabled and pahole supports it

2020-11-10 Thread Song Liu 



> On Nov 9, 2020, at 5:19 PM, Andrii Nakryiko  wrote:

[...]

> SPLIT BTF
> =
> 
> $ for f in $(find . -name '*.ko'); do size -A -d $f | grep BTF | awk '{print 
> $2}'; done | awk '{ s += $1 } END { print s }'
> 5194047
> 
> $ for f in $(find . -name '*.ko'); do printf "%s %d\n" $f $(size -A -d $f | 
> grep BTF | awk '{print $2}'); done | sort -nr -k2 | head -n10
> ./drivers/gpu/drm/i915/i915.ko 293206
> ./drivers/gpu/drm/radeon/radeon.ko 282103
> ./fs/xfs/xfs.ko 222150
> ./drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.ko 198503
> ./drivers/infiniband/hw/mlx5/mlx5_ib.ko 198356
> ./drivers/net/ethernet/broadcom/bnx2x/bnx2x.ko 113444
> ./fs/cifs/cifs.ko 109379
> ./arch/x86/kvm/kvm.ko 100225
> ./drivers/gpu/drm/drm.ko 94827
> ./drivers/infiniband/core/ib_core.ko 91188
> 
> Cc: Masahiro Yamada 
> Signed-off-by: Andrii Nakryiko 

Acked-by: Song Liu