LinuxLists.cc - [PATCH] Kbuild: Update config_data.gz only if KCONFIG

2021-04-01 22:46:56

Subject: [PATCH] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

2021-04-03 05:15:22

Subject: Re: [PATCH] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

On Fri, Apr 2, 2021 at 7:45 AM Elliot Berman <[email protected]> wrote:
>
> If you update the timestamp of KCONFIG_CONFIG without actually changing
> anything, config_data.gz is re-generated and causes vmlinux to re-link.
> When Link Time Optimization is enabled, unnecessary re-linking of
> vmlinux is highly desirable since it adds several minutes to build time.
>
> Avoid touching config_data.gz by using filechk to compare the existing
> config_data.gz and update only if it changed.
>
> The .config can be touched, for instance, by a build script which
> installs the default defconfig and then applies a defconfig fragment on
> top.
>
> For a simple example on my x86 machine, I modified x86 default defconfig to set
> CONFIG_IKCONFIG=y and run:
> make -j50 defconfig tiny.config vmlinux
> make -j50 defconfig tiny.config vmlinux
> With this patch, vmlinux is not re-built as a result of config_data.gz
> change.
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> kernel/Makefile | 2 +-
> scripts/Makefile.lib | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 320f1f3..bd4e558 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -140,7 +140,7 @@ $(obj)/configs.o: $(obj)/config_data.gz
>
> targets += config_data.gz
> $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
> - $(call if_changed,gzip)
> + $(call filechk,gzip)

I do not think this is the right approach
because gzip is executed every time, even
if the time stamp is not changed.

>
> $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
>
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index a4fbaf8..81d3ec1 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -282,6 +282,8 @@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
> quiet_cmd_gzip = GZIP $@
> cmd_gzip = cat $(real-prereqs) | $(KGZIP) -n -f -9 > $@
>
> +filechk_gzip = cat $(real-prereqs) | $(KGZIP) -n -f -9
> +
> # DTC
> # ---------------------------------------------------------------------------
> DTC ?= $(objtree)/scripts/dtc/dtc
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

--
Best Regards
Masahiro Yamada

2021-04-05 05:15:02

by Elliot Berman

[permalink] [raw]

Subject: Re: [PATCH] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

On 4/2/2021 10:11 PM, Masahiro Yamada wrote:
> On Fri, Apr 2, 2021 at 7:45 AM Elliot Berman <[email protected]> wrote:
>>
>> If you update the timestamp of KCONFIG_CONFIG without actually changing
>> anything, config_data.gz is re-generated and causes vmlinux to re-link.
>> When Link Time Optimization is enabled, unnecessary re-linking of
>> vmlinux is highly desirable since it adds several minutes to build time.
>>
>> Avoid touching config_data.gz by using filechk to compare the existing
>> config_data.gz and update only if it changed.
>>
>> The .config can be touched, for instance, by a build script which
>> installs the default defconfig and then applies a defconfig fragment on
>> top.
>>
>> For a simple example on my x86 machine, I modified x86 default defconfig to set
>> CONFIG_IKCONFIG=y and run:
>> make -j50 defconfig tiny.config vmlinux
>> make -j50 defconfig tiny.config vmlinux
>> With this patch, vmlinux is not re-built as a result of config_data.gz
>> change.
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> kernel/Makefile | 2 +-
>> scripts/Makefile.lib | 2 ++
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/kernel/Makefile b/kernel/Makefile
>> index 320f1f3..bd4e558 100644
>> --- a/kernel/Makefile
>> +++ b/kernel/Makefile
>> @@ -140,7 +140,7 @@ $(obj)/configs.o: $(obj)/config_data.gz
>>
>> targets += config_data.gz
>> $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
>> - $(call if_changed,gzip)
>> + $(call filechk,gzip)
>
>
> I do not think this is the right approach
> because gzip is executed every time, even
> if the time stamp is not changed.
>

Since .config is relatively small, gzip was quickly producing the same
binary in multiple runs on my host. I thought about following
gen_ikheaders.sh approach of comparing the md5sum, but I felt it was
more complex than using filechk and re-compressing every time. I'll send
a v2 tomorrow which follows gen_ikheaders.sh approach.

>
>>
>> $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
>>
>> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
>> index a4fbaf8..81d3ec1 100644
>> --- a/scripts/Makefile.lib
>> +++ b/scripts/Makefile.lib
>> @@ -282,6 +282,8 @@ cmd_objcopy = $(OBJCOPY) $(OBJCOPYFLAGS) $(OBJCOPYFLAGS_$(@F)) $< $@
>> quiet_cmd_gzip = GZIP $@
>> cmd_gzip = cat $(real-prereqs) | $(KGZIP) -n -f -9 > $@
>>
>> +filechk_gzip = cat $(real-prereqs) | $(KGZIP) -n -f -9
>> +
>> # DTC
>> # ---------------------------------------------------------------------------
>> DTC ?= $(objtree)/scripts/dtc/dtc
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>

2021-04-06 08:12:24

by Elliot Berman

[permalink] [raw]

Subject: [PATCH v2] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

If you update the timestamp of KCONFIG_CONFIG without actually changing
anything, config_data.gz is re-generated and causes vmlinux to re-link.
When Link Time Optimization is enabled, unnecessary re-linking of
vmlinux is highly desirable since it adds several minutes to build time.

Avoid touching config_data.gz by using a script to compare the existing
config_data.gz, KCONFIG_CONFIG, or script itself to update only if any
is mismatched. The script follows gen_kheaders.sh approach for
determing in update is needed. The script intentionally avoids
re-compressing KCONFIG_CONFIG.

The .config can be touched, for instance, by a build script which
installs the default defconfig and then applies a defconfig fragment on
top.

For a simple example on my x86 machine, I modified x86 default defconfig to set
CONFIG_IKCONFIG=y and run:
make -j50 defconfig tiny.config vmlinux
make -j50 defconfig tiny.config vmlinux
With this patch, vmlinux is not re-built as a result of config_data.gz
change.

Changes in v2:
- Use md5 checksum to compare .config instead of gzip'ing again

Signed-off-by: Elliot Berman <[email protected]>
---
kernel/.gitignore | 1 +
kernel/Makefile | 4 +++-
kernel/gen_config_data.sh | 31 +++++++++++++++++++++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
create mode 100755 kernel/gen_config_data.sh

diff --git a/kernel/.gitignore b/kernel/.gitignore
index 78701ea..a191136 100644
--- a/kernel/.gitignore
+++ b/kernel/.gitignore
@@ -1,4 +1,5 @@
# SPDX-License-Identifier: GPL-2.0-only
+config_data.gz.md5
kheaders.md5
timeconst.h
hz.bc
diff --git a/kernel/Makefile b/kernel/Makefile
index 320f1f3..0784bf3d 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -139,8 +139,10 @@ obj-$(CONFIG_SCF_TORTURE_TEST) += scftorture.o
$(obj)/configs.o: $(obj)/config_data.gz

targets += config_data.gz
+quiet_cmd_genicfg = CHK $(obj)/config_data.gz
+ cmd_genicfg = $(CONFIG_SHELL) $(srctree)/kernel/gen_config_data.sh $@ $<
$(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
- $(call if_changed,gzip)
+ $(call cmd,genicfg)

$(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz

diff --git a/kernel/gen_config_data.sh b/kernel/gen_config_data.sh
new file mode 100755
index 00000000..e9ff193
--- /dev/null
+++ b/kernel/gen_config_data.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+# This script generates a compressed version of .config, if its checksum has changed
+set -e
+
+this_file="$(readlink -f "$0")"
+outfile=$1
+infile=$2
+
+config_md5="$(md5sum $infile | cut -d ' ' -f1)"
+# Any changes to this script will also cause a rebuild of config_data.
+this_file_md5="$(md5sum $sfile | cut -d ' ' -f1)"
+if [ -f $outfile ]; then outfile_md5="$(md5sum $outfile | cut -d ' ' -f1)"; fi
+
+if [ -f $outfile.md5 ] &&
+ [ "$(head -n 1 $outfile.md5)" = "$config_md5" ] &&
+ [ "$(head -n 2 $outfile.md5 | tail -n 1)" = "$this_file_md5" ] &&
+ [ "$(tail -n 1 $outfile.md5)" = "$outfile_md5" ]; then
+ exit
+fi
+
+if [ "${quiet}" != "silent_" ]; then
+ echo " GEN $outfile"
+fi
+
+${KGZIP} -c -n -f -9 $infile > $outfile
+
+echo "$config_md5" > $outfile.md5
+echo "$this_file_md5" >> $outfile.md5
+echo "$(md5sum $outfile | cut -d ' ' -f1)" >> $outfile.md5
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2021-04-23 17:03:38

by Elliot Berman

[permalink] [raw]

Subject: [RESEND v2] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

2021-04-25 06:53:05

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [RESEND v2] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

On Sat, Apr 24, 2021 at 2:02 AM Elliot Berman <[email protected]> wrote:

Sorry for the delay.

This patch is over-engineering.

I will apply this.
https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/

The 'cmp' command is not expensive.
md5sum is unneeded.

> If you update the timestamp of KCONFIG_CONFIG without actually changing
> anything, config_data.gz is re-generated and causes vmlinux to re-link.
> When Link Time Optimization is enabled, unnecessary re-linking of
> vmlinux is highly desirable since it adds several minutes to build time.
>
> Avoid touching config_data.gz by using a script to compare the existing
> config_data.gz, KCONFIG_CONFIG, or script itself to update only if any
> is mismatched. The script follows gen_kheaders.sh approach for
> determing in update is needed. The script intentionally avoids
> re-compressing KCONFIG_CONFIG.
>
> The .config can be touched, for instance, by a build script which
> installs the default defconfig and then applies a defconfig fragment on
> top.
>
> For a simple example on my x86 machine, I modified x86 default defconfig to set
> CONFIG_IKCONFIG=y and run:
> make -j50 defconfig tiny.config vmlinux
> make -j50 defconfig tiny.config vmlinux
> With this patch, vmlinux is not re-built as a result of config_data.gz
> change.
>
> Changes in v2:
> - Use md5 checksum to compare .config instead of gzip'ing again
>
> Signed-off-by: Elliot Berman <[email protected]>
> ---
> kernel/.gitignore | 1 +
> kernel/Makefile | 4 +++-
> kernel/gen_config_data.sh | 31 +++++++++++++++++++++++++++++++
> 3 files changed, 35 insertions(+), 1 deletion(-)
> create mode 100755 kernel/gen_config_data.sh
>
> diff --git a/kernel/.gitignore b/kernel/.gitignore
> index 78701ea..a191136 100644
> --- a/kernel/.gitignore
> +++ b/kernel/.gitignore
> @@ -1,4 +1,5 @@
> # SPDX-License-Identifier: GPL-2.0-only
> +config_data.gz.md5
> kheaders.md5
> timeconst.h
> hz.bc
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 320f1f3..0784bf3d 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -139,8 +139,10 @@ obj-$(CONFIG_SCF_TORTURE_TEST) += scftorture.o
> $(obj)/configs.o: $(obj)/config_data.gz
>
> targets += config_data.gz
> +quiet_cmd_genicfg = CHK $(obj)/config_data.gz
> + cmd_genicfg = $(CONFIG_SHELL) $(srctree)/kernel/gen_config_data.sh $@ $<
> $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
> - $(call if_changed,gzip)
> + $(call cmd,genicfg)
>
> $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
>
> diff --git a/kernel/gen_config_data.sh b/kernel/gen_config_data.sh
> new file mode 100755
> index 00000000..e9ff193
> --- /dev/null
> +++ b/kernel/gen_config_data.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# This script generates a compressed version of .config, if its checksum has changed
> +set -e
> +
> +this_file="$(readlink -f "$0")"
> +outfile=$1
> +infile=$2
> +
> +config_md5="$(md5sum $infile | cut -d ' ' -f1)"
> +# Any changes to this script will also cause a rebuild of config_data.
> +this_file_md5="$(md5sum $sfile | cut -d ' ' -f1)"
> +if [ -f $outfile ]; then outfile_md5="$(md5sum $outfile | cut -d ' ' -f1)"; fi
> +
> +if [ -f $outfile.md5 ] &&
> + [ "$(head -n 1 $outfile.md5)" = "$config_md5" ] &&
> + [ "$(head -n 2 $outfile.md5 | tail -n 1)" = "$this_file_md5" ] &&
> + [ "$(tail -n 1 $outfile.md5)" = "$outfile_md5" ]; then
> + exit
> +fi
> +
> +if [ "${quiet}" != "silent_" ]; then
> + echo " GEN $outfile"
> +fi
> +
> +${KGZIP} -c -n -f -9 $infile > $outfile
> +
> +echo "$config_md5" > $outfile.md5
> +echo "$this_file_md5" >> $outfile.md5
> +echo "$(md5sum $outfile | cut -d ' ' -f1)" >> $outfile.md5
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

--
Best Regards
Masahiro Yamada

2021-04-26 16:18:53

by Elliot Berman

[permalink] [raw]

Subject: Re: [RESEND v2] Kbuild: Update config_data.gz only if KCONFIG_CONFIG materially changed

On 4/24/2021 11:50 PM, Masahiro Yamada wrote:
> On Sat, Apr 24, 2021 at 2:02 AM Elliot Berman <[email protected]> wrote:
>
>
> Sorry for the delay.

No problem!

>
> This patch is over-engineering.

I thought so, too.

>
> I will apply this.
> https://patchwork.kernel.org/project/linux-kbuild/patch/[email protected]/
>
> The 'cmp' command is not expensive.
> md5sum is unneeded.

Thanks!

>
>> If you update the timestamp of KCONFIG_CONFIG without actually changing
>> anything, config_data.gz is re-generated and causes vmlinux to re-link.
>> When Link Time Optimization is enabled, unnecessary re-linking of
>> vmlinux is highly desirable since it adds several minutes to build time.
>>
>> Avoid touching config_data.gz by using a script to compare the existing
>> config_data.gz, KCONFIG_CONFIG, or script itself to update only if any
>> is mismatched. The script follows gen_kheaders.sh approach for
>> determing in update is needed. The script intentionally avoids
>> re-compressing KCONFIG_CONFIG.
>>
>> The .config can be touched, for instance, by a build script which
>> installs the default defconfig and then applies a defconfig fragment on
>> top.
>>
>> For a simple example on my x86 machine, I modified x86 default defconfig to set
>> CONFIG_IKCONFIG=y and run:
>> make -j50 defconfig tiny.config vmlinux
>> make -j50 defconfig tiny.config vmlinux
>> With this patch, vmlinux is not re-built as a result of config_data.gz
>> change.
>>
>> Changes in v2:
>> - Use md5 checksum to compare .config instead of gzip'ing again
>>
>> Signed-off-by: Elliot Berman <[email protected]>
>> ---
>> kernel/.gitignore | 1 +
>> kernel/Makefile | 4 +++-
>> kernel/gen_config_data.sh | 31 +++++++++++++++++++++++++++++++
>> 3 files changed, 35 insertions(+), 1 deletion(-)
>> create mode 100755 kernel/gen_config_data.sh
>>
>> diff --git a/kernel/.gitignore b/kernel/.gitignore
>> index 78701ea..a191136 100644
>> --- a/kernel/.gitignore
>> +++ b/kernel/.gitignore
>> @@ -1,4 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0-only
>> +config_data.gz.md5
>> kheaders.md5
>> timeconst.h
>> hz.bc
>> diff --git a/kernel/Makefile b/kernel/Makefile
>> index 320f1f3..0784bf3d 100644
>> --- a/kernel/Makefile
>> +++ b/kernel/Makefile
>> @@ -139,8 +139,10 @@ obj-$(CONFIG_SCF_TORTURE_TEST) += scftorture.o
>> $(obj)/configs.o: $(obj)/config_data.gz
>>
>> targets += config_data.gz
>> +quiet_cmd_genicfg = CHK $(obj)/config_data.gz
>> + cmd_genicfg = $(CONFIG_SHELL) $(srctree)/kernel/gen_config_data.sh $@ $<
>> $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
>> - $(call if_changed,gzip)
>> + $(call cmd,genicfg)
>>
>> $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
>>
>> diff --git a/kernel/gen_config_data.sh b/kernel/gen_config_data.sh
>> new file mode 100755
>> index 00000000..e9ff193
>> --- /dev/null
>> +++ b/kernel/gen_config_data.sh
>> @@ -0,0 +1,31 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +# This script generates a compressed version of .config, if its checksum has changed
>> +set -e
>> +
>> +this_file="$(readlink -f "$0")"
>> +outfile=$1
>> +infile=$2
>> +
>> +config_md5="$(md5sum $infile | cut -d ' ' -f1)"
>> +# Any changes to this script will also cause a rebuild of config_data.
>> +this_file_md5="$(md5sum $sfile | cut -d ' ' -f1)"
>> +if [ -f $outfile ]; then outfile_md5="$(md5sum $outfile | cut -d ' ' -f1)"; fi
>> +
>> +if [ -f $outfile.md5 ] &&
>> + [ "$(head -n 1 $outfile.md5)" = "$config_md5" ] &&
>> + [ "$(head -n 2 $outfile.md5 | tail -n 1)" = "$this_file_md5" ] &&
>> + [ "$(tail -n 1 $outfile.md5)" = "$outfile_md5" ]; then
>> + exit
>> +fi
>> +
>> +if [ "${quiet}" != "silent_" ]; then
>> + echo " GEN $outfile"
>> +fi
>> +
>> +${KGZIP} -c -n -f -9 $infile > $outfile
>> +
>> +echo "$config_md5" > $outfile.md5
>> +echo "$this_file_md5" >> $outfile.md5
>> +echo "$(md5sum $outfile | cut -d ' ' -f1)" >> $outfile.md5
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
>> a Linux Foundation Collaborative Project
>>
>
>