* [bpf-next PATCH v2 1/4] kbuild: remove ARCH_POSTLINK from module builds @ 2023-10-18 15:19 Masahiro Yamada 2023-10-18 15:19 ` [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Masahiro Yamada @ 2023-10-18 15:19 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, Albert Ou, Borislav Petkov, Christophe Leroy, Dave Hansen, H. Peter Anvin, Ingo Molnar, Michael Ellerman, Nathan Chancellor, Nicholas Piggin, Nick Desaulniers, Nicolas Schier, Palmer Dabbelt, Paul Walmsley, Thomas Bogendoerfer, Thomas Gleixner, bpf, linux-mips, linux-riscv, linuxppc-dev, x86 The '%.ko' rule in arch/*/Makefile.postlink does nothing but call the 'true' command. Remove the meaningless code. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <n.schier@avm.de> --- (no changes since v1) arch/mips/Makefile.postlink | 3 --- arch/powerpc/Makefile.postlink | 3 --- arch/riscv/Makefile.postlink | 3 --- arch/x86/Makefile.postlink | 3 --- scripts/Makefile.modfinal | 5 +---- 5 files changed, 1 insertion(+), 16 deletions(-) diff --git a/arch/mips/Makefile.postlink b/arch/mips/Makefile.postlink index 34e3bd71f3b0..6cfdc149d3bc 100644 --- a/arch/mips/Makefile.postlink +++ b/arch/mips/Makefile.postlink @@ -31,9 +31,6 @@ ifeq ($(CONFIG_RELOCATABLE),y) $(call if_changed,relocs) endif -%.ko: FORCE - @true - clean: @true diff --git a/arch/powerpc/Makefile.postlink b/arch/powerpc/Makefile.postlink index 1f860b3c9bec..ae5a4256b03d 100644 --- a/arch/powerpc/Makefile.postlink +++ b/arch/powerpc/Makefile.postlink @@ -35,9 +35,6 @@ ifdef CONFIG_RELOCATABLE $(call if_changed,relocs_check) endif -%.ko: FORCE - @true - clean: rm -f .tmp_symbols.txt diff --git a/arch/riscv/Makefile.postlink b/arch/riscv/Makefile.postlink index a46fc578b30b..829b9abc91f6 100644 --- a/arch/riscv/Makefile.postlink +++ b/arch/riscv/Makefile.postlink @@ -36,9 +36,6 @@ ifdef CONFIG_RELOCATABLE $(call if_changed,relocs_strip) endif -%.ko: FORCE - @true - clean: @true diff --git a/arch/x86/Makefile.postlink b/arch/x86/Makefile.postlink index 936093d29160..fef2e977cc7d 100644 --- a/arch/x86/Makefile.postlink +++ b/arch/x86/Makefile.postlink @@ -34,9 +34,6 @@ ifeq ($(CONFIG_X86_NEED_RELOCS),y) $(call cmd,strip_relocs) endif -%.ko: FORCE - @true - clean: @rm -f $(OUT_RELOCS)/vmlinux.relocs diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index b3a6aa8fbe8c..8568d256d6fb 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -28,14 +28,11 @@ quiet_cmd_cc_o_c = CC [M] $@ %.mod.o: %.mod.c FORCE $(call if_changed_dep,cc_o_c) -ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink) - quiet_cmd_ld_ko_o = LD [M] $@ cmd_ld_ko_o += \ $(LD) -r $(KBUILD_LDFLAGS) \ $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ - -T scripts/module.lds -o $@ $(filter %.o, $^); \ - $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) + -T scripts/module.lds -o $@ $(filter %.o, $^) quiet_cmd_btf_ko = BTF [M] $@ cmd_btf_ko = \ -- 2.40.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh 2023-10-18 15:19 [bpf-next PATCH v2 1/4] kbuild: remove ARCH_POSTLINK from module builds Masahiro Yamada @ 2023-10-18 15:19 ` Masahiro Yamada 2023-10-19 8:47 ` Jiri Olsa 2023-10-23 0:33 ` Martin Rodriguez Reboredo 2023-10-18 15:19 ` [bpf-next PATCH v2 3/4] kbuild: skip module BTF with one-time check for vmlinux Masahiro Yamada 2023-10-18 15:19 ` [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Masahiro Yamada 2 siblings, 2 replies; 19+ messages in thread From: Masahiro Yamada @ 2023-10-18 15:19 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Alan Maguire, Nicolas Schier, Miguel Ojeda, Alex Gaynor, Alexei Starovoitov, Alice Ryhl, Andreas Hindborg, Andrii Nakryiko, Benno Lossin, Björn Roy Baron, Boqun Feng, Daniel Borkmann, Gary Guo, Hao Luo, Jiri Olsa, John Fastabend, KP Singh, Martin KaFai Lau, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song, bpf, rust-for-linux scripts/pahole-flags.sh is executed so many times. You can check how many times it is invoked during the build, as follows: $ cat <<EOF >> scripts/pahole-flags.sh > echo "scripts/pahole-flags.sh was executed" >&2 > EOF $ make -s scripts/pahole-flags.sh was executed scripts/pahole-flags.sh was executed scripts/pahole-flags.sh was executed scripts/pahole-flags.sh was executed scripts/pahole-flags.sh was executed [ lots of repeated lines suppressed... ] This scripts is executed more than 20 times during the kernel build because PAHOLE_FLAGS is a recursively expanded variable and exported to sub-processes. With the GNU Make >= 4.4, it is executed more than 60 times because exported variables are also passed to other $(shell ) invocations. Without careful coding, it is known to cause an exponential fork explosion. [1] The use of $(shell ) in an exported recursive variable is likely wrong because $(shell ) is always evaluated due to the 'export' keyword, and the evaluation can occur multiple times by the nature of recursive variables. Convert the shell script to a Makefile, which is included only when CONFIG_DEBUG_INFO_BTF=y. [1]: https://savannah.gnu.org/bugs/index.php?64746 Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Alan Maguire <alan.maguire@oracle.com> Tested-by: Alan Maguire <alan.maguire@oracle.com> Reviewed-by: Nicolas Schier <n.schier@avm.de> Tested-by: Miguel Ojeda <ojeda@kernel.org> Acked-by: Miguel Ojeda <ojeda@kernel.org> --- Changes in v2: - Fix a typo in commit description - Update MAINTAINERS MAINTAINERS | 2 +- Makefile | 4 +--- scripts/Makefile.btf | 19 +++++++++++++++++++ scripts/pahole-flags.sh | 30 ------------------------------ 4 files changed, 21 insertions(+), 34 deletions(-) create mode 100644 scripts/Makefile.btf delete mode 100755 scripts/pahole-flags.sh diff --git a/MAINTAINERS b/MAINTAINERS index 35977b269d5e..a08d558b1aaa 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3742,7 +3742,7 @@ F: net/sched/act_bpf.c F: net/sched/cls_bpf.c F: samples/bpf/ F: scripts/bpf_doc.py -F: scripts/pahole-flags.sh +F: scripts/Makefile.btf F: scripts/pahole-version.sh F: tools/bpf/ F: tools/lib/bpf/ diff --git a/Makefile b/Makefile index fed9a6cc3665..eaddec67e5e1 100644 --- a/Makefile +++ b/Makefile @@ -513,8 +513,6 @@ LZ4 = lz4c XZ = xz ZSTD = zstd -PAHOLE_FLAGS = $(shell PAHOLE=$(PAHOLE) $(srctree)/scripts/pahole-flags.sh) - CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ -Wbitwise -Wno-return-void -Wno-unknown-attribute $(CF) NOSTDINC_FLAGS := @@ -605,7 +603,6 @@ export KBUILD_RUSTFLAGS RUSTFLAGS_KERNEL RUSTFLAGS_MODULE export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_RUSTFLAGS_MODULE KBUILD_LDFLAGS_MODULE export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL KBUILD_RUSTFLAGS_KERNEL -export PAHOLE_FLAGS # Files to ignore in find ... statements @@ -1002,6 +999,7 @@ KBUILD_CPPFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) # include additional Makefiles when needed include-y := scripts/Makefile.extrawarn include-$(CONFIG_DEBUG_INFO) += scripts/Makefile.debug +include-$(CONFIG_DEBUG_INFO_BTF)+= scripts/Makefile.btf include-$(CONFIG_KASAN) += scripts/Makefile.kasan include-$(CONFIG_KCSAN) += scripts/Makefile.kcsan include-$(CONFIG_KMSAN) += scripts/Makefile.kmsan diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf new file mode 100644 index 000000000000..82377e470aed --- /dev/null +++ b/scripts/Makefile.btf @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0 + +pahole-ver := $(CONFIG_PAHOLE_VERSION) +pahole-flags-y := + +# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars +ifeq ($(call test-le, $(pahole-ver), 121),y) +pahole-flags-$(call test-ge, $(pahole-ver), 118) += --skip_encoding_btf_vars +endif + +pahole-flags-$(call test-ge, $(pahole-ver), 121) += --btf_gen_floats + +pahole-flags-$(call test-ge, $(pahole-ver), 122) += -j + +pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust + +pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized + +export PAHOLE_FLAGS := $(pahole-flags-y) diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh deleted file mode 100755 index 728d55190d97..000000000000 --- a/scripts/pahole-flags.sh +++ /dev/null @@ -1,30 +0,0 @@ -#!/bin/sh -# SPDX-License-Identifier: GPL-2.0 - -extra_paholeopt= - -if ! [ -x "$(command -v ${PAHOLE})" ]; then - exit 0 -fi - -pahole_ver=$($(dirname $0)/pahole-version.sh ${PAHOLE}) - -if [ "${pahole_ver}" -ge "118" ] && [ "${pahole_ver}" -le "121" ]; then - # pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars - extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_vars" -fi -if [ "${pahole_ver}" -ge "121" ]; then - extra_paholeopt="${extra_paholeopt} --btf_gen_floats" -fi -if [ "${pahole_ver}" -ge "122" ]; then - extra_paholeopt="${extra_paholeopt} -j" -fi -if [ "${pahole_ver}" -ge "124" ]; then - # see PAHOLE_HAS_LANG_EXCLUDE - extra_paholeopt="${extra_paholeopt} --lang_exclude=rust" -fi -if [ "${pahole_ver}" -ge "125" ]; then - extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_inconsistent_proto --btf_gen_optimized" -fi - -echo ${extra_paholeopt} -- 2.40.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh 2023-10-18 15:19 ` [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada @ 2023-10-19 8:47 ` Jiri Olsa 2023-10-23 0:33 ` Martin Rodriguez Reboredo 1 sibling, 0 replies; 19+ messages in thread From: Jiri Olsa @ 2023-10-19 8:47 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Alan Maguire, Nicolas Schier, Miguel Ojeda, Alex Gaynor, Alexei Starovoitov, Alice Ryhl, Andreas Hindborg, Andrii Nakryiko, Benno Lossin, Björn Roy Baron, Boqun Feng, Daniel Borkmann, Gary Guo, Hao Luo, John Fastabend, KP Singh, Martin KaFai Lau, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song, bpf, rust-for-linux On Thu, Oct 19, 2023 at 12:19:48AM +0900, Masahiro Yamada wrote: > scripts/pahole-flags.sh is executed so many times. > > You can check how many times it is invoked during the build, as follows: > > $ cat <<EOF >> scripts/pahole-flags.sh > > echo "scripts/pahole-flags.sh was executed" >&2 > > EOF > > $ make -s > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > [ lots of repeated lines suppressed... ] > > This scripts is executed more than 20 times during the kernel build > because PAHOLE_FLAGS is a recursively expanded variable and exported > to sub-processes. > > With the GNU Make >= 4.4, it is executed more than 60 times because > exported variables are also passed to other $(shell ) invocations. > Without careful coding, it is known to cause an exponential fork > explosion. [1] > > The use of $(shell ) in an exported recursive variable is likely wrong > because $(shell ) is always evaluated due to the 'export' keyword, and > the evaluation can occur multiple times by the nature of recursive > variables. > > Convert the shell script to a Makefile, which is included only when > CONFIG_DEBUG_INFO_BTF=y. > > [1]: https://savannah.gnu.org/bugs/index.php?64746 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com> > Tested-by: Alan Maguire <alan.maguire@oracle.com> > Reviewed-by: Nicolas Schier <n.schier@avm.de> > Tested-by: Miguel Ojeda <ojeda@kernel.org> > Acked-by: Miguel Ojeda <ojeda@kernel.org> nice! Acked-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > --- > > Changes in v2: > - Fix a typo in commit description > - Update MAINTAINERS > > MAINTAINERS | 2 +- > Makefile | 4 +--- > scripts/Makefile.btf | 19 +++++++++++++++++++ > scripts/pahole-flags.sh | 30 ------------------------------ > 4 files changed, 21 insertions(+), 34 deletions(-) > create mode 100644 scripts/Makefile.btf > delete mode 100755 scripts/pahole-flags.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index 35977b269d5e..a08d558b1aaa 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3742,7 +3742,7 @@ F: net/sched/act_bpf.c > F: net/sched/cls_bpf.c > F: samples/bpf/ > F: scripts/bpf_doc.py > -F: scripts/pahole-flags.sh > +F: scripts/Makefile.btf > F: scripts/pahole-version.sh > F: tools/bpf/ > F: tools/lib/bpf/ > diff --git a/Makefile b/Makefile > index fed9a6cc3665..eaddec67e5e1 100644 > --- a/Makefile > +++ b/Makefile > @@ -513,8 +513,6 @@ LZ4 = lz4c > XZ = xz > ZSTD = zstd > > -PAHOLE_FLAGS = $(shell PAHOLE=$(PAHOLE) $(srctree)/scripts/pahole-flags.sh) > - > CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ > -Wbitwise -Wno-return-void -Wno-unknown-attribute $(CF) > NOSTDINC_FLAGS := > @@ -605,7 +603,6 @@ export KBUILD_RUSTFLAGS RUSTFLAGS_KERNEL RUSTFLAGS_MODULE > export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE > export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_RUSTFLAGS_MODULE KBUILD_LDFLAGS_MODULE > export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL KBUILD_RUSTFLAGS_KERNEL > -export PAHOLE_FLAGS > > # Files to ignore in find ... statements > > @@ -1002,6 +999,7 @@ KBUILD_CPPFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) > # include additional Makefiles when needed > include-y := scripts/Makefile.extrawarn > include-$(CONFIG_DEBUG_INFO) += scripts/Makefile.debug > +include-$(CONFIG_DEBUG_INFO_BTF)+= scripts/Makefile.btf > include-$(CONFIG_KASAN) += scripts/Makefile.kasan > include-$(CONFIG_KCSAN) += scripts/Makefile.kcsan > include-$(CONFIG_KMSAN) += scripts/Makefile.kmsan > diff --git a/scripts/Makefile.btf b/scripts/Makefile.btf > new file mode 100644 > index 000000000000..82377e470aed > --- /dev/null > +++ b/scripts/Makefile.btf > @@ -0,0 +1,19 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +pahole-ver := $(CONFIG_PAHOLE_VERSION) > +pahole-flags-y := > + > +# pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars > +ifeq ($(call test-le, $(pahole-ver), 121),y) > +pahole-flags-$(call test-ge, $(pahole-ver), 118) += --skip_encoding_btf_vars > +endif > + > +pahole-flags-$(call test-ge, $(pahole-ver), 121) += --btf_gen_floats > + > +pahole-flags-$(call test-ge, $(pahole-ver), 122) += -j > + > +pahole-flags-$(CONFIG_PAHOLE_HAS_LANG_EXCLUDE) += --lang_exclude=rust > + > +pahole-flags-$(call test-ge, $(pahole-ver), 125) += --skip_encoding_btf_inconsistent_proto --btf_gen_optimized > + > +export PAHOLE_FLAGS := $(pahole-flags-y) > diff --git a/scripts/pahole-flags.sh b/scripts/pahole-flags.sh > deleted file mode 100755 > index 728d55190d97..000000000000 > --- a/scripts/pahole-flags.sh > +++ /dev/null > @@ -1,30 +0,0 @@ > -#!/bin/sh > -# SPDX-License-Identifier: GPL-2.0 > - > -extra_paholeopt= > - > -if ! [ -x "$(command -v ${PAHOLE})" ]; then > - exit 0 > -fi > - > -pahole_ver=$($(dirname $0)/pahole-version.sh ${PAHOLE}) > - > -if [ "${pahole_ver}" -ge "118" ] && [ "${pahole_ver}" -le "121" ]; then > - # pahole 1.18 through 1.21 can't handle zero-sized per-CPU vars > - extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_vars" > -fi > -if [ "${pahole_ver}" -ge "121" ]; then > - extra_paholeopt="${extra_paholeopt} --btf_gen_floats" > -fi > -if [ "${pahole_ver}" -ge "122" ]; then > - extra_paholeopt="${extra_paholeopt} -j" > -fi > -if [ "${pahole_ver}" -ge "124" ]; then > - # see PAHOLE_HAS_LANG_EXCLUDE > - extra_paholeopt="${extra_paholeopt} --lang_exclude=rust" > -fi > -if [ "${pahole_ver}" -ge "125" ]; then > - extra_paholeopt="${extra_paholeopt} --skip_encoding_btf_inconsistent_proto --btf_gen_optimized" > -fi > - > -echo ${extra_paholeopt} > -- > 2.40.1 > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh 2023-10-18 15:19 ` [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada 2023-10-19 8:47 ` Jiri Olsa @ 2023-10-23 0:33 ` Martin Rodriguez Reboredo 1 sibling, 0 replies; 19+ messages in thread From: Martin Rodriguez Reboredo @ 2023-10-23 0:33 UTC (permalink / raw) To: Masahiro Yamada, linux-kbuild Cc: linux-kernel, Alan Maguire, Nicolas Schier, Miguel Ojeda, Alex Gaynor, Alexei Starovoitov, Alice Ryhl, Andreas Hindborg, Andrii Nakryiko, Benno Lossin, Björn Roy Baron, Boqun Feng, Daniel Borkmann, Gary Guo, Hao Luo, Jiri Olsa, John Fastabend, KP Singh, Martin KaFai Lau, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song, bpf, rust-for-linux On 10/18/23 12:19, Masahiro Yamada wrote: > scripts/pahole-flags.sh is executed so many times. > > You can check how many times it is invoked during the build, as follows: > > $ cat <<EOF >> scripts/pahole-flags.sh > > echo "scripts/pahole-flags.sh was executed" >&2 > > EOF > > $ make -s > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > scripts/pahole-flags.sh was executed > [ lots of repeated lines suppressed... ] > > This scripts is executed more than 20 times during the kernel build > because PAHOLE_FLAGS is a recursively expanded variable and exported > to sub-processes. > > With the GNU Make >= 4.4, it is executed more than 60 times because > exported variables are also passed to other $(shell ) invocations. > Without careful coding, it is known to cause an exponential fork > explosion. [1] > > The use of $(shell ) in an exported recursive variable is likely wrong > because $(shell ) is always evaluated due to the 'export' keyword, and > the evaluation can occur multiple times by the nature of recursive > variables. > > Convert the shell script to a Makefile, which is included only when > CONFIG_DEBUG_INFO_BTF=y. > > [1]: https://savannah.gnu.org/bugs/index.php?64746 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Alan Maguire <alan.maguire@oracle.com> > Tested-by: Alan Maguire <alan.maguire@oracle.com> > Reviewed-by: Nicolas Schier <n.schier@avm.de> > Tested-by: Miguel Ojeda <ojeda@kernel.org> > Acked-by: Miguel Ojeda <ojeda@kernel.org> > --- > [...] > @@ -1002,6 +999,7 @@ KBUILD_CPPFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=) > # include additional Makefiles when needed > include-y := scripts/Makefile.extrawarn > include-$(CONFIG_DEBUG_INFO) += scripts/Makefile.debug > +include-$(CONFIG_DEBUG_INFO_BTF)+= scripts/Makefile.btf Would've used a tab, for legibility sake. > include-$(CONFIG_KASAN) += scripts/Makefile.kasan > include-$(CONFIG_KCSAN) += scripts/Makefile.kcsan > include-$(CONFIG_KMSAN) += scripts/Makefile.kmsan > [...] Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com> ^ permalink raw reply [flat|nested] 19+ messages in thread
* [bpf-next PATCH v2 3/4] kbuild: skip module BTF with one-time check for vmlinux 2023-10-18 15:19 [bpf-next PATCH v2 1/4] kbuild: remove ARCH_POSTLINK from module builds Masahiro Yamada 2023-10-18 15:19 ` [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada @ 2023-10-18 15:19 ` Masahiro Yamada 2023-10-19 8:17 ` Jiri Olsa 2023-10-18 15:19 ` [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Masahiro Yamada 2 siblings, 1 reply; 19+ messages in thread From: Masahiro Yamada @ 2023-10-18 15:19 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf When CONFIG_DEBUG_INFO_BTF_MODULES is enabled, vmlinux presence is checked in every module build, resulting in repetitive warning messages if vmlinux is missing. Check vmlinux and print a warning just once. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> Reviewed-by: Nicolas Schier <n.schier@avm.de> --- (no changes since v1) scripts/Makefile.modfinal | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 8568d256d6fb..9fd7a26e4fe9 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -14,6 +14,15 @@ include $(srctree)/scripts/Makefile.lib # find all modules listed in modules.order modules := $(call read-file, $(MODORDER)) +vmlinux := + +ifdef CONFIG_DEBUG_INFO_BTF_MODULES +ifneq ($(wildcard vmlinux),) +vmlinux := vmlinux +else +$(warning Skipping BTF generation due to unavailability of vmlinux) +endif +endif __modfinal: $(modules:%.o=%.ko) @: @@ -36,12 +45,8 @@ quiet_cmd_ld_ko_o = LD [M] $@ quiet_cmd_btf_ko = BTF [M] $@ cmd_btf_ko = \ - if [ ! -f vmlinux ]; then \ - printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ - else \ LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ - $(RESOLVE_BTFIDS) -b vmlinux $@; \ - fi; + $(RESOLVE_BTFIDS) -b vmlinux $@ # Same as newer-prereqs, but allows to exclude specified extra dependencies newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) @@ -52,9 +57,9 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) # Re-generate module BTFs if either module's .ko or vmlinux changed -%.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE +%.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE +$(call if_changed_except,ld_ko_o,vmlinux) -ifdef CONFIG_DEBUG_INFO_BTF_MODULES +ifdef vmlinux +$(if $(newer-prereqs),$(call cmd,btf_ko)) endif -- 2.40.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 3/4] kbuild: skip module BTF with one-time check for vmlinux 2023-10-18 15:19 ` [bpf-next PATCH v2 3/4] kbuild: skip module BTF with one-time check for vmlinux Masahiro Yamada @ 2023-10-19 8:17 ` Jiri Olsa 0 siblings, 0 replies; 19+ messages in thread From: Jiri Olsa @ 2023-10-19 8:17 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nicolas Schier, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Thu, Oct 19, 2023 at 12:19:49AM +0900, Masahiro Yamada wrote: > When CONFIG_DEBUG_INFO_BTF_MODULES is enabled, vmlinux presence is > checked in every module build, resulting in repetitive warning > messages if vmlinux is missing. > > Check vmlinux and print a warning just once. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > Reviewed-by: Nicolas Schier <n.schier@avm.de> lgtm Acked-by: Jiri Olsa <jolsa@kernel.org> jirka > --- > > (no changes since v1) > > scripts/Makefile.modfinal | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > index 8568d256d6fb..9fd7a26e4fe9 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -14,6 +14,15 @@ include $(srctree)/scripts/Makefile.lib > > # find all modules listed in modules.order > modules := $(call read-file, $(MODORDER)) > +vmlinux := > + > +ifdef CONFIG_DEBUG_INFO_BTF_MODULES > +ifneq ($(wildcard vmlinux),) > +vmlinux := vmlinux > +else > +$(warning Skipping BTF generation due to unavailability of vmlinux) > +endif > +endif > > __modfinal: $(modules:%.o=%.ko) > @: > @@ -36,12 +45,8 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > quiet_cmd_btf_ko = BTF [M] $@ > cmd_btf_ko = \ > - if [ ! -f vmlinux ]; then \ > - printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \ > - else \ > LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > - $(RESOLVE_BTFIDS) -b vmlinux $@; \ > - fi; > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > # Same as newer-prereqs, but allows to exclude specified extra dependencies > newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > @@ -52,9 +57,9 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > > # Re-generate module BTFs if either module's .ko or vmlinux changed > -%.ko: %.o %.mod.o scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),vmlinux) FORCE > +%.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > +$(call if_changed_except,ld_ko_o,vmlinux) > -ifdef CONFIG_DEBUG_INFO_BTF_MODULES > +ifdef vmlinux > +$(if $(newer-prereqs),$(call cmd,btf_ko)) > endif > > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-18 15:19 [bpf-next PATCH v2 1/4] kbuild: remove ARCH_POSTLINK from module builds Masahiro Yamada 2023-10-18 15:19 ` [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada 2023-10-18 15:19 ` [bpf-next PATCH v2 3/4] kbuild: skip module BTF with one-time check for vmlinux Masahiro Yamada @ 2023-10-18 15:19 ` Masahiro Yamada 2023-10-18 15:49 ` Nicolas Schier 2023-10-19 8:15 ` Jiri Olsa 2 siblings, 2 replies; 19+ messages in thread From: Masahiro Yamada @ 2023-10-18 15:19 UTC (permalink / raw) To: linux-kbuild Cc: linux-kernel, Masahiro Yamada, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf newer_prereqs_except and if_changed_except are ugly hacks of the newer-prereqs and if_changed in scripts/Kbuild.include. Remove. Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- Changes in v2: - Fix if_changed_except to if_changed scripts/Makefile.modfinal | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal index 9fd7a26e4fe9..fc07854bb7b9 100644 --- a/scripts/Makefile.modfinal +++ b/scripts/Makefile.modfinal @@ -19,6 +19,9 @@ vmlinux := ifdef CONFIG_DEBUG_INFO_BTF_MODULES ifneq ($(wildcard vmlinux),) vmlinux := vmlinux +cmd_btf = ; \ + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ + $(RESOLVE_BTFIDS) -b vmlinux $@ else $(warning Skipping BTF generation due to unavailability of vmlinux) endif @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ cmd_ld_ko_o += \ $(LD) -r $(KBUILD_LDFLAGS) \ $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ - -T scripts/module.lds -o $@ $(filter %.o, $^) + -T scripts/module.lds -o $@ $(filter %.o, $^) \ + $(cmd_btf) -quiet_cmd_btf_ko = BTF [M] $@ - cmd_btf_ko = \ - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ - $(RESOLVE_BTFIDS) -b vmlinux $@ - -# Same as newer-prereqs, but allows to exclude specified extra dependencies -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) - -# Same as if_changed, but allows to exclude specified extra dependencies -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ - $(cmd); \ - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) - -# Re-generate module BTFs if either module's .ko or vmlinux changed %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE - +$(call if_changed_except,ld_ko_o,vmlinux) -ifdef vmlinux - +$(if $(newer-prereqs),$(call cmd,btf_ko)) -endif + +$(call if_changed,ld_ko_o) targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) -- 2.40.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-18 15:19 ` [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Masahiro Yamada @ 2023-10-18 15:49 ` Nicolas Schier 2023-10-19 8:15 ` Jiri Olsa 1 sibling, 0 replies; 19+ messages in thread From: Nicolas Schier @ 2023-10-18 15:49 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, bpf On Thu, Oct 19, 2023 at 12:19:50AM +0900 Masahiro Yamada wrote: > newer_prereqs_except and if_changed_except are ugly hacks of the > newer-prereqs and if_changed in scripts/Kbuild.include. > > Remove. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Changes in v2: > - Fix if_changed_except to if_changed thanks Reviewed-by: Nicolas Schier <nicolas@fjasle.eu> > scripts/Makefile.modfinal | 25 ++++++------------------- > 1 file changed, 6 insertions(+), 19 deletions(-) > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > index 9fd7a26e4fe9..fc07854bb7b9 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -19,6 +19,9 @@ vmlinux := > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > ifneq ($(wildcard vmlinux),) > vmlinux := vmlinux > +cmd_btf = ; \ > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > + $(RESOLVE_BTFIDS) -b vmlinux $@ > else > $(warning Skipping BTF generation due to unavailability of vmlinux) > endif > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > cmd_ld_ko_o += \ > $(LD) -r $(KBUILD_LDFLAGS) \ > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > - -T scripts/module.lds -o $@ $(filter %.o, $^) > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > + $(cmd_btf) > > -quiet_cmd_btf_ko = BTF [M] $@ > - cmd_btf_ko = \ > - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > - $(RESOLVE_BTFIDS) -b vmlinux $@ > - > -# Same as newer-prereqs, but allows to exclude specified extra dependencies > -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > - > -# Same as if_changed, but allows to exclude specified extra dependencies > -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > - $(cmd); \ > - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > - > -# Re-generate module BTFs if either module's .ko or vmlinux changed > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > - +$(call if_changed_except,ld_ko_o,vmlinux) > -ifdef vmlinux > - +$(if $(newer-prereqs),$(call cmd,btf_ko)) > -endif > + +$(call if_changed,ld_ko_o) > > targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) > > -- > 2.40.1 -- epost|xmpp: nicolas@fjasle.eu irc://oftc.net/nsc ↳ gpg: 18ed 52db e34f 860e e9fb c82b 7d97 0932 55a0 ce7f -- frykten for herren er opphav til kunnskap -- ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-18 15:19 ` [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Masahiro Yamada 2023-10-18 15:49 ` Nicolas Schier @ 2023-10-19 8:15 ` Jiri Olsa 2023-10-19 22:54 ` Andrii Nakryiko 1 sibling, 1 reply; 19+ messages in thread From: Jiri Olsa @ 2023-10-19 8:15 UTC (permalink / raw) To: Masahiro Yamada Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > newer_prereqs_except and if_changed_except are ugly hacks of the > newer-prereqs and if_changed in scripts/Kbuild.include. > > Remove. > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Changes in v2: > - Fix if_changed_except to if_changed > > scripts/Makefile.modfinal | 25 ++++++------------------- > 1 file changed, 6 insertions(+), 19 deletions(-) > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > index 9fd7a26e4fe9..fc07854bb7b9 100644 > --- a/scripts/Makefile.modfinal > +++ b/scripts/Makefile.modfinal > @@ -19,6 +19,9 @@ vmlinux := > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > ifneq ($(wildcard vmlinux),) > vmlinux := vmlinux > +cmd_btf = ; \ > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > + $(RESOLVE_BTFIDS) -b vmlinux $@ > else > $(warning Skipping BTF generation due to unavailability of vmlinux) > endif > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > cmd_ld_ko_o += \ > $(LD) -r $(KBUILD_LDFLAGS) \ > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > - -T scripts/module.lds -o $@ $(filter %.o, $^) > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > + $(cmd_btf) > > -quiet_cmd_btf_ko = BTF [M] $@ nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, I don't mind not displaying that, but we should mention that in changelog jirka > - cmd_btf_ko = \ > - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > - $(RESOLVE_BTFIDS) -b vmlinux $@ > - > -# Same as newer-prereqs, but allows to exclude specified extra dependencies > -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > - > -# Same as if_changed, but allows to exclude specified extra dependencies > -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > - $(cmd); \ > - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > - > -# Re-generate module BTFs if either module's .ko or vmlinux changed > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > - +$(call if_changed_except,ld_ko_o,vmlinux) > -ifdef vmlinux > - +$(if $(newer-prereqs),$(call cmd,btf_ko)) > -endif > + +$(call if_changed,ld_ko_o) > > targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) > > -- > 2.40.1 > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-19 8:15 ` Jiri Olsa @ 2023-10-19 22:54 ` Andrii Nakryiko 2023-10-20 7:03 ` Masahiro Yamada 0 siblings, 1 reply; 19+ messages in thread From: Andrii Nakryiko @ 2023-10-19 22:54 UTC (permalink / raw) To: Jiri Olsa Cc: Masahiro Yamada, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > newer_prereqs_except and if_changed_except are ugly hacks of the > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > Remove. > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > Changes in v2: > > - Fix if_changed_except to if_changed > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > --- a/scripts/Makefile.modfinal > > +++ b/scripts/Makefile.modfinal > > @@ -19,6 +19,9 @@ vmlinux := > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > ifneq ($(wildcard vmlinux),) > > vmlinux := vmlinux > > +cmd_btf = ; \ > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > else > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > endif > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > cmd_ld_ko_o += \ > > $(LD) -r $(KBUILD_LDFLAGS) \ > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > + $(cmd_btf) > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > I don't mind not displaying that, but we should mention that in changelog > Thanks for spotting this! I think those messages are useful and important to keep. Masahiro, is it possible to preserve them? > jirka > > > - cmd_btf_ko = \ > > - LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > - $(RESOLVE_BTFIDS) -b vmlinux $@ > > - > > -# Same as newer-prereqs, but allows to exclude specified extra dependencies > > -newer_prereqs_except = $(filter-out $(PHONY) $(1),$?) > > - > > -# Same as if_changed, but allows to exclude specified extra dependencies > > -if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check), \ > > - $(cmd); \ > > - printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:) > > - > > -# Re-generate module BTFs if either module's .ko or vmlinux changed > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > - +$(call if_changed_except,ld_ko_o,vmlinux) > > -ifdef vmlinux > > - +$(if $(newer-prereqs),$(call cmd,btf_ko)) > > -endif > > + +$(call if_changed,ld_ko_o) > > > > targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) > > > > -- > > 2.40.1 > > > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-19 22:54 ` Andrii Nakryiko @ 2023-10-20 7:03 ` Masahiro Yamada 2023-10-20 20:51 ` Andrii Nakryiko 0 siblings, 1 reply; 19+ messages in thread From: Masahiro Yamada @ 2023-10-20 7:03 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > Remove. > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > Changes in v2: > > > - Fix if_changed_except to if_changed > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > --- a/scripts/Makefile.modfinal > > > +++ b/scripts/Makefile.modfinal > > > @@ -19,6 +19,9 @@ vmlinux := > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > ifneq ($(wildcard vmlinux),) > > > vmlinux := vmlinux > > > +cmd_btf = ; \ > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > else > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > endif > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > cmd_ld_ko_o += \ > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > + $(cmd_btf) > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > I don't mind not displaying that, but we should mention that in changelog > > > > Thanks for spotting this! I think those messages are useful and > important to keep. Masahiro, is it possible to preserve them? No, I do not think so. Your code is wrong. To clarify this is a fix, I will replace the commit as follows: ------------------->8---------------------- kbuild: detect btf command change for modules Currently, the command change in cmd_btf_ko does not cause to rebuild the modules because it is not passed to if_changed. Pass everything to if_change so that the btf command is also recorded in the .*.cmd files. This removes the hacky newer_prereqs_except and if_changed_except macros too. ------------------->8---------------------- -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-20 7:03 ` Masahiro Yamada @ 2023-10-20 20:51 ` Andrii Nakryiko 2023-10-21 11:37 ` Masahiro Yamada 0 siblings, 1 reply; 19+ messages in thread From: Andrii Nakryiko @ 2023-10-20 20:51 UTC (permalink / raw) To: Masahiro Yamada Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > Remove. > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > --- > > > > > > > > Changes in v2: > > > > - Fix if_changed_except to if_changed > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > --- a/scripts/Makefile.modfinal > > > > +++ b/scripts/Makefile.modfinal > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > ifneq ($(wildcard vmlinux),) > > > > vmlinux := vmlinux > > > > +cmd_btf = ; \ > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > else > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > endif > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > cmd_ld_ko_o += \ > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > + $(cmd_btf) > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > Thanks for spotting this! I think those messages are useful and > > important to keep. Masahiro, is it possible to preserve them? > > > > No, I do not think so. > That's too bad, I think it's a useful one. > Your code is wrong. > Could be, but note the comment you are removing: # Re-generate module BTFs if either module's .ko or vmlinux changed BTF has to be re-generated not just when module .ko is regenerated, but also when the vmlinux image itself changes. I don't see where this is done with your changes. Can you please point it out explicitly? > > To clarify this is a fix, > I will replace the commit as follows: > > > > > ------------------->8---------------------- > kbuild: detect btf command change for modules > > Currently, the command change in cmd_btf_ko does not cause to rebuild > the modules because it is not passed to if_changed. > > Pass everything to if_change so that the btf command is also recorded > in the .*.cmd files. This removes the hacky newer_prereqs_except and > if_changed_except macros too. > ------------------->8---------------------- > > > > > -- > Best Regards > > Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-20 20:51 ` Andrii Nakryiko @ 2023-10-21 11:37 ` Masahiro Yamada 2023-10-21 19:33 ` Andrii Nakryiko 0 siblings, 1 reply; 19+ messages in thread From: Masahiro Yamada @ 2023-10-21 11:37 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > Remove. > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > --- > > > > > > > > > > Changes in v2: > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > --- a/scripts/Makefile.modfinal > > > > > +++ b/scripts/Makefile.modfinal > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > ifneq ($(wildcard vmlinux),) > > > > > vmlinux := vmlinux > > > > > +cmd_btf = ; \ > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > else > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > endif > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > cmd_ld_ko_o += \ > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > + $(cmd_btf) > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > No, I do not think so. > > > > That's too bad, I think it's a useful one. I prioritize that the code is correct. > > > Your code is wrong. > > > > Could be, but note the comment you are removing: > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > BTF has to be re-generated not just when module .ko is regenerated, > but also when the vmlinux image itself changes. > > I don't see where this is done with your changes. Can you please point > it out explicitly? That is too obvious; %.ko depends on $(vmlinux). %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-21 11:37 ` Masahiro Yamada @ 2023-10-21 19:33 ` Andrii Nakryiko 2023-10-22 20:23 ` Masahiro Yamada 0 siblings, 1 reply; 19+ messages in thread From: Andrii Nakryiko @ 2023-10-21 19:33 UTC (permalink / raw) To: Masahiro Yamada Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > Remove. > > > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > --- > > > > > > > > > > > > Changes in v2: > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > --- a/scripts/Makefile.modfinal > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > vmlinux := vmlinux > > > > > > +cmd_btf = ; \ > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > else > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > endif > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > cmd_ld_ko_o += \ > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > + $(cmd_btf) > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > No, I do not think so. > > > > > > > That's too bad, I think it's a useful one. > > > > I prioritize that the code is correct. > Could you please also prioritize not regressing informativeness of a build log? With your changes it's not clear now if BTF was generated or not for a kernel module, while previously it was obvious and was easy to spot if for some reason BTF was not generated. I'd like to preserve this property, thank you. E.g, can we still have BTF generation as a separate command and do a separate $(call if_changed,btf_ko)? Or something along those lines. Would that work? > > > > > > > Your code is wrong. > > > > > > > Could be, but note the comment you are removing: > > > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > > > BTF has to be re-generated not just when module .ko is regenerated, > > but also when the vmlinux image itself changes. > > > > I don't see where this is done with your changes. Can you please point > > it out explicitly? > > > > That is too obvious; %.ko depends on $(vmlinux). Thank you for your gracious answer. We used to not rebuild module's .ko's when vmlinux didn't change (but we did regen BTFs), and that's why I was confused. Now we forcefully recompile modules, which is a change in behavior which would be nice to call out in the commit message. > > > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > > > > -- > Best Regards > Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-21 19:33 ` Andrii Nakryiko @ 2023-10-22 20:23 ` Masahiro Yamada 2023-10-23 3:19 ` Andrii Nakryiko 0 siblings, 1 reply; 19+ messages in thread From: Masahiro Yamada @ 2023-10-22 20:23 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > > > Remove. > > > > > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > > --- > > > > > > > > > > > > > > Changes in v2: > > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > > --- a/scripts/Makefile.modfinal > > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > > vmlinux := vmlinux > > > > > > > +cmd_btf = ; \ > > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > > else > > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > > endif > > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > > cmd_ld_ko_o += \ > > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > > + $(cmd_btf) > > > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > > > > > No, I do not think so. > > > > > > > > > > That's too bad, I think it's a useful one. > > > > > > > > I prioritize that the code is correct. > > > > Could you please also prioritize not regressing informativeness of a > build log? With your changes it's not clear now if BTF was generated > or not for a kernel module, while previously it was obvious and was > easy to spot if for some reason BTF was not generated. I'd like to > preserve this > property, thank you. > > E.g, can we still have BTF generation as a separate command and do a > separate $(call if_changed,btf_ko)? Or something along those lines. > Would that work? If we have an intermediate file (say, *.no-btf.ko), it would make sense to have separate $(call if_changed,ld_ko_o) and $(call if_changed,btf_ko). LD RESOLVE_BTFIDS *.mod.o ------> *.no-btf.ko ------------> *.ko When vmlinux is changed, only the second step would be re-run, but that would require extra file copy. Is this what you want to see? > > > > > > > > > > > > Your code is wrong. > > > > > > > > > > Could be, but note the comment you are removing: > > > > > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > > > > > BTF has to be re-generated not just when module .ko is regenerated, > > > but also when the vmlinux image itself changes. > > > > > > I don't see where this is done with your changes. Can you please point > > > it out explicitly? > > > > > > > > That is too obvious; %.ko depends on $(vmlinux). > > Thank you for your gracious answer. We used to not rebuild module's > .ko's when vmlinux didn't change (but we did regen BTFs), and that's > why I was confused. Now we forcefully recompile modules, which is a > change in behavior which would be nice to call out in the commit > message. > > > > > > > > > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > > > > > > > > > -- > > Best Regards > > Masahiro Yamada -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-22 20:23 ` Masahiro Yamada @ 2023-10-23 3:19 ` Andrii Nakryiko 2023-10-28 12:00 ` Masahiro Yamada 0 siblings, 1 reply; 19+ messages in thread From: Andrii Nakryiko @ 2023-10-23 3:19 UTC (permalink / raw) To: Masahiro Yamada Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > > > > > Remove. > > > > > > > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > > > --- > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > > > --- a/scripts/Makefile.modfinal > > > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > > > vmlinux := vmlinux > > > > > > > > +cmd_btf = ; \ > > > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > > > else > > > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > > > endif > > > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > > > cmd_ld_ko_o += \ > > > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > > > + $(cmd_btf) > > > > > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > > > > > > > > > No, I do not think so. > > > > > > > > > > > > > That's too bad, I think it's a useful one. > > > > > > > > > > > > I prioritize that the code is correct. > > > > > > > Could you please also prioritize not regressing informativeness of a > > build log? With your changes it's not clear now if BTF was generated > > or not for a kernel module, while previously it was obvious and was > > easy to spot if for some reason BTF was not generated. I'd like to > > preserve this > > property, thank you. > > > > E.g, can we still have BTF generation as a separate command and do a > > separate $(call if_changed,btf_ko)? Or something along those lines. > > Would that work? > > If we have an intermediate file (say, *.no-btf.ko), > it would make sense to have separate > $(call if_changed,ld_ko_o) and $(call if_changed,btf_ko). Currently we don't generate intermediate files, but we do rewrite original .ko file as a post-processing step. And that rewriting step might not happen depending on Kconfig and toolchain (e.g., too old pahole makes it impossible to generate kernel module BTF). And that's why having a separate BTF [M] message in the build log is important. > > > LD RESOLVE_BTFIDS > *.mod.o ------> *.no-btf.ko ------------> *.ko > > > When vmlinux is changed, only the second step would > be re-run, but that would require extra file copy. Today we rewrite .ko with a new .ko ELF file which gains a new ELF section (.BTF), so we already pay this price when BTF is enabled (if that's your concern). > > Is this what you want to see? I don't have strong preferences for exact implementation, but what you propose will work, I think. What I'd like to avoid is unnecessarily relinking .ko files if all we need to do is regenerate BTF. > > > > > > > > > > > > > > > > > > > > > > Your code is wrong. > > > > > > > > > > > > > Could be, but note the comment you are removing: > > > > > > > > # Re-generate module BTFs if either module's .ko or vmlinux changed > > > > > > > > BTF has to be re-generated not just when module .ko is regenerated, > > > > but also when the vmlinux image itself changes. > > > > > > > > I don't see where this is done with your changes. Can you please point > > > > it out explicitly? > > > > > > > > > > > > That is too obvious; %.ko depends on $(vmlinux). > > > > Thank you for your gracious answer. We used to not rebuild module's > > .ko's when vmlinux didn't change (but we did regen BTFs), and that's > > why I was confused. Now we forcefully recompile modules, which is a > > change in behavior which would be nice to call out in the commit > > message. > > > > > > > > > > > > > > > > %.ko: %.o %.mod.o scripts/module.lds $(vmlinux) FORCE > > > > > > > > > > > > > > > -- > > > Best Regards > > > Masahiro Yamada > > > > -- > Best Regards > Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-23 3:19 ` Andrii Nakryiko @ 2023-10-28 12:00 ` Masahiro Yamada 2023-10-28 13:36 ` Jiri Olsa 0 siblings, 1 reply; 19+ messages in thread From: Masahiro Yamada @ 2023-10-28 12:00 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > > > > > > > Remove. > > > > > > > > > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > > > > --- > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > > > > --- a/scripts/Makefile.modfinal > > > > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > > > > vmlinux := vmlinux > > > > > > > > > +cmd_btf = ; \ > > > > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > > > > else > > > > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > > > > endif > > > > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > > > > cmd_ld_ko_o += \ > > > > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > > > > + $(cmd_btf) > > > > > > > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > > > > > > > > > > > > > No, I do not think so. > > > > > > > > > > > > > > > > That's too bad, I think it's a useful one. > > > > > > > > > > > > > > > > I prioritize that the code is correct. > > > > > > > > > > Could you please also prioritize not regressing informativeness of a > > > build log? With your changes it's not clear now if BTF was generated > > > or not for a kernel module, while previously it was obvious and was > > > easy to spot if for some reason BTF was not generated. I'd like to > > > preserve this > > > property, thank you. > > > > > > E.g, can we still have BTF generation as a separate command and do a > > > separate $(call if_changed,btf_ko)? Or something along those lines. > > > Would that work? > > > > If we have an intermediate file (say, *.no-btf.ko), > > it would make sense to have separate > > $(call if_changed,ld_ko_o) and $(call if_changed,btf_ko). > > Currently we don't generate intermediate files, but we do rewrite > original .ko file as a post-processing step. > > And that rewriting step might not happen depending on Kconfig and > toolchain (e.g., too old pahole makes it impossible to generate kernel > module BTF). And that's why having a separate BTF [M] message in the > build log is important. > > > > > > > LD RESOLVE_BTFIDS > > *.mod.o ------> *.no-btf.ko ------------> *.ko > > > > > > When vmlinux is changed, only the second step would > > be re-run, but that would require extra file copy. > > Today we rewrite .ko with a new .ko ELF file which gains a new ELF > section (.BTF), so we already pay this price when BTF is enabled (if > that's your concern). > > > > > Is this what you want to see? > > I don't have strong preferences for exact implementation, but what you > propose will work, I think. What I'd like to avoid is unnecessarily > relinking .ko files if all we need to do is regenerate BTF. Is there any way to make pahole/resolve_btfids take separate input and output files instead of in-place modification? Otherwise, explicit "cp *.no-btf.ko *.ko" would be needed. -- Best Regards Masahiro Yamada ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-28 12:00 ` Masahiro Yamada @ 2023-10-28 13:36 ` Jiri Olsa 2023-10-31 18:44 ` Andrii Nakryiko 0 siblings, 1 reply; 19+ messages in thread From: Jiri Olsa @ 2023-10-28 13:36 UTC (permalink / raw) To: Masahiro Yamada Cc: Andrii Nakryiko, Jiri Olsa, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Sat, Oct 28, 2023 at 09:00:11PM +0900, Masahiro Yamada wrote: > On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: > > > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > > > > > > > > > Remove. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > > > > > --- a/scripts/Makefile.modfinal > > > > > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > > > > > vmlinux := vmlinux > > > > > > > > > > +cmd_btf = ; \ > > > > > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > > > > > else > > > > > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > > > > > endif > > > > > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > > > > > cmd_ld_ko_o += \ > > > > > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > > > > > + $(cmd_btf) > > > > > > > > > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, I do not think so. > > > > > > > > > > > > > > > > > > > That's too bad, I think it's a useful one. > > > > > > > > > > > > > > > > > > > > I prioritize that the code is correct. > > > > > > > > > > > > > Could you please also prioritize not regressing informativeness of a > > > > build log? With your changes it's not clear now if BTF was generated > > > > or not for a kernel module, while previously it was obvious and was > > > > easy to spot if for some reason BTF was not generated. I'd like to > > > > preserve this > > > > property, thank you. > > > > > > > > E.g, can we still have BTF generation as a separate command and do a > > > > separate $(call if_changed,btf_ko)? Or something along those lines. > > > > Would that work? > > > > > > If we have an intermediate file (say, *.no-btf.ko), > > > it would make sense to have separate > > > $(call if_changed,ld_ko_o) and $(call if_changed,btf_ko). > > > > Currently we don't generate intermediate files, but we do rewrite > > original .ko file as a post-processing step. > > > > And that rewriting step might not happen depending on Kconfig and > > toolchain (e.g., too old pahole makes it impossible to generate kernel > > module BTF). And that's why having a separate BTF [M] message in the > > build log is important. > > > > > > > > > > > LD RESOLVE_BTFIDS > > > *.mod.o ------> *.no-btf.ko ------------> *.ko > > > > > > > > > When vmlinux is changed, only the second step would > > > be re-run, but that would require extra file copy. > > > > Today we rewrite .ko with a new .ko ELF file which gains a new ELF > > section (.BTF), so we already pay this price when BTF is enabled (if > > that's your concern). > > > > > > > > Is this what you want to see? > > > > I don't have strong preferences for exact implementation, but what you > > propose will work, I think. What I'd like to avoid is unnecessarily > > relinking .ko files if all we need to do is regenerate BTF. > > > > > Is there any way to make pahole/resolve_btfids > take separate input and output files > instead of in-place modification? for pahole I think it'd be possible to get object file with .BTF section and just link it with other module objects (it's done like that for vmlinux) but I'm not sure which module linking stage this could happen for resolve_btfids it's not possible at the moment, it just updates the .BTF_ids section in the object file I'm working on changing resolve_btfids to actually generate separate object with .BTF_ids section, which is then link-ed with the final object, but will take more time.. especially because I'm not sure where to place this logic in module linking ;-) jirka ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule 2023-10-28 13:36 ` Jiri Olsa @ 2023-10-31 18:44 ` Andrii Nakryiko 0 siblings, 0 replies; 19+ messages in thread From: Andrii Nakryiko @ 2023-10-31 18:44 UTC (permalink / raw) To: Jiri Olsa Cc: Masahiro Yamada, linux-kbuild, linux-kernel, Nathan Chancellor, Nick Desaulniers, Nicolas Schier, bpf On Sat, Oct 28, 2023 at 6:36 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Sat, Oct 28, 2023 at 09:00:11PM +0900, Masahiro Yamada wrote: > > On Mon, Oct 23, 2023 at 12:19 PM Andrii Nakryiko > > <andrii.nakryiko@gmail.com> wrote: > > > > > > On Sun, Oct 22, 2023 at 1:24 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > On Sun, Oct 22, 2023 at 4:33 AM Andrii Nakryiko > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > On Sat, Oct 21, 2023 at 4:38 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > On Sat, Oct 21, 2023 at 5:52 AM Andrii Nakryiko > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 12:03 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > > > > > > > > > On Fri, Oct 20, 2023 at 7:55 AM Andrii Nakryiko > > > > > > > > <andrii.nakryiko@gmail.com> wrote: > > > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 1:15 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > On Thu, Oct 19, 2023 at 12:19:50AM +0900, Masahiro Yamada wrote: > > > > > > > > > > > newer_prereqs_except and if_changed_except are ugly hacks of the > > > > > > > > > > > newer-prereqs and if_changed in scripts/Kbuild.include. > > > > > > > > > > > > > > > > > > > > > > Remove. > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > > > > > > > > --- > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > - Fix if_changed_except to if_changed > > > > > > > > > > > > > > > > > > > > > > scripts/Makefile.modfinal | 25 ++++++------------------- > > > > > > > > > > > 1 file changed, 6 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > > > > > > > > > diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal > > > > > > > > > > > index 9fd7a26e4fe9..fc07854bb7b9 100644 > > > > > > > > > > > --- a/scripts/Makefile.modfinal > > > > > > > > > > > +++ b/scripts/Makefile.modfinal > > > > > > > > > > > @@ -19,6 +19,9 @@ vmlinux := > > > > > > > > > > > ifdef CONFIG_DEBUG_INFO_BTF_MODULES > > > > > > > > > > > ifneq ($(wildcard vmlinux),) > > > > > > > > > > > vmlinux := vmlinux > > > > > > > > > > > +cmd_btf = ; \ > > > > > > > > > > > + LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J $(PAHOLE_FLAGS) --btf_base vmlinux $@; \ > > > > > > > > > > > + $(RESOLVE_BTFIDS) -b vmlinux $@ > > > > > > > > > > > else > > > > > > > > > > > $(warning Skipping BTF generation due to unavailability of vmlinux) > > > > > > > > > > > endif > > > > > > > > > > > @@ -41,27 +44,11 @@ quiet_cmd_ld_ko_o = LD [M] $@ > > > > > > > > > > > cmd_ld_ko_o += \ > > > > > > > > > > > $(LD) -r $(KBUILD_LDFLAGS) \ > > > > > > > > > > > $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE) \ > > > > > > > > > > > - -T scripts/module.lds -o $@ $(filter %.o, $^) > > > > > > > > > > > + -T scripts/module.lds -o $@ $(filter %.o, $^) \ > > > > > > > > > > > + $(cmd_btf) > > > > > > > > > > > > > > > > > > > > > > -quiet_cmd_btf_ko = BTF [M] $@ > > > > > > > > > > > > > > > > > > > > nit not sure it's intentional but we no longer display 'BTF [M] ...ko' lines, > > > > > > > > > > I don't mind not displaying that, but we should mention that in changelog > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for spotting this! I think those messages are useful and > > > > > > > > > important to keep. Masahiro, is it possible to preserve them? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > No, I do not think so. > > > > > > > > > > > > > > > > > > > > > > That's too bad, I think it's a useful one. > > > > > > > > > > > > > > > > > > > > > > > > I prioritize that the code is correct. > > > > > > > > > > > > > > > > Could you please also prioritize not regressing informativeness of a > > > > > build log? With your changes it's not clear now if BTF was generated > > > > > or not for a kernel module, while previously it was obvious and was > > > > > easy to spot if for some reason BTF was not generated. I'd like to > > > > > preserve this > > > > > property, thank you. > > > > > > > > > > E.g, can we still have BTF generation as a separate command and do a > > > > > separate $(call if_changed,btf_ko)? Or something along those lines. > > > > > Would that work? > > > > > > > > If we have an intermediate file (say, *.no-btf.ko), > > > > it would make sense to have separate > > > > $(call if_changed,ld_ko_o) and $(call if_changed,btf_ko). > > > > > > Currently we don't generate intermediate files, but we do rewrite > > > original .ko file as a post-processing step. > > > > > > And that rewriting step might not happen depending on Kconfig and > > > toolchain (e.g., too old pahole makes it impossible to generate kernel > > > module BTF). And that's why having a separate BTF [M] message in the > > > build log is important. > > > > > > > > > > > > > > > LD RESOLVE_BTFIDS > > > > *.mod.o ------> *.no-btf.ko ------------> *.ko > > > > > > > > > > > > When vmlinux is changed, only the second step would > > > > be re-run, but that would require extra file copy. > > > > > > Today we rewrite .ko with a new .ko ELF file which gains a new ELF > > > section (.BTF), so we already pay this price when BTF is enabled (if > > > that's your concern). > > > > > > > > > > > Is this what you want to see? > > > > > > I don't have strong preferences for exact implementation, but what you > > > propose will work, I think. What I'd like to avoid is unnecessarily > > > relinking .ko files if all we need to do is regenerate BTF. > > > > > > > > > > Is there any way to make pahole/resolve_btfids > > take separate input and output files > > instead of in-place modification? > > for pahole I think it'd be possible to get object file with .BTF section > and just link it with other module objects (it's done like that for vmlinux) > but I'm not sure which module linking stage this could happen > > for resolve_btfids it's not possible at the moment, it just updates the > .BTF_ids section in the object file > > I'm working on changing resolve_btfids to actually generate separate object > with .BTF_ids section, which is then link-ed with the final object, but will > take more time.. especially because I'm not sure where to place this logic > in module linking ;-) pahole also supports mode of generating BTF into a separate file without modifying the original one. The option is called --btf_encode_detached. It was added in v1.22 (currently the minimal version is v1.16), though, so depending on whether we are willing to bump the minimum pahole version, we might use that. That will allow us to also simplify and clean up link-vmlinux.sh a bit, I think. But I don't know if it's worth the trouble right now. > > jirka ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2023-10-31 18:44 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-10-18 15:19 [bpf-next PATCH v2 1/4] kbuild: remove ARCH_POSTLINK from module builds Masahiro Yamada 2023-10-18 15:19 ` [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada 2023-10-19 8:47 ` Jiri Olsa 2023-10-23 0:33 ` Martin Rodriguez Reboredo 2023-10-18 15:19 ` [bpf-next PATCH v2 3/4] kbuild: skip module BTF with one-time check for vmlinux Masahiro Yamada 2023-10-19 8:17 ` Jiri Olsa 2023-10-18 15:19 ` [bpf-next PATCH v2 4/4] kbuild: refactor module BTF rule Masahiro Yamada 2023-10-18 15:49 ` Nicolas Schier 2023-10-19 8:15 ` Jiri Olsa 2023-10-19 22:54 ` Andrii Nakryiko 2023-10-20 7:03 ` Masahiro Yamada 2023-10-20 20:51 ` Andrii Nakryiko 2023-10-21 11:37 ` Masahiro Yamada 2023-10-21 19:33 ` Andrii Nakryiko 2023-10-22 20:23 ` Masahiro Yamada 2023-10-23 3:19 ` Andrii Nakryiko 2023-10-28 12:00 ` Masahiro Yamada 2023-10-28 13:36 ` Jiri Olsa 2023-10-31 18:44 ` Andrii Nakryiko
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).