* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 10:37 ` [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada
@ 2023-10-17 12:14 ` Alan Maguire
2023-10-17 12:15 ` Jiri Olsa
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Alan Maguire @ 2023-10-17 12:14 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
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}
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 10:37 ` [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada
2023-10-17 12:14 ` Alan Maguire
@ 2023-10-17 12:15 ` Jiri Olsa
2023-10-17 12:29 ` Nicolas Schier
` (3 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Jiri Olsa @ 2023-10-17 12:15 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, 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, Miguel Ojeda,
Nathan Chancellor, Nick Desaulniers, Nicolas Schier, Song Liu,
Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song, bpf,
rust-for-linux
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 10:37 ` [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada
2023-10-17 12:14 ` Alan Maguire
2023-10-17 12:15 ` Jiri Olsa
@ 2023-10-17 12:29 ` Nicolas Schier
2023-10-17 15:21 ` Miguel Ojeda
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Nicolas Schier @ 2023-10-17 12:29 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
> 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>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 10:37 ` [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada
` (2 preceding siblings ...)
2023-10-17 12:29 ` Nicolas Schier
@ 2023-10-17 15:21 ` Miguel Ojeda
2023-10-18 12:14 ` Masahiro Yamada
2023-10-17 19:56 ` Andrii Nakryiko
2023-10-23 0:30 ` Martin Rodriguez Reboredo
5 siblings, 1 reply; 17+ messages in thread
From: Miguel Ojeda @ 2023-10-17 15:21 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 15:21 ` Miguel Ojeda
@ 2023-10-18 12:14 ` Masahiro Yamada
0 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2023-10-18 12:14 UTC (permalink / raw)
To: Miguel Ojeda
Cc: linux-kbuild, linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
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
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 10:37 ` [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada
` (3 preceding siblings ...)
2023-10-17 15:21 ` Miguel Ojeda
@ 2023-10-17 19:56 ` Andrii Nakryiko
2023-10-18 5:00 ` Masahiro Yamada
2023-10-23 0:30 ` Martin Rodriguez Reboredo
5 siblings, 1 reply; 17+ messages in thread
From: Andrii Nakryiko @ 2023-10-17 19:56 UTC (permalink / raw)
To: Masahiro Yamada
Cc: linux-kbuild, linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
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
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 19:56 ` Andrii Nakryiko
@ 2023-10-18 5:00 ` Masahiro Yamada
0 siblings, 0 replies; 17+ messages in thread
From: Masahiro Yamada @ 2023-10-18 5:00 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: linux-kbuild, linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
2023-10-17 10:37 ` [PATCH 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh Masahiro Yamada
` (4 preceding siblings ...)
2023-10-17 19:56 ` Andrii Nakryiko
@ 2023-10-23 0:30 ` Martin Rodriguez Reboredo
5 siblings, 0 replies; 17+ messages in thread
From: Martin Rodriguez Reboredo @ 2023-10-23 0:30 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
Cc: linux-kernel, 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,
Miguel Ojeda, Nathan Chancellor, Nick Desaulniers, Nicolas Schier,
Song Liu, Stanislav Fomichev, Wedson Almeida Filho, Yonghong Song,
bpf, rust-for-linux
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>
^ permalink raw reply [flat|nested] 17+ messages in thread