* [bpf-next PATCH v2 2/4] kbuild: avoid too many execution of scripts/pahole-flags.sh
[not found] <20231018151950.205265-1-masahiroy@kernel.org>
@ 2023-10-18 15:19 ` Masahiro Yamada
2023-10-19 8:47 ` Jiri Olsa
2023-10-23 0:33 ` Martin Rodriguez Reboredo
0 siblings, 2 replies; 3+ 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] 3+ 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; 3+ 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] 3+ 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; 3+ 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] 3+ messages in thread
end of thread, other threads:[~2023-10-23 0:34 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20231018151950.205265-1-masahiroy@kernel.org>
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
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).