| Message ID | 20231017103742.130927-2-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | None | expand |
On 17/10/2023 11:37, 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 exectuted 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> Looks like a neat change to me. I tested with CONFIG_DEBUG_INFO_BTF=y builds for full kernel build and 'make M=modpath' module-only builds, and both work well with correct pahole flags. Feel free to add either/both of Reviewed-by: Alan Maguire <alan.maguire@oracle.com> Tested-by: Alan Maguire <alan.maguire@oracle.com> > --- > > Makefile | 4 +--- > scripts/Makefile.btf | 19 +++++++++++++++++++ > scripts/pahole-flags.sh | 30 ------------------------------ > 3 files changed, 20 insertions(+), 33 deletions(-) > create mode 100644 scripts/Makefile.btf > delete mode 100755 scripts/pahole-flags.sh > > 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}
On Tue, Oct 17, 2023 at 07:37:40PM +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 exectuted 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] nice :-\ > > 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. looks good.. could you please resend this patch with bpf-next in subject so CI tests would trigger for it? thanks, jirka > > [1]: https://savannah.gnu.org/bugs/index.php?64746 > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- > > Makefile | 4 +--- > scripts/Makefile.btf | 19 +++++++++++++++++++ > scripts/pahole-flags.sh | 30 ------------------------------ > 3 files changed, 20 insertions(+), 33 deletions(-) > create mode 100644 scripts/Makefile.btf > delete mode 100755 scripts/pahole-flags.sh > > 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 >
> Subject: kbuild: avoid too many execution of scripts/pahole-flags.sh executions? On Tue, Oct 17, 2023 at 07:37:40PM +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 exectuted more than 20 times during the kernel build executed > 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> > --- Thanks, looks good to me! Reviewed-by: Nicolas Schier <n.schier@avm.de>
On Tue, Oct 17, 2023 at 12:38 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > 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> The field in `MAINTAINERS` should be removed: F: scripts/pahole-flags.sh But other than that, it looks good to me! I tried it for a given config and it does call `pahole` with the same flags. Tested-by: Miguel Ojeda <ojeda@kernel.org> Acked-by: Miguel Ojeda <ojeda@kernel.org> Cheers, Miguel
On Tue, Oct 17, 2023 at 3:38 AM Masahiro Yamada <masahiroy@kernel.org> 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 exectuted 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> > --- > > Makefile | 4 +--- > scripts/Makefile.btf | 19 +++++++++++++++++++ > scripts/pahole-flags.sh | 30 ------------------------------ > 3 files changed, 20 insertions(+), 33 deletions(-) > create mode 100644 scripts/Makefile.btf > delete mode 100755 scripts/pahole-flags.sh > > 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) What if we just used := here? Wouldn't it avoid unnecessary multiple executions? I don't make Makefile.btf approach, just curious why := doesn't work, if it doesn't. > - > 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 >
On Wed, Oct 18, 2023 at 4:57 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Oct 17, 2023 at 3:38 AM Masahiro Yamada <masahiroy@kernel.org> 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 exectuted 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> > > --- > > > > Makefile | 4 +--- > > scripts/Makefile.btf | 19 +++++++++++++++++++ > > scripts/pahole-flags.sh | 30 ------------------------------ > > 3 files changed, 20 insertions(+), 33 deletions(-) > > create mode 100644 scripts/Makefile.btf > > delete mode 100755 scripts/pahole-flags.sh > > > > 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) > > What if we just used := here? Wouldn't it avoid unnecessary multiple executions? Yeah, := is less silly than =. But, I do not like to run the script for non-build targets such as 'make clean', 'make help', etc. Also, when building with CONFIG_DEBUG_INFO_BTF=n, the shell is forked to compute PAHOLE_FLAGS, which we know are unnecessary. -- Best Regards Masahiro Yamada
On Wed, Oct 18, 2023 at 12:21 AM Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote: > > On Tue, Oct 17, 2023 at 12:38 PM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > 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> > > The field in `MAINTAINERS` should be removed: > > F: scripts/pahole-flags.sh I will replace it with F: scripts/Makefile.btf Thanks. > > But other than that, it looks good to me! I tried it for a given > config and it does call `pahole` with the same flags. > > Tested-by: Miguel Ojeda <ojeda@kernel.org> > Acked-by: Miguel Ojeda <ojeda@kernel.org> > > Cheers, > Miguel
On 10/17/23 07:37, 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 exectuted 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> > --- > [...] > +include-$(CONFIG_DEBUG_INFO_BTF)+= scripts/Makefile.btf Would have used a tab. > [...] Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@gmail.com>
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}