Currently Makefile.modinst does three tasks on each module built:
- Install modules
- Sign modules
- Compress modules
All the above tasks happen from a single place.
This patch divides this task further and uses a different makefile for
each task.
Signing module logic is completely refactored and everything happens
from a shell script now.
Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
---
scripts/Makefile.compress | 53 ++++++++++++++++++
scripts/Makefile.install | 66 +++++++++++++++++++++++
scripts/Makefile.modinst | 111 +++-----------------------------------
scripts/Makefile.sign | 37 +++++++++++++
scripts/signfile.sh | 24 +++++++++
5 files changed, 186 insertions(+), 105 deletions(-)
create mode 100644 scripts/Makefile.compress
create mode 100644 scripts/Makefile.install
create mode 100644 scripts/Makefile.sign
create mode 100755 scripts/signfile.sh
diff --git a/scripts/Makefile.compress b/scripts/Makefile.compress
new file mode 100644
index 000000000000..35d337ac9b6c
--- /dev/null
+++ b/scripts/Makefile.compress
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: GPL-2.0
+# ==========================================================================
+# Compressing modules
+# ==========================================================================
+
+PHONY := __modcompress
+__modcompress:
+
+include include/config/auto.conf
+include $(srctree)/scripts/Kbuild.include
+
+modules := $(call read-file, $(MODORDER))
+
+ifeq ($(KBUILD_EXTMOD),)
+dst := $(MODLIB)/kernel
+else
+INSTALL_MOD_DIR ?= updates
+dst := $(MODLIB)/$(INSTALL_MOD_DIR)
+endif
+
+suffix-y :=
+suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
+suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
+suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
+
+modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
+
+__modcompress: $(modules)
+ @:
+
+#
+# Compression
+#
+quiet_cmd_gzip = GZIP $@
+ cmd_gzip = $(KGZIP) -n -f $<
+quiet_cmd_xz = XZ $@
+ cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
+quiet_cmd_zstd = ZSTD $@
+ cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
+
+$(dst)/%.ko.gz: $(dst)/%.ko FORCE
+ $(call cmd,gzip)
+
+$(dst)/%.ko.xz: $(dst)/%.ko FORCE
+ $(call cmd,xz)
+
+$(dst)/%.ko.zst: $(dst)/%.ko FORCE
+ $(call cmd,zstd)
+
+PHONY += FORCE
+FORCE:
+
+.PHONY: $(PHONY)
diff --git a/scripts/Makefile.install b/scripts/Makefile.install
new file mode 100644
index 000000000000..40c496cb99dc
--- /dev/null
+++ b/scripts/Makefile.install
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: GPL-2.0
+# ==========================================================================
+# Installing modules
+# ==========================================================================
+
+PHONY := __modinstall
+__modinstall:
+
+include include/config/auto.conf
+include $(srctree)/scripts/Kbuild.include
+
+modules := $(call read-file, $(MODORDER))
+
+ifeq ($(KBUILD_EXTMOD),)
+dst := $(MODLIB)/kernel
+else
+INSTALL_MOD_DIR ?= updates
+dst := $(MODLIB)/$(INSTALL_MOD_DIR)
+endif
+
+$(foreach x, % :, $(if $(findstring $x, $(dst)), \
+ $(error module installation path cannot contain '$x')))
+
+modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
+
+__modinstall: $(modules)
+ @:
+
+#
+# Installation
+#
+quiet_cmd_install = INSTALL $@
+ cmd_install = mkdir -p $(dir $@); cp $< $@
+
+# Strip
+#
+# INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they
+# are installed. If INSTALL_MOD_STRIP is '1', then the default option
+# --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will be used
+# as the options to the strip command.
+ifdef INSTALL_MOD_STRIP
+
+ifeq ($(INSTALL_MOD_STRIP),1)
+strip-option := --strip-debug
+else
+strip-option := $(INSTALL_MOD_STRIP)
+endif
+
+quiet_cmd_strip = STRIP $@
+ cmd_strip = $(STRIP) $(strip-option) $@
+
+else
+
+quiet_cmd_strip =
+ cmd_strip = :
+
+endif
+
+$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
+ $(call cmd,install)
+ $(call cmd,strip)
+
+PHONY += FORCE
+FORCE:
+
+.PHONY: $(PHONY)
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index ab0c5bd1a60f..d87e09e57963 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -1,116 +1,17 @@
# SPDX-License-Identifier: GPL-2.0
# ==========================================================================
-# Installing modules
+# Install, Sign & Compress modules
# ==========================================================================
-PHONY := __modinst
-__modinst:
-
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include
-modules := $(call read-file, $(MODORDER))
-
-ifeq ($(KBUILD_EXTMOD),)
-dst := $(MODLIB)/kernel
-else
-INSTALL_MOD_DIR ?= updates
-dst := $(MODLIB)/$(INSTALL_MOD_DIR)
-endif
-
-$(foreach x, % :, $(if $(findstring $x, $(dst)), \
- $(error module installation path cannot contain '$x')))
-
-suffix-y :=
-suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
-suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
-suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
-
-modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
-
-__modinst: $(modules)
- @:
-
-#
-# Installation
-#
-quiet_cmd_install = INSTALL $@
- cmd_install = mkdir -p $(dir $@); cp $< $@
-
-# Strip
-#
-# INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they
-# are installed. If INSTALL_MOD_STRIP is '1', then the default option
-# --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will be used
-# as the options to the strip command.
-ifdef INSTALL_MOD_STRIP
-
-ifeq ($(INSTALL_MOD_STRIP),1)
-strip-option := --strip-debug
-else
-strip-option := $(INSTALL_MOD_STRIP)
-endif
-
-quiet_cmd_strip = STRIP $@
- cmd_strip = $(STRIP) $(strip-option) $@
-
-else
-
-quiet_cmd_strip =
- cmd_strip = :
-
-endif
-
-#
-# Signing
-# Don't stop modules_install even if we can't sign external modules.
-#
-ifeq ($(CONFIG_MODULE_SIG_ALL),y)
-ifeq ($(filter pkcs11:%, $(CONFIG_MODULE_SIG_KEY)),)
-sig-key := $(if $(wildcard $(CONFIG_MODULE_SIG_KEY)),,$(srctree)/)$(CONFIG_MODULE_SIG_KEY)
-else
-sig-key := $(CONFIG_MODULE_SIG_KEY)
-endif
-quiet_cmd_sign = SIGN $@
- cmd_sign = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) "$(sig-key)" certs/signing_key.x509 $@ \
- $(if $(KBUILD_EXTMOD),|| true)
-else
-quiet_cmd_sign :=
- cmd_sign := :
-endif
-
-ifeq ($(modules_sign_only),)
-
-$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
- $(call cmd,install)
- $(call cmd,strip)
- $(call cmd,sign)
-
-else
-
-$(dst)/%.ko: FORCE
- $(call cmd,sign)
-
-endif
-
-#
-# Compression
-#
-quiet_cmd_gzip = GZIP $@
- cmd_gzip = $(KGZIP) -n -f $<
-quiet_cmd_xz = XZ $@
- cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
-quiet_cmd_zstd = ZSTD $@
- cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
-
-$(dst)/%.ko.gz: $(dst)/%.ko FORCE
- $(call cmd,gzip)
-
-$(dst)/%.ko.xz: $(dst)/%.ko FORCE
- $(call cmd,xz)
+PHONY := __modinst
-$(dst)/%.ko.zst: $(dst)/%.ko FORCE
- $(call cmd,zstd)
+__modinst: FORCE
+ $(MAKE) -f scripts/Makefile.install
+ $(MAKE) -f scripts/Makefile.sign
+ $(MAKE) -f scripts/Makefile.compress
PHONY += FORCE
FORCE:
diff --git a/scripts/Makefile.sign b/scripts/Makefile.sign
new file mode 100644
index 000000000000..d6b242b16657
--- /dev/null
+++ b/scripts/Makefile.sign
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: GPL-2.0
+# ==========================================================================
+# Signing modules
+# ==========================================================================
+
+PHONY := __modsign
+__modsign:
+
+include include/config/auto.conf
+include $(srctree)/scripts/Kbuild.include
+
+#
+# Signing
+# Don't stop modules_install even if we can't sign external modules.
+#
+ifeq ($(CONFIG_MODULE_SIG_ALL),y)
+ifeq ($(filter pkcs11:%, $(CONFIG_MODULE_SIG_KEY)),)
+sig-key := $(if $(wildcard $(CONFIG_MODULE_SIG_KEY)),,$(srctree)/)$(CONFIG_MODULE_SIG_KEY)
+else
+sig-key := $(CONFIG_MODULE_SIG_KEY)
+endif
+quiet_cmd_sign = SIGNING ALL MODULES ...
+ cmd_sign = $(CONFIG_SHELL) $(srctree)/scripts/signfile.sh \
+ "$(CONFIG_MODULE_SIG_HASH)" \
+ "$(sig-key)"
+else
+quiet_cmd_sign :=
+ cmd_sign := :
+endif
+
+__modsign: FORCE
+ $(call cmd,sign)
+
+PHONY += FORCE
+FORCE:
+
+.PHONY: $(PHONY)
diff --git a/scripts/signfile.sh b/scripts/signfile.sh
new file mode 100755
index 000000000000..b2b58bfbd5ba
--- /dev/null
+++ b/scripts/signfile.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# A sign-file wrapper used by scripts/Makefile.sign
+
+#set -x
+
+if test $# -ne 2; then
+ echo "Usage: $0 <hash-algo> <sign-key>" >&2
+ exit 1
+fi
+
+SIG_HASH="$1"
+SIG_KEY="$2"
+
+MODULES_PATH="${INSTALL_MOD_PATH}/lib/modules/${KERNELRELEASE}"
+
+find "${MODULES_PATH}" -name *.ko -type f -print0 | \
+ xargs -r -0 -P$(nproc) -x -n32 sh -c "\
+${srctree}/scripts/sign-file \
+-a \"${SIG_HASH}\" \
+-i \"${SIG_KEY}\" \
+-x ${srctree}/certs/signing_key.x509 \
+-b \$@ \$0"
--
2.41.0
On Fri, Jun 23, 2023 at 11:54 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>
> Currently Makefile.modinst does three tasks on each module built:
> - Install modules
> - Sign modules
> - Compress modules
>
> All the above tasks happen from a single place.
>
> This patch divides this task further and uses a different makefile for
> each task.
> Signing module logic is completely refactored and everything happens
> from a shell script now.
>
> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
This patch is bad in multiple ways.
1. Break "make modules_sign"
2. Serialize the installation steps, that is, works less efficiently
3. Increase code without adding any benefits.
There is no good reason to do these changes.
NACK.
> ---
> scripts/Makefile.compress | 53 ++++++++++++++++++
> scripts/Makefile.install | 66 +++++++++++++++++++++++
> scripts/Makefile.modinst | 111 +++-----------------------------------
> scripts/Makefile.sign | 37 +++++++++++++
> scripts/signfile.sh | 24 +++++++++
> 5 files changed, 186 insertions(+), 105 deletions(-)
> create mode 100644 scripts/Makefile.compress
> create mode 100644 scripts/Makefile.install
> create mode 100644 scripts/Makefile.sign
> create mode 100755 scripts/signfile.sh
>
> diff --git a/scripts/Makefile.compress b/scripts/Makefile.compress
> new file mode 100644
> index 000000000000..35d337ac9b6c
> --- /dev/null
> +++ b/scripts/Makefile.compress
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ==========================================================================
> +# Compressing modules
> +# ==========================================================================
> +
> +PHONY := __modcompress
> +__modcompress:
> +
> +include include/config/auto.conf
> +include $(srctree)/scripts/Kbuild.include
> +
> +modules := $(call read-file, $(MODORDER))
> +
> +ifeq ($(KBUILD_EXTMOD),)
> +dst := $(MODLIB)/kernel
> +else
> +INSTALL_MOD_DIR ?= updates
> +dst := $(MODLIB)/$(INSTALL_MOD_DIR)
> +endif
> +
> +suffix-y :=
> +suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
> +suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
> +suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
> +
> +modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
> +
> +__modcompress: $(modules)
> + @:
> +
> +#
> +# Compression
> +#
> +quiet_cmd_gzip = GZIP $@
> + cmd_gzip = $(KGZIP) -n -f $<
> +quiet_cmd_xz = XZ $@
> + cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
> +quiet_cmd_zstd = ZSTD $@
> + cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
> +
> +$(dst)/%.ko.gz: $(dst)/%.ko FORCE
> + $(call cmd,gzip)
> +
> +$(dst)/%.ko.xz: $(dst)/%.ko FORCE
> + $(call cmd,xz)
> +
> +$(dst)/%.ko.zst: $(dst)/%.ko FORCE
> + $(call cmd,zstd)
> +
> +PHONY += FORCE
> +FORCE:
> +
> +.PHONY: $(PHONY)
> diff --git a/scripts/Makefile.install b/scripts/Makefile.install
> new file mode 100644
> index 000000000000..40c496cb99dc
> --- /dev/null
> +++ b/scripts/Makefile.install
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ==========================================================================
> +# Installing modules
> +# ==========================================================================
> +
> +PHONY := __modinstall
> +__modinstall:
> +
> +include include/config/auto.conf
> +include $(srctree)/scripts/Kbuild.include
> +
> +modules := $(call read-file, $(MODORDER))
> +
> +ifeq ($(KBUILD_EXTMOD),)
> +dst := $(MODLIB)/kernel
> +else
> +INSTALL_MOD_DIR ?= updates
> +dst := $(MODLIB)/$(INSTALL_MOD_DIR)
> +endif
> +
> +$(foreach x, % :, $(if $(findstring $x, $(dst)), \
> + $(error module installation path cannot contain '$x')))
> +
> +modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
> +
> +__modinstall: $(modules)
> + @:
> +
> +#
> +# Installation
> +#
> +quiet_cmd_install = INSTALL $@
> + cmd_install = mkdir -p $(dir $@); cp $< $@
> +
> +# Strip
> +#
> +# INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they
> +# are installed. If INSTALL_MOD_STRIP is '1', then the default option
> +# --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will be used
> +# as the options to the strip command.
> +ifdef INSTALL_MOD_STRIP
> +
> +ifeq ($(INSTALL_MOD_STRIP),1)
> +strip-option := --strip-debug
> +else
> +strip-option := $(INSTALL_MOD_STRIP)
> +endif
> +
> +quiet_cmd_strip = STRIP $@
> + cmd_strip = $(STRIP) $(strip-option) $@
> +
> +else
> +
> +quiet_cmd_strip =
> + cmd_strip = :
> +
> +endif
> +
> +$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
> + $(call cmd,install)
> + $(call cmd,strip)
> +
> +PHONY += FORCE
> +FORCE:
> +
> +.PHONY: $(PHONY)
> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
> index ab0c5bd1a60f..d87e09e57963 100644
> --- a/scripts/Makefile.modinst
> +++ b/scripts/Makefile.modinst
> @@ -1,116 +1,17 @@
> # SPDX-License-Identifier: GPL-2.0
> # ==========================================================================
> -# Installing modules
> +# Install, Sign & Compress modules
> # ==========================================================================
>
> -PHONY := __modinst
> -__modinst:
> -
> include include/config/auto.conf
> include $(srctree)/scripts/Kbuild.include
>
> -modules := $(call read-file, $(MODORDER))
> -
> -ifeq ($(KBUILD_EXTMOD),)
> -dst := $(MODLIB)/kernel
> -else
> -INSTALL_MOD_DIR ?= updates
> -dst := $(MODLIB)/$(INSTALL_MOD_DIR)
> -endif
> -
> -$(foreach x, % :, $(if $(findstring $x, $(dst)), \
> - $(error module installation path cannot contain '$x')))
> -
> -suffix-y :=
> -suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
> -suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
> -suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
> -
> -modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
> -
> -__modinst: $(modules)
> - @:
> -
> -#
> -# Installation
> -#
> -quiet_cmd_install = INSTALL $@
> - cmd_install = mkdir -p $(dir $@); cp $< $@
> -
> -# Strip
> -#
> -# INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they
> -# are installed. If INSTALL_MOD_STRIP is '1', then the default option
> -# --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will be used
> -# as the options to the strip command.
> -ifdef INSTALL_MOD_STRIP
> -
> -ifeq ($(INSTALL_MOD_STRIP),1)
> -strip-option := --strip-debug
> -else
> -strip-option := $(INSTALL_MOD_STRIP)
> -endif
> -
> -quiet_cmd_strip = STRIP $@
> - cmd_strip = $(STRIP) $(strip-option) $@
> -
> -else
> -
> -quiet_cmd_strip =
> - cmd_strip = :
> -
> -endif
> -
> -#
> -# Signing
> -# Don't stop modules_install even if we can't sign external modules.
> -#
> -ifeq ($(CONFIG_MODULE_SIG_ALL),y)
> -ifeq ($(filter pkcs11:%, $(CONFIG_MODULE_SIG_KEY)),)
> -sig-key := $(if $(wildcard $(CONFIG_MODULE_SIG_KEY)),,$(srctree)/)$(CONFIG_MODULE_SIG_KEY)
> -else
> -sig-key := $(CONFIG_MODULE_SIG_KEY)
> -endif
> -quiet_cmd_sign = SIGN $@
> - cmd_sign = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) "$(sig-key)" certs/signing_key.x509 $@ \
> - $(if $(KBUILD_EXTMOD),|| true)
> -else
> -quiet_cmd_sign :=
> - cmd_sign := :
> -endif
> -
> -ifeq ($(modules_sign_only),)
> -
> -$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
> - $(call cmd,install)
> - $(call cmd,strip)
> - $(call cmd,sign)
> -
> -else
> -
> -$(dst)/%.ko: FORCE
> - $(call cmd,sign)
> -
> -endif
> -
> -#
> -# Compression
> -#
> -quiet_cmd_gzip = GZIP $@
> - cmd_gzip = $(KGZIP) -n -f $<
> -quiet_cmd_xz = XZ $@
> - cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
> -quiet_cmd_zstd = ZSTD $@
> - cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
> -
> -$(dst)/%.ko.gz: $(dst)/%.ko FORCE
> - $(call cmd,gzip)
> -
> -$(dst)/%.ko.xz: $(dst)/%.ko FORCE
> - $(call cmd,xz)
> +PHONY := __modinst
>
> -$(dst)/%.ko.zst: $(dst)/%.ko FORCE
> - $(call cmd,zstd)
> +__modinst: FORCE
> + $(MAKE) -f scripts/Makefile.install
> + $(MAKE) -f scripts/Makefile.sign
> + $(MAKE) -f scripts/Makefile.compress
>
> PHONY += FORCE
> FORCE:
> diff --git a/scripts/Makefile.sign b/scripts/Makefile.sign
> new file mode 100644
> index 000000000000..d6b242b16657
> --- /dev/null
> +++ b/scripts/Makefile.sign
> @@ -0,0 +1,37 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# ==========================================================================
> +# Signing modules
> +# ==========================================================================
> +
> +PHONY := __modsign
> +__modsign:
> +
> +include include/config/auto.conf
> +include $(srctree)/scripts/Kbuild.include
> +
> +#
> +# Signing
> +# Don't stop modules_install even if we can't sign external modules.
> +#
> +ifeq ($(CONFIG_MODULE_SIG_ALL),y)
> +ifeq ($(filter pkcs11:%, $(CONFIG_MODULE_SIG_KEY)),)
> +sig-key := $(if $(wildcard $(CONFIG_MODULE_SIG_KEY)),,$(srctree)/)$(CONFIG_MODULE_SIG_KEY)
> +else
> +sig-key := $(CONFIG_MODULE_SIG_KEY)
> +endif
> +quiet_cmd_sign = SIGNING ALL MODULES ...
> + cmd_sign = $(CONFIG_SHELL) $(srctree)/scripts/signfile.sh \
> + "$(CONFIG_MODULE_SIG_HASH)" \
> + "$(sig-key)"
> +else
> +quiet_cmd_sign :=
> + cmd_sign := :
> +endif
> +
> +__modsign: FORCE
> + $(call cmd,sign)
> +
> +PHONY += FORCE
> +FORCE:
> +
> +.PHONY: $(PHONY)
> diff --git a/scripts/signfile.sh b/scripts/signfile.sh
> new file mode 100755
> index 000000000000..b2b58bfbd5ba
> --- /dev/null
> +++ b/scripts/signfile.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# A sign-file wrapper used by scripts/Makefile.sign
> +
> +#set -x
> +
> +if test $# -ne 2; then
> + echo "Usage: $0 <hash-algo> <sign-key>" >&2
> + exit 1
> +fi
> +
> +SIG_HASH="$1"
> +SIG_KEY="$2"
> +
> +MODULES_PATH="${INSTALL_MOD_PATH}/lib/modules/${KERNELRELEASE}"
> +
> +find "${MODULES_PATH}" -name *.ko -type f -print0 | \
> + xargs -r -0 -P$(nproc) -x -n32 sh -c "\
> +${srctree}/scripts/sign-file \
> +-a \"${SIG_HASH}\" \
> +-i \"${SIG_KEY}\" \
> +-x ${srctree}/certs/signing_key.x509 \
> +-b \$@ \$0"
> --
> 2.41.0
>
--
Best Regards
Masahiro Yamada
On 07/08/23 01:02, Masahiro Yamada wrote:
> On Fri, Jun 23, 2023 at 11:54 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>>
>> Currently Makefile.modinst does three tasks on each module built:
>> - Install modules
>> - Sign modules
>> - Compress modules
>>
>> All the above tasks happen from a single place.
>>
>> This patch divides this task further and uses a different makefile for
>> each task.
>> Signing module logic is completely refactored and everything happens
>> from a shell script now.
>>
>> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
>
>
> This patch is bad in multiple ways.
>
> 1. Break "make modules_sign"
Correct, somehow I missed it. I will fix it.
I'm using below command to test sign only option. Please let me know if
I should use something else.
make modules_sign modules_sign_only=1 INSTALL_MOD_PATH=$PWD/tmp -j8
> 2. Serialize the installation steps, that is, works less efficiently
Even in the existing system it happens in serially. And the existing
method takes more time than the proposed version.
root@ph5dev:~/linux-6.3.5 # ./test.sh orig
real 0m14.699s
user 0m55.519s
sys 0m9.036s
root@ph5dev:~/linux-6.3.5 # ./test.sh new
real 0m13.327s
user 0m46.885s
sys 0m6.770s
Here is my test script.
```
#!/bin/bash
set -e
if [ "$1" != "new" ] && [ "$1" != "orig" ]; then
echo "invalid arg, ($0 [orig|new])" >&2
exit 1
fi
rm -rf $PWD/tmp
s="scripts/sign-file.c"
m="scripts/Makefile.modinst"
fns=($s $m)
for f in ${fns[@]}; do
cp $f.$1 $f
done
cd scripts
gcc -o sign-file sign-file.c -lcrypto
cd -
time make modules_install INSTALL_MOD_PATH=$PWD/tmp -s -j$(nproc)
```
> 3. Increase code without adding any benefits.
>Agree with increased code but this change is one step closer to Unix
philosophy, do one thing well wrt modules_install.
>
> There is no good reason to do these chang >I hope the data I provided above to your 2nd point provides evidence
that this fix is improving existing system. Please take a look again.
> NACK.
Hi Masahiro Yamada,
Replies inline above.
Please correct me if my understanding is wrong. Thanks a lot for your
time and patience. Have a nice time ahead.
>
>
>
>
>
>> ---
>> scripts/Makefile.compress | 53 ++++++++++++++++++
>> scripts/Makefile.install | 66 +++++++++++++++++++++++
>> scripts/Makefile.modinst | 111 +++-----------------------------------
>> scripts/Makefile.sign | 37 +++++++++++++
>> scripts/signfile.sh | 24 +++++++++
>> 5 files changed, 186 insertions(+), 105 deletions(-)
>> create mode 100644 scripts/Makefile.compress
>> create mode 100644 scripts/Makefile.install
>> create mode 100644 scripts/Makefile.sign
>> create mode 100755 scripts/signfile.sh
>>
>> diff --git a/scripts/Makefile.compress b/scripts/Makefile.compress
>> new file mode 100644
>> index 000000000000..35d337ac9b6c
>> --- /dev/null
>> +++ b/scripts/Makefile.compress
>> @@ -0,0 +1,53 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ==========================================================================
>> +# Compressing modules
>> +# ==========================================================================
>> +
>> +PHONY := __modcompress
>> +__modcompress:
>> +
>> +include include/config/auto.conf
>> +include $(srctree)/scripts/Kbuild.include
>> +
>> +modules := $(call read-file, $(MODORDER))
>> +
>> +ifeq ($(KBUILD_EXTMOD),)
>> +dst := $(MODLIB)/kernel
>> +else
>> +INSTALL_MOD_DIR ?= updates
>> +dst := $(MODLIB)/$(INSTALL_MOD_DIR)
>> +endif
>> +
>> +suffix-y :=
>> +suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
>> +suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
>> +suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
>> +
>> +modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
>> +
>> +__modcompress: $(modules)
>> + @:
>> +
>> +#
>> +# Compression
>> +#
>> +quiet_cmd_gzip = GZIP $@
>> + cmd_gzip = $(KGZIP) -n -f $<
>> +quiet_cmd_xz = XZ $@
>> + cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
>> +quiet_cmd_zstd = ZSTD $@
>> + cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
>> +
>> +$(dst)/%.ko.gz: $(dst)/%.ko FORCE
>> + $(call cmd,gzip)
>> +
>> +$(dst)/%.ko.xz: $(dst)/%.ko FORCE
>> + $(call cmd,xz)
>> +
>> +$(dst)/%.ko.zst: $(dst)/%.ko FORCE
>> + $(call cmd,zstd)
>> +
>> +PHONY += FORCE
>> +FORCE:
>> +
>> +.PHONY: $(PHONY)
>> diff --git a/scripts/Makefile.install b/scripts/Makefile.install
>> new file mode 100644
>> index 000000000000..40c496cb99dc
>> --- /dev/null
>> +++ b/scripts/Makefile.install
>> @@ -0,0 +1,66 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ==========================================================================
>> +# Installing modules
>> +# ==========================================================================
>> +
>> +PHONY := __modinstall
>> +__modinstall:
>> +
>> +include include/config/auto.conf
>> +include $(srctree)/scripts/Kbuild.include
>> +
>> +modules := $(call read-file, $(MODORDER))
>> +
>> +ifeq ($(KBUILD_EXTMOD),)
>> +dst := $(MODLIB)/kernel
>> +else
>> +INSTALL_MOD_DIR ?= updates
>> +dst := $(MODLIB)/$(INSTALL_MOD_DIR)
>> +endif
>> +
>> +$(foreach x, % :, $(if $(findstring $x, $(dst)), \
>> + $(error module installation path cannot contain '$x')))
>> +
>> +modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
>> +
>> +__modinstall: $(modules)
>> + @:
>> +
>> +#
>> +# Installation
>> +#
>> +quiet_cmd_install = INSTALL $@
>> + cmd_install = mkdir -p $(dir $@); cp $< $@
>> +
>> +# Strip
>> +#
>> +# INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they
>> +# are installed. If INSTALL_MOD_STRIP is '1', then the default option
>> +# --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will be used
>> +# as the options to the strip command.
>> +ifdef INSTALL_MOD_STRIP
>> +
>> +ifeq ($(INSTALL_MOD_STRIP),1)
>> +strip-option := --strip-debug
>> +else
>> +strip-option := $(INSTALL_MOD_STRIP)
>> +endif
>> +
>> +quiet_cmd_strip = STRIP $@
>> + cmd_strip = $(STRIP) $(strip-option) $@
>> +
>> +else
>> +
>> +quiet_cmd_strip =
>> + cmd_strip = :
>> +
>> +endif
>> +
>> +$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
>> + $(call cmd,install)
>> + $(call cmd,strip)
>> +
>> +PHONY += FORCE
>> +FORCE:
>> +
>> +.PHONY: $(PHONY)
>> diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
>> index ab0c5bd1a60f..d87e09e57963 100644
>> --- a/scripts/Makefile.modinst
>> +++ b/scripts/Makefile.modinst
>> @@ -1,116 +1,17 @@
>> # SPDX-License-Identifier: GPL-2.0
>> # ==========================================================================
>> -# Installing modules
>> +# Install, Sign & Compress modules
>> # ==========================================================================
>>
>> -PHONY := __modinst
>> -__modinst:
>> -
>> include include/config/auto.conf
>> include $(srctree)/scripts/Kbuild.include
>>
>> -modules := $(call read-file, $(MODORDER))
>> -
>> -ifeq ($(KBUILD_EXTMOD),)
>> -dst := $(MODLIB)/kernel
>> -else
>> -INSTALL_MOD_DIR ?= updates
>> -dst := $(MODLIB)/$(INSTALL_MOD_DIR)
>> -endif
>> -
>> -$(foreach x, % :, $(if $(findstring $x, $(dst)), \
>> - $(error module installation path cannot contain '$x')))
>> -
>> -suffix-y :=
>> -suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
>> -suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
>> -suffix-$(CONFIG_MODULE_COMPRESS_ZSTD) := .zst
>> -
>> -modules := $(patsubst $(extmod_prefix)%.o, $(dst)/%.ko$(suffix-y), $(modules))
>> -
>> -__modinst: $(modules)
>> - @:
>> -
>> -#
>> -# Installation
>> -#
>> -quiet_cmd_install = INSTALL $@
>> - cmd_install = mkdir -p $(dir $@); cp $< $@
>> -
>> -# Strip
>> -#
>> -# INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they
>> -# are installed. If INSTALL_MOD_STRIP is '1', then the default option
>> -# --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will be used
>> -# as the options to the strip command.
>> -ifdef INSTALL_MOD_STRIP
>> -
>> -ifeq ($(INSTALL_MOD_STRIP),1)
>> -strip-option := --strip-debug
>> -else
>> -strip-option := $(INSTALL_MOD_STRIP)
>> -endif
>> -
>> -quiet_cmd_strip = STRIP $@
>> - cmd_strip = $(STRIP) $(strip-option) $@
>> -
>> -else
>> -
>> -quiet_cmd_strip =
>> - cmd_strip = :
>> -
>> -endif
>> -
>> -#
>> -# Signing
>> -# Don't stop modules_install even if we can't sign external modules.
>> -#
>> -ifeq ($(CONFIG_MODULE_SIG_ALL),y)
>> -ifeq ($(filter pkcs11:%, $(CONFIG_MODULE_SIG_KEY)),)
>> -sig-key := $(if $(wildcard $(CONFIG_MODULE_SIG_KEY)),,$(srctree)/)$(CONFIG_MODULE_SIG_KEY)
>> -else
>> -sig-key := $(CONFIG_MODULE_SIG_KEY)
>> -endif
>> -quiet_cmd_sign = SIGN $@
>> - cmd_sign = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) "$(sig-key)" certs/signing_key.x509 $@ \
>> - $(if $(KBUILD_EXTMOD),|| true)
>> -else
>> -quiet_cmd_sign :=
>> - cmd_sign := :
>> -endif
>> -
>> -ifeq ($(modules_sign_only),)
>> -
>> -$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
>> - $(call cmd,install)
>> - $(call cmd,strip)
>> - $(call cmd,sign)
>> -
>> -else
>> -
>> -$(dst)/%.ko: FORCE
>> - $(call cmd,sign)
>> -
>> -endif
>> -
>> -#
>> -# Compression
>> -#
>> -quiet_cmd_gzip = GZIP $@
>> - cmd_gzip = $(KGZIP) -n -f $<
>> -quiet_cmd_xz = XZ $@
>> - cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
>> -quiet_cmd_zstd = ZSTD $@
>> - cmd_zstd = $(ZSTD) -T0 --rm -f -q $<
>> -
>> -$(dst)/%.ko.gz: $(dst)/%.ko FORCE
>> - $(call cmd,gzip)
>> -
>> -$(dst)/%.ko.xz: $(dst)/%.ko FORCE
>> - $(call cmd,xz)
>> +PHONY := __modinst
>>
>> -$(dst)/%.ko.zst: $(dst)/%.ko FORCE
>> - $(call cmd,zstd)
>> +__modinst: FORCE
>> + $(MAKE) -f scripts/Makefile.install
>> + $(MAKE) -f scripts/Makefile.sign
>> + $(MAKE) -f scripts/Makefile.compress
>>
>> PHONY += FORCE
>> FORCE:
>> diff --git a/scripts/Makefile.sign b/scripts/Makefile.sign
>> new file mode 100644
>> index 000000000000..d6b242b16657
>> --- /dev/null
>> +++ b/scripts/Makefile.sign
>> @@ -0,0 +1,37 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +# ==========================================================================
>> +# Signing modules
>> +# ==========================================================================
>> +
>> +PHONY := __modsign
>> +__modsign:
>> +
>> +include include/config/auto.conf
>> +include $(srctree)/scripts/Kbuild.include
>> +
>> +#
>> +# Signing
>> +# Don't stop modules_install even if we can't sign external modules.
>> +#
>> +ifeq ($(CONFIG_MODULE_SIG_ALL),y)
>> +ifeq ($(filter pkcs11:%, $(CONFIG_MODULE_SIG_KEY)),)
>> +sig-key := $(if $(wildcard $(CONFIG_MODULE_SIG_KEY)),,$(srctree)/)$(CONFIG_MODULE_SIG_KEY)
>> +else
>> +sig-key := $(CONFIG_MODULE_SIG_KEY)
>> +endif
>> +quiet_cmd_sign = SIGNING ALL MODULES ...
>> + cmd_sign = $(CONFIG_SHELL) $(srctree)/scripts/signfile.sh \
>> + "$(CONFIG_MODULE_SIG_HASH)" \
>> + "$(sig-key)"
>> +else
>> +quiet_cmd_sign :=
>> + cmd_sign := :
>> +endif
>> +
>> +__modsign: FORCE
>> + $(call cmd,sign)
>> +
>> +PHONY += FORCE
>> +FORCE:
>> +
>> +.PHONY: $(PHONY)
>> diff --git a/scripts/signfile.sh b/scripts/signfile.sh
>> new file mode 100755
>> index 000000000000..b2b58bfbd5ba
>> --- /dev/null
>> +++ b/scripts/signfile.sh
>> @@ -0,0 +1,24 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# A sign-file wrapper used by scripts/Makefile.sign
>> +
>> +#set -x
>> +
>> +if test $# -ne 2; then
>> + echo "Usage: $0 <hash-algo> <sign-key>" >&2
>> + exit 1
>> +fi
>> +
>> +SIG_HASH="$1"
>> +SIG_KEY="$2"
>> +
>> +MODULES_PATH="${INSTALL_MOD_PATH}/lib/modules/${KERNELRELEASE}"
>> +
>> +find "${MODULES_PATH}" -name *.ko -type f -print0 | \
>> + xargs -r -0 -P$(nproc) -x -n32 sh -c "\
>> +${srctree}/scripts/sign-file \
>> +-a \"${SIG_HASH}\" \
>> +-i \"${SIG_KEY}\" \
>> +-x ${srctree}/certs/signing_key.x509 \
>> +-b \$@ \$0"
>> --
>> 2.41.0
>>
>
>
--
Shedi
On Mon, Aug 7, 2023 at 5:08 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>
> On 07/08/23 01:02, Masahiro Yamada wrote:
> > On Fri, Jun 23, 2023 at 11:54 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
> >>
> >> Currently Makefile.modinst does three tasks on each module built:
> >> - Install modules
> >> - Sign modules
> >> - Compress modules
> >>
> >> All the above tasks happen from a single place.
> >>
> >> This patch divides this task further and uses a different makefile for
> >> each task.
> >> Signing module logic is completely refactored and everything happens
> >> from a shell script now.
> >>
> >> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
> >
> >
> > This patch is bad in multiple ways.
> >
> > 1. Break "make modules_sign"
>
> Correct, somehow I missed it. I will fix it.
> I'm using below command to test sign only option. Please let me know if
> I should use something else.
>
> make modules_sign modules_sign_only=1 INSTALL_MOD_PATH=$PWD/tmp -j8
>
> > 2. Serialize the installation steps, that is, works less efficiently
>
> Even in the existing system it happens in serially.
The existing code runs in parallel.
1. Copy the module "foo.ko" to the destination
2. Sign the module "bar.ko"
3. Compress the module "baz.ko"
Those three have no dependency among them, so
should be able to run in parallel.
Your code serializes 1 -> 2 -> 3
> And the existing
> method takes more time than the proposed version.
>
> root@ph5dev:~/linux-6.3.5 # ./test.sh orig
>
> real 0m14.699s
> user 0m55.519s
> sys 0m9.036s
>
> root@ph5dev:~/linux-6.3.5 # ./test.sh new
>
> real 0m13.327s
> user 0m46.885s
> sys 0m6.770s
>
> Here is my test script.
> ```
> #!/bin/bash
>
> set -e
>
> if [ "$1" != "new" ] && [ "$1" != "orig" ]; then
> echo "invalid arg, ($0 [orig|new])" >&2
> exit 1
> fi
>
> rm -rf $PWD/tmp
>
> s="scripts/sign-file.c"
> m="scripts/Makefile.modinst"
> fns=($s $m)
>
> for f in ${fns[@]}; do
> cp $f.$1 $f
> done
>
> cd scripts
> gcc -o sign-file sign-file.c -lcrypto
> cd -
>
> time make modules_install INSTALL_MOD_PATH=$PWD/tmp -s -j$(nproc)
> ```
>
> > 3. Increase code without adding any benefits.
> >Agree with increased code but this change is one step closer to Unix
> philosophy, do one thing well wrt modules_install.
I do not understand why "closer to Unix philosophy"?
You are adding extra/unnecessary complexity.
Currently, the parallel job is managed by Make's job server.
You are introducing another way of parallel execution
in scripts/signfile.sh
(and you completely ignored -j <jobs> option to Make,
and always spawned $(nproc) threads).
Leave the parallel execution GNU Make.
That is how Kbuild works _properly_.
> > There is no good reason to do these chang >I hope the data I provided above to your 2nd point provides evidence
> that this fix is improving existing system. Please take a look again.
I saw it. I re-confirmed this is not an improvement. Thanks for the data.
As I replied to the other thread, my measurement did not show an
attractive result.
https://lore.kernel.org/lkml/CAK7LNATNRchNoj0Y6sdb+_81xwV3kAX57-w5q2zew-q8RyzJVg@mail.gmail.com/T/#m8234fc76e631363fbe6bdfb6e45ef6727fc48e80
>
> > NACK.
>
> Hi Masahiro Yamada,
>
> Replies inline above.
>
> Please correct me if my understanding is wrong. Thanks a lot for your
> time and patience. Have a nice time ahead.
I must let you know you are misunderstanding
the meaning of NACK.
NACK means:
"I do not like it. Please do not submit it any more".
--
Best Regards
Masahiro Yamada
On 08/08/23 00:14, Masahiro Yamada wrote:
> On Mon, Aug 7, 2023 at 5:08 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>>
>> On 07/08/23 01:02, Masahiro Yamada wrote:
>>> On Fri, Jun 23, 2023 at 11:54 PM Shreenidhi Shedi <yesshedi@gmail.com> wrote:
>>>>
>>>> Currently Makefile.modinst does three tasks on each module built:
>>>> - Install modules
>>>> - Sign modules
>>>> - Compress modules
>>>>
>>>> All the above tasks happen from a single place.
>>>>
>>>> This patch divides this task further and uses a different makefile for
>>>> each task.
>>>> Signing module logic is completely refactored and everything happens
>>>> from a shell script now.
>>>>
>>>> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com>
>>>
>>>
>>> This patch is bad in multiple ways.
>>>
>>> 1. Break "make modules_sign"
>>
>> Correct, somehow I missed it. I will fix it.
>> I'm using below command to test sign only option. Please let me know if
>> I should use something else.
>>
>> make modules_sign modules_sign_only=1 INSTALL_MOD_PATH=$PWD/tmp -j8
>>
>>> 2. Serialize the installation steps, that is, works less efficiently
>>
>> Even in the existing system it happens in serially.
>
> The existing code runs in parallel.
>
> 1. Copy the module "foo.ko" to the destination
> 2. Sign the module "bar.ko"
> 3. Compress the module "baz.ko"
>
> Those three have no dependency among them, so
> should be able to run in parallel.
>
> Your code serializes 1 -> 2 -> 3
>
>
>
>> And the existing
>> method takes more time than the proposed version.
>>
>> root@ph5dev:~/linux-6.3.5 # ./test.sh orig
>>
>> real 0m14.699s
>> user 0m55.519s
>> sys 0m9.036s
>>
>> root@ph5dev:~/linux-6.3.5 # ./test.sh new
>>
>> real 0m13.327s
>> user 0m46.885s
>> sys 0m6.770s
>>
>> Here is my test script.
>> ```
>> #!/bin/bash
>>
>> set -e
>>
>> if [ "$1" != "new" ] && [ "$1" != "orig" ]; then
>> echo "invalid arg, ($0 [orig|new])" >&2
>> exit 1
>> fi
>>
>> rm -rf $PWD/tmp
>>
>> s="scripts/sign-file.c"
>> m="scripts/Makefile.modinst"
>> fns=($s $m)
>>
>> for f in ${fns[@]}; do
>> cp $f.$1 $f
>> done
>>
>> cd scripts
>> gcc -o sign-file sign-file.c -lcrypto
>> cd -
>>
>> time make modules_install INSTALL_MOD_PATH=$PWD/tmp -s -j$(nproc)
>> ```
>>
>>> 3. Increase code without adding any benefits.
>>> Agree with increased code but this change is one step closer to Unix
>> philosophy, do one thing well wrt modules_install.
>
>
> I do not understand why "closer to Unix philosophy"?
>
> You are adding extra/unnecessary complexity.
>
> Currently, the parallel job is managed by Make's job server.
>
> You are introducing another way of parallel execution
> in scripts/signfile.sh
> (and you completely ignored -j <jobs> option to Make,
> and always spawned $(nproc) threads).
>
>
> Leave the parallel execution GNU Make.
> That is how Kbuild works _properly_.
>
>
>
>
>>> There is no good reason to do these chang >I hope the data I provided above to your 2nd point provides evidence
>> that this fix is improving existing system. Please take a look again.
>
>
> I saw it. I re-confirmed this is not an improvement. Thanks for the data.
>
> As I replied to the other thread, my measurement did not show an
> attractive result.
> https://lore.kernel.org/lkml/CAK7LNATNRchNoj0Y6sdb+_81xwV3kAX57-w5q2zew-q8RyzJVg@mail.gmail.com/T/#m8234fc76e631363fbe6bdfb6e45ef6727fc48e80
>
>
>>
>>> NACK.
>>
>> Hi Masahiro Yamada,
>>
>> Replies inline above.
>>
>> Please correct me if my understanding is wrong. Thanks a lot for your
>> time and patience. Have a nice time ahead.
>
>
> I must let you know you are misunderstanding
> the meaning of NACK.
>
>
> NACK means:
> "I do not like it. Please do not submit it any more".
>
>
>
> --
> Best Regards
> Masahiro Yamada
Thanks for all the advice and your time, I'm getting used to this
process. I dropped Kbuild changes from my patch series v9. PTAL.
Have a nice time ahead.
--
Shedi
On Fri, Jun 23, 2023 at 08:23:58PM +0530, Shreenidhi Shedi wrote: > Currently Makefile.modinst does three tasks on each module built: > - Install modules > - Sign modules > - Compress modules > > All the above tasks happen from a single place. > > This patch divides this task further and uses a different makefile for > each task. > Signing module logic is completely refactored and everything happens > from a shell script now. > > Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com> > --- > scripts/Makefile.compress | 53 ++++++++++++++++++ > scripts/Makefile.install | 66 +++++++++++++++++++++++ > scripts/Makefile.modinst | 111 +++----------------------------------- > scripts/Makefile.sign | 37 +++++++++++++ > scripts/signfile.sh | 24 +++++++++ > 5 files changed, 186 insertions(+), 105 deletions(-) > create mode 100644 scripts/Makefile.compress > create mode 100644 scripts/Makefile.install > create mode 100644 scripts/Makefile.sign > create mode 100755 scripts/signfile.sh As you are touching the build process, you should always cc: the proper mailing list, and the KBUILD maintainer. Please do so for this series, as that is the proper tree for this to go through. thanks, greg k-h
On 04/08/23 19:36, Greg KH wrote: > On Fri, Jun 23, 2023 at 08:23:58PM +0530, Shreenidhi Shedi wrote: >> Currently Makefile.modinst does three tasks on each module built: >> - Install modules >> - Sign modules >> - Compress modules >> >> All the above tasks happen from a single place. >> >> This patch divides this task further and uses a different makefile for >> each task. >> Signing module logic is completely refactored and everything happens >> from a shell script now. >> >> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com> >> --- >> scripts/Makefile.compress | 53 ++++++++++++++++++ >> scripts/Makefile.install | 66 +++++++++++++++++++++++ >> scripts/Makefile.modinst | 111 +++----------------------------------- >> scripts/Makefile.sign | 37 +++++++++++++ >> scripts/signfile.sh | 24 +++++++++ >> 5 files changed, 186 insertions(+), 105 deletions(-) >> create mode 100644 scripts/Makefile.compress >> create mode 100644 scripts/Makefile.install >> create mode 100644 scripts/Makefile.sign >> create mode 100755 scripts/signfile.sh > > As you are touching the build process, you should always cc: the proper > mailing list, and the KBUILD maintainer. Please do so for this series, > as that is the proper tree for this to go through. > > thanks, > > greg k-h Thanks for the inputs Greg. CC-ing linux-kbuild@vger.kernel.org as suggested. -- Shedi
On Sun, Aug 06, 2023 at 12:30:22AM +0530, Shreenidhi Shedi wrote: > On 04/08/23 19:36, Greg KH wrote: > > On Fri, Jun 23, 2023 at 08:23:58PM +0530, Shreenidhi Shedi wrote: > > > Currently Makefile.modinst does three tasks on each module built: > > > - Install modules > > > - Sign modules > > > - Compress modules > > > > > > All the above tasks happen from a single place. > > > > > > This patch divides this task further and uses a different makefile for > > > each task. > > > Signing module logic is completely refactored and everything happens > > > from a shell script now. > > > > > > Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com> > > > --- > > > scripts/Makefile.compress | 53 ++++++++++++++++++ > > > scripts/Makefile.install | 66 +++++++++++++++++++++++ > > > scripts/Makefile.modinst | 111 +++----------------------------------- > > > scripts/Makefile.sign | 37 +++++++++++++ > > > scripts/signfile.sh | 24 +++++++++ > > > 5 files changed, 186 insertions(+), 105 deletions(-) > > > create mode 100644 scripts/Makefile.compress > > > create mode 100644 scripts/Makefile.install > > > create mode 100644 scripts/Makefile.sign > > > create mode 100755 scripts/signfile.sh > > > > As you are touching the build process, you should always cc: the proper > > mailing list, and the KBUILD maintainer. Please do so for this series, > > as that is the proper tree for this to go through. > > > > thanks, > > > > greg k-h > > Thanks for the inputs Greg. > > CC-ing linux-kbuild@vger.kernel.org as suggested. This doesn't actually do anything, sorry. Please resend the whole patchset again and add the proper people and list. thanks, greg k-h
On 06/08/23 12:15, Greg KH wrote: > On Sun, Aug 06, 2023 at 12:30:22AM +0530, Shreenidhi Shedi wrote: >> On 04/08/23 19:36, Greg KH wrote: >>> On Fri, Jun 23, 2023 at 08:23:58PM +0530, Shreenidhi Shedi wrote: >>>> Currently Makefile.modinst does three tasks on each module built: >>>> - Install modules >>>> - Sign modules >>>> - Compress modules >>>> >>>> All the above tasks happen from a single place. >>>> >>>> This patch divides this task further and uses a different makefile for >>>> each task. >>>> Signing module logic is completely refactored and everything happens >>>> from a shell script now. >>>> >>>> Signed-off-by: Shreenidhi Shedi <yesshedi@gmail.com> >>>> --- >>>> scripts/Makefile.compress | 53 ++++++++++++++++++ >>>> scripts/Makefile.install | 66 +++++++++++++++++++++++ >>>> scripts/Makefile.modinst | 111 +++----------------------------------- >>>> scripts/Makefile.sign | 37 +++++++++++++ >>>> scripts/signfile.sh | 24 +++++++++ >>>> 5 files changed, 186 insertions(+), 105 deletions(-) >>>> create mode 100644 scripts/Makefile.compress >>>> create mode 100644 scripts/Makefile.install >>>> create mode 100644 scripts/Makefile.sign >>>> create mode 100755 scripts/signfile.sh >>> >>> As you are touching the build process, you should always cc: the proper >>> mailing list, and the KBUILD maintainer. Please do so for this series, >>> as that is the proper tree for this to go through. >>> >>> thanks, >>> >>> greg k-h >> >> Thanks for the inputs Greg. >> >> CC-ing linux-kbuild@vger.kernel.org as suggested. > > This doesn't actually do anything, sorry. Please resend the whole > patchset again and add the proper people and list. > > thanks, > > greg k-h Done. Addressed comments from Masahiro Yamada and sent a new patch series, hopefully I have added everyone this time :) Thanks. -- Shedi
© 2016 - 2026 Red Hat, Inc.