2021-03-31 13:40:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 1/9] kbuild: remove unneeded mkdir for external modules_install

scripts/Makefile.modinst creates directories as needed.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 2 --
1 file changed, 2 deletions(-)

diff --git a/Makefile b/Makefile
index ed8bd815e8a3..0e06db5ed9d8 100644
--- a/Makefile
+++ b/Makefile
@@ -1779,10 +1779,8 @@ $(MODORDER): descend
PHONY += modules_install
modules_install: _emodinst_ _emodinst_post

-install-dir := $(if $(INSTALL_MOD_DIR),$(INSTALL_MOD_DIR),extra)
PHONY += _emodinst_
_emodinst_:
- $(Q)mkdir -p $(MODLIB)/$(install-dir)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst

PHONY += _emodinst_post
--
2.27.0


2021-03-31 13:40:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 7/9] kbuild: move module strip/compression code into scripts/Makefile.modinst

Both mod_strip_cmd and mod_compress_cmd are only used in
scripts/Makefile.modinst, hence there is no good reason to define them
in the top Makefile. Move the relevant code to scripts/Makefile.modinst.

Also, show separate log messages for each of install, strip, sign, and
compress.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 32 -----------------
scripts/Makefile.modinst | 76 +++++++++++++++++++++++++++++++++++-----
2 files changed, 68 insertions(+), 40 deletions(-)

diff --git a/Makefile b/Makefile
index 88e5c15e1186..f96ae09d111b 100644
--- a/Makefile
+++ b/Makefile
@@ -1063,38 +1063,6 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
export MODLIB

-#
-# 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)
-mod_strip_cmd = $(STRIP) --strip-debug
-else
-mod_strip_cmd = $(STRIP) $(INSTALL_MOD_STRIP)
-endif # INSTALL_MOD_STRIP=1
-else
-mod_strip_cmd = true
-endif # INSTALL_MOD_STRIP
-export mod_strip_cmd
-
-# CONFIG_MODULE_COMPRESS, if defined, will cause module to be compressed
-# after they are installed in agreement with CONFIG_MODULE_COMPRESS_GZIP
-# or CONFIG_MODULE_COMPRESS_XZ.
-
-mod_compress_cmd = true
-ifdef CONFIG_MODULE_COMPRESS
- ifdef CONFIG_MODULE_COMPRESS_GZIP
- mod_compress_cmd = $(KGZIP) -n -f
- endif # CONFIG_MODULE_COMPRESS_GZIP
- ifdef CONFIG_MODULE_COMPRESS_XZ
- mod_compress_cmd = $(XZ) --lzma2=dict=2MiB -f
- endif # CONFIG_MODULE_COMPRESS_XZ
-endif # CONFIG_MODULE_COMPRESS
-export mod_compress_cmd
-
ifdef CONFIG_MODULE_SIG_ALL
$(eval $(call config_filename,MODULE_SIG_KEY))

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index 3b2d0380504d..943806b0abb5 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -6,6 +6,7 @@
PHONY := __modinst
__modinst:

+include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

modules := $(sort $(shell cat $(MODORDER)))
@@ -19,21 +20,80 @@ endif

dst := $(MODLIB)/$(subdir)

-modules := $(patsubst $(extmod_prefix)%, $(dst)/%, $(modules))
+suffix-y :=
+suffix-$(CONFIG_MODULE_COMPRESS_GZIP) := .gz
+suffix-$(CONFIG_MODULE_COMPRESS_XZ) := .xz
+
+modules := $(patsubst $(extmod_prefix)%, $(dst)/%$(suffix-y), $(modules))

__modinst: $(modules)
@:

-# Don't stop modules_install if we can't sign external modules.
+quiet_cmd_none =
+ cmd_none = :
+
+#
+# Installation
+#
quiet_cmd_install = INSTALL $@
- cmd_install = \
- mkdir -p $(dir $@); cp $< $@; \
- $(mod_strip_cmd) $@; \
- $(mod_sign_cmd) $@ $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
- $(mod_compress_cmd) $@
+ 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

-$(modules): $(dst)/%: $(extmod_prefix)% FORCE
+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)
+quiet_cmd_sign = SIGN $@
+$(eval $(call config_filename,MODULE_SIG_KEY))
+ cmd_sign = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY) certs/signing_key.x509 $@ \
+ $(if $(KBUILD_EXTMOD),|| true)
+else
+quiet_cmd_sign :=
+ cmd_sign := :
+endif
+
+$(dst)/%.ko: $(extmod_prefix)%.ko FORCE
$(call cmd,install)
+ $(call cmd,strip)
+ $(call cmd,sign)
+
+#
+# Compression
+#
+quiet_cmd_gzip = GZIP $@
+ cmd_gzip = $(KGZIP) -n -f $<
+quiet_cmd_xz = XZ $@
+ cmd_xz = $(XZ) --lzma2=dict=2MiB -f $<
+
+$(dst)/%.ko.gz: $(dst)/%.ko FORCE
+ $(call cmd,gzip)
+
+$(dst)/%.ko.xz: $(dst)/%.ko FORCE
+ $(call cmd,xz)

PHONY += FORCE
FORCE:
--
2.27.0

2021-03-31 13:40:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 4/9] kbuild: check module name conflict for external modules as well

If there are multiple modules with the same name in the same external
module tree, there is ambiguity about which one will be loaded, and
very likely something odd is happening.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 10 +++++-----
scripts/modules-check.sh | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index a6f73335757d..b5ff4753eba8 100644
--- a/Makefile
+++ b/Makefile
@@ -1459,10 +1459,6 @@ endif
PHONY += modules
modules: $(if $(KBUILD_BUILTIN),vmlinux) modules_check modules_prepare

-PHONY += modules_check
-modules_check: modules.order
- $(Q)$(CONFIG_SHELL) $(srctree)/scripts/modules-check.sh $<
-
cmd_modules_order = $(AWK) '!x[$$0]++' $(real-prereqs) > $@

modules.order: $(subdir-modorder) FORCE
@@ -1775,9 +1771,13 @@ PHONY += modules modules_install

ifdef CONFIG_MODULES

-modules: $(MODORDER)
+modules: modules_check
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost

+PHONY += modules_check
+modules_check: $(MODORDER)
+ $(Q)$(CONFIG_SHELL) $(srctree)/scripts/modules-check.sh $<
+
quiet_cmd_depmod = DEPMOD $(MODLIB)
cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
$(KERNELRELEASE)
diff --git a/scripts/modules-check.sh b/scripts/modules-check.sh
index 43de226071ae..e06327722263 100755
--- a/scripts/modules-check.sh
+++ b/scripts/modules-check.sh
@@ -13,10 +13,10 @@ exit_code=0
# Check uniqueness of module names
check_same_name_modules()
{
- for m in $(sed 's:.*/::' $1 | sort | uniq -d)
+ for m in $(sed 's:.*/::' "$1" | sort | uniq -d)
do
echo "error: the following would cause module name conflict:" >&2
- sed -n "/\/$m/s:^: :p" modules.order >&2
+ sed -n "/\/$m/s:^: :p" "$1" >&2
exit_code=1
done
}
--
2.27.0

2021-03-31 13:40:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 2/9] kbuild: unify modules(_install) for in-tree and external modules

If you attempt to build/install modules ('make modules(_install)' with
CONFIG_MODULES disabled, you will get a clear error message, but nothing
for external module builds.

Factor out the modules and modules_install rules into the common part,
then you will get the same error message when you try to build external
modules with CONFIG_MODULES=n.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 85 ++++++++++++++++++++++++--------------------------------
1 file changed, 36 insertions(+), 49 deletions(-)

diff --git a/Makefile b/Makefile
index 0e06db5ed9d8..99a2bd51c02d 100644
--- a/Makefile
+++ b/Makefile
@@ -1458,7 +1458,6 @@ endif

PHONY += modules
modules: $(if $(KBUILD_BUILTIN),vmlinux) modules_check modules_prepare
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost

PHONY += modules_check
modules_check: modules.order
@@ -1476,12 +1475,9 @@ PHONY += modules_prepare
modules_prepare: prepare
$(Q)$(MAKE) $(build)=scripts scripts/module.lds

-# Target to install modules
-PHONY += modules_install
-modules_install: _modinst_ _modinst_post
-
-PHONY += _modinst_
-_modinst_:
+modules_install: __modinst_pre
+PHONY += __modinst_pre
+__modinst_pre:
@rm -rf $(MODLIB)/kernel
@rm -f $(MODLIB)/source
@mkdir -p $(MODLIB)/kernel
@@ -1493,14 +1489,6 @@ _modinst_:
@sed 's:^:kernel/:' modules.order > $(MODLIB)/modules.order
@cp -f modules.builtin $(MODLIB)/
@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
-
-# This depmod is only for convenience to give the initial
-# boot a modules.dep even before / is mounted read-write. However the
-# boot script depmod is the master version.
-PHONY += _modinst_post
-_modinst_post: _modinst_
- $(call cmd,depmod)

ifeq ($(CONFIG_MODULE_SIG), y)
PHONY += modules_sign
@@ -1508,20 +1496,6 @@ modules_sign:
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modsign
endif

-else # CONFIG_MODULES
-
-# Modules not configured
-# ---------------------------------------------------------------------------
-
-PHONY += modules modules_install
-modules modules_install:
- @echo >&2
- @echo >&2 "The present kernel configuration has modules disabled."
- @echo >&2 "Type 'make config' and enable loadable module support."
- @echo >&2 "Then build a kernel with module support enabled."
- @echo >&2
- @exit 1
-
endif # CONFIG_MODULES

###
@@ -1769,24 +1743,9 @@ KBUILD_BUILTIN :=
KBUILD_MODULES := 1

build-dirs := $(KBUILD_EXTMOD)
-PHONY += modules
-modules: $(MODORDER)
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
-
$(MODORDER): descend
@:

-PHONY += modules_install
-modules_install: _emodinst_ _emodinst_post
-
-PHONY += _emodinst_
-_emodinst_:
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
-
-PHONY += _emodinst_post
-_emodinst_post: _emodinst_
- $(call cmd,depmod)
-
compile_commands.json: $(extmod-prefix)compile_commands.json
PHONY += compile_commands.json

@@ -1809,6 +1768,39 @@ PHONY += prepare modules_prepare

endif # KBUILD_EXTMOD

+# ---------------------------------------------------------------------------
+# Modules
+
+PHONY += modules modules_install
+
+ifdef CONFIG_MODULES
+
+modules: $(MODORDER)
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
+
+quiet_cmd_depmod = DEPMOD $(KERNELRELEASE)
+ cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
+ $(KERNELRELEASE)
+
+modules_install:
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
+ $(call cmd,depmod)
+
+else # CONFIG_MODULES
+
+# Modules not configured
+# ---------------------------------------------------------------------------
+
+modules modules_install:
+ @echo >&2 '***'
+ @echo >&2 '*** The present kernel configuration has modules disabled.'
+ @echo >&2 '*** To use the module feature, please run "make menuconfig" etc.'
+ @echo >&2 '*** to enable CONFIG_MODULES.'
+ @echo >&2 '***'
+ @exit 1
+
+endif # CONFIG_MODULES
+
# Single targets
# ---------------------------------------------------------------------------
# To build individual files in subdirectories, you can do like this:
@@ -1997,11 +1989,6 @@ tools/%: FORCE
quiet_cmd_rmfiles = $(if $(wildcard $(rm-files)),CLEAN $(wildcard $(rm-files)))
cmd_rmfiles = rm -rf $(rm-files)

-# Run depmod only if we have System.map and depmod is executable
-quiet_cmd_depmod = DEPMOD $(KERNELRELEASE)
- cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
- $(KERNELRELEASE)
-
# read saved command lines for existing targets
existing-targets := $(wildcard $(sort $(targets)))

--
2.27.0

2021-03-31 13:40:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 3/9] kbuild: show the target directory for depmod log

It is clearer to show the directory which depmod will work on.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 99a2bd51c02d..a6f73335757d 100644
--- a/Makefile
+++ b/Makefile
@@ -1778,7 +1778,7 @@ ifdef CONFIG_MODULES
modules: $(MODORDER)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost

-quiet_cmd_depmod = DEPMOD $(KERNELRELEASE)
+quiet_cmd_depmod = DEPMOD $(MODLIB)
cmd_depmod = $(CONFIG_SHELL) $(srctree)/scripts/depmod.sh $(DEPMOD) \
$(KERNELRELEASE)

--
2.27.0

2021-03-31 13:40:14

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 6/9] kbuild: refactor scripts/Makefile.modinst

scripts/Makefile.modinst is ugly and weird in multiple ways; it
specifies real files $(modules) as phony, makes directory manipulation
needlessly too complicated.

Clean up the Makefile code, and show the full path of installed modules
in the log.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 2 +-
scripts/Makefile.modinst | 42 +++++++++++++++++++++++-----------------
2 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/Makefile b/Makefile
index e3c2bd1b6f42..88e5c15e1186 100644
--- a/Makefile
+++ b/Makefile
@@ -1141,7 +1141,7 @@ endif # CONFIG_BPF

PHONY += prepare0

-extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
+export extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
export MODORDER := $(extmod_prefix)modules.order
export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps

diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index ad1981233d0b..3b2d0380504d 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -8,28 +8,34 @@ __modinst:

include $(srctree)/scripts/Kbuild.include

-modules := $(sort $(shell cat $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)modules.order))
+modules := $(sort $(shell cat $(MODORDER)))
+
+ifeq ($(KBUILD_EXTMOD),)
+subdir := kernel
+else
+INSTALL_MOD_DIR ?= extra
+subdir := $(INSTALL_MOD_DIR)
+endif
+
+dst := $(MODLIB)/$(subdir)
+
+modules := $(patsubst $(extmod_prefix)%, $(dst)/%, $(modules))

-PHONY += $(modules)
__modinst: $(modules)
@:

# Don't stop modules_install if we can't sign external modules.
-quiet_cmd_modules_install = INSTALL $@
- cmd_modules_install = \
- mkdir -p $(2) ; \
- cp $@ $(2) ; \
- $(mod_strip_cmd) $(2)/$(notdir $@) ; \
- $(mod_sign_cmd) $(2)/$(notdir $@) $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
- $(mod_compress_cmd) $(2)/$(notdir $@)
-
-# Modules built outside the kernel source tree go into extra by default
-INSTALL_MOD_DIR ?= extra
-ext-mod-dir = $(INSTALL_MOD_DIR)$(subst $(patsubst %/,%,$(KBUILD_EXTMOD)),,$(@D))
-
-modinst_dir = $(if $(KBUILD_EXTMOD),$(ext-mod-dir),kernel/$(@D))
-
-$(modules):
- $(call cmd,modules_install,$(MODLIB)/$(modinst_dir))
+quiet_cmd_install = INSTALL $@
+ cmd_install = \
+ mkdir -p $(dir $@); cp $< $@; \
+ $(mod_strip_cmd) $@; \
+ $(mod_sign_cmd) $@ $(patsubst %,|| true,$(KBUILD_EXTMOD)) ; \
+ $(mod_compress_cmd) $@
+
+$(modules): $(dst)/%: $(extmod_prefix)% FORCE
+ $(call cmd,install)
+
+PHONY += FORCE
+FORCE:

.PHONY: $(PHONY)
--
2.27.0

2021-03-31 13:40:42

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 8/9] kbuild: merge scripts/Makefile.modsign to scripts/Makefile.modinst

scripts/Makefile.modsign is a subset of scripts/Makefile.modinst,
and duplicates the code. Let's merge them.

By the way, you do not need to run 'make modules_sign' explicitly
because modules are signed as a part of 'make modules_install' when
CONFIG_MODULE_SIG_ALL=y. If CONFIG_MODULE_SIG_ALL=n, mod_sign_cmd is
set to 'true', so 'make modules_sign' is not functional.

In my understanding, the reason of still keeping this is to handle
corner cases like commit 64178cb62c32 ("builddeb: fix stripped module
signatures if CONFIG_DEBUG_INFO and CONFIG_MODULE_SIG_ALL are set").

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 36 ++++++++++++++++++++----------------
scripts/Makefile.modinst | 9 +++++++++
scripts/Makefile.modsign | 29 -----------------------------
3 files changed, 29 insertions(+), 45 deletions(-)
delete mode 100644 scripts/Makefile.modsign

diff --git a/Makefile b/Makefile
index f96ae09d111b..b14483742a67 100644
--- a/Makefile
+++ b/Makefile
@@ -1063,15 +1063,6 @@ export INSTALL_DTBS_PATH ?= $(INSTALL_PATH)/dtbs/$(KERNELRELEASE)
MODLIB = $(INSTALL_MOD_PATH)/lib/modules/$(KERNELRELEASE)
export MODLIB

-ifdef CONFIG_MODULE_SIG_ALL
-$(eval $(call config_filename,MODULE_SIG_KEY))
-
-mod_sign_cmd = scripts/sign-file $(CONFIG_MODULE_SIG_HASH) $(MODULE_SIG_KEY_SRCPREFIX)$(CONFIG_MODULE_SIG_KEY) certs/signing_key.x509
-else
-mod_sign_cmd = true
-endif
-export mod_sign_cmd
-
HOST_LIBELF_LIBS = $(shell pkg-config libelf --libs 2>/dev/null || echo -lelf)

has_libelf = $(call try-run,\
@@ -1439,7 +1430,26 @@ PHONY += modules_prepare
modules_prepare: prepare
$(Q)$(MAKE) $(build)=scripts scripts/module.lds

-modules_install: __modinst_pre
+export modules_sign_only :=
+
+ifeq ($(CONFIG_MODULE_SIG),y)
+PHONY += modules_sign
+modules_sign: modules_install
+ @:
+
+# modules_sign is a subset of modules_install.
+# 'make modules_install modules_sign' is equivalent to 'make modules_install'.
+ifeq ($(filter modules_install,$(MAKECMDGOALS)),)
+modules_sign_only := y
+endif
+endif
+
+modinst_pre :=
+ifneq ($(filter modules_install,$(MAKECMDGOALS)),)
+modinst_pre := __modinst_pre
+endif
+
+modules_install: $(modinst_pre)
PHONY += __modinst_pre
__modinst_pre:
@rm -rf $(MODLIB)/kernel
@@ -1454,12 +1464,6 @@ __modinst_pre:
@cp -f modules.builtin $(MODLIB)/
@cp -f $(objtree)/modules.builtin.modinfo $(MODLIB)/

-ifeq ($(CONFIG_MODULE_SIG), y)
-PHONY += modules_sign
-modules_sign:
- $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modsign
-endif
-
endif # CONFIG_MODULES

###
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index 943806b0abb5..156eb8239abc 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -76,11 +76,20 @@ 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
#
diff --git a/scripts/Makefile.modsign b/scripts/Makefile.modsign
deleted file mode 100644
index ddf9b5ca77d7..000000000000
--- a/scripts/Makefile.modsign
+++ /dev/null
@@ -1,29 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-# ==========================================================================
-# Signing modules
-# ==========================================================================
-
-PHONY := __modsign
-__modsign:
-
-include $(srctree)/scripts/Kbuild.include
-
-modules := $(sort $(shell cat modules.order))
-
-PHONY += $(modules)
-__modsign: $(modules)
- @:
-
-quiet_cmd_sign_ko = SIGN [M] $(2)/$(notdir $@)
- cmd_sign_ko = $(mod_sign_cmd) $(2)/$(notdir $@)
-
-# Modules built outside the kernel source tree go into extra by default
-INSTALL_MOD_DIR ?= extra
-ext-mod-dir = $(INSTALL_MOD_DIR)$(subst $(patsubst %/,%,$(KBUILD_EXTMOD)),,$(@D))
-
-modinst_dir = $(if $(KBUILD_EXTMOD),$(ext-mod-dir),kernel/$(@D))
-
-$(modules):
- $(call cmd,sign_ko,$(MODLIB)/$(modinst_dir))
-
-.PHONY: $(PHONY)
--
2.27.0

2021-03-31 13:40:44

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 5/9] kbuild: rename extmod-prefix to extmod_prefix

This seems to be useful in sub-make as well. As a preparation of
exporting it, rename extmod-prefix to extmod_prefix because exported
variables cannot contain hyphens.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Makefile | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index b5ff4753eba8..e3c2bd1b6f42 100644
--- a/Makefile
+++ b/Makefile
@@ -919,7 +919,7 @@ endif
ifdef CONFIG_LTO_CLANG
ifdef CONFIG_LTO_CLANG_THIN
CC_FLAGS_LTO := -flto=thin -fsplit-lto-unit
-KBUILD_LDFLAGS += --thinlto-cache-dir=$(extmod-prefix).thinlto-cache
+KBUILD_LDFLAGS += --thinlto-cache-dir=$(extmod_prefix).thinlto-cache
else
CC_FLAGS_LTO := -flto
endif
@@ -1141,9 +1141,9 @@ endif # CONFIG_BPF

PHONY += prepare0

-extmod-prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
-export MODORDER := $(extmod-prefix)modules.order
-export MODULES_NSDEPS := $(extmod-prefix)modules.nsdeps
+extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
+export MODORDER := $(extmod_prefix)modules.order
+export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps

ifeq ($(KBUILD_EXTMOD),)
core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
@@ -1742,7 +1742,7 @@ build-dirs := $(KBUILD_EXTMOD)
$(MODORDER): descend
@:

-compile_commands.json: $(extmod-prefix)compile_commands.json
+compile_commands.json: $(extmod_prefix)compile_commands.json
PHONY += compile_commands.json

clean-dirs := $(KBUILD_EXTMOD)
@@ -1832,12 +1832,12 @@ endif

PHONY += single_modpost
single_modpost: $(single-no-ko) modules_prepare
- $(Q){ $(foreach m, $(single-ko), echo $(extmod-prefix)$m;) } > $(MODORDER)
+ $(Q){ $(foreach m, $(single-ko), echo $(extmod_prefix)$m;) } > $(MODORDER)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost

KBUILD_MODULES := 1

-export KBUILD_SINGLE_TARGETS := $(addprefix $(extmod-prefix), $(single-no-ko))
+export KBUILD_SINGLE_TARGETS := $(addprefix $(extmod_prefix), $(single-no-ko))

# trim unrelated directories
build-dirs := $(foreach d, $(build-dirs), \
@@ -1906,12 +1906,12 @@ nsdeps: modules
quiet_cmd_gen_compile_commands = GEN $@
cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<, $(real-prereqs))

-$(extmod-prefix)compile_commands.json: scripts/clang-tools/gen_compile_commands.py \
+$(extmod_prefix)compile_commands.json: scripts/clang-tools/gen_compile_commands.py \
$(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
$(if $(CONFIG_MODULES), $(MODORDER)) FORCE
$(call if_changed,gen_compile_commands)

-targets += $(extmod-prefix)compile_commands.json
+targets += $(extmod_prefix)compile_commands.json

PHONY += clang-tidy clang-analyzer

@@ -1919,7 +1919,7 @@ ifdef CONFIG_CC_IS_CLANG
quiet_cmd_clang_tools = CHECK $<
cmd_clang_tools = $(PYTHON3) $(srctree)/scripts/clang-tools/run-clang-tools.py $@ $<

-clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
+clang-tidy clang-analyzer: $(extmod_prefix)compile_commands.json
$(call cmd,clang_tools)
else
clang-tidy clang-analyzer:
--
2.27.0

2021-03-31 13:43:36

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH 9/9] kbuild: remove CONFIG_MODULE_COMPRESS

CONFIG_MODULE_COMPRESS is only used to activate the choice for module
compression algorithm. It will be simpler to make the choice visible
all the time by adding CONFIG_MODULE_COMPRESS_NONE to allow the user to
disable module compression.

This is more consistent with the "Kernel compression mode" and "Built-in
initramfs compression mode" choices.

CONFIG_KERNEL_UNCOMPRESSED and CONFIG_INITRAMFS_COMPRESSION_NONE are
available to choose to not compress the kernel, initrd, respectively.

Signed-off-by: Masahiro Yamada <[email protected]>
---

init/Kconfig | 45 ++++++++++++++++++++++++++-------------------
1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 019c1874e609..3ca1ffd219c4 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2225,40 +2225,47 @@ config MODULE_SIG_HASH
default "sha384" if MODULE_SIG_SHA384
default "sha512" if MODULE_SIG_SHA512

-config MODULE_COMPRESS
- bool "Compress modules on installation"
+choice
+ prompt "Module compression mode"
help
+ This option allows you to choose the algorithm which will be used to
+ compress modules when 'make modules_install' is run. (or, you can
+ choose to not compress modules at all.)

- Compresses kernel modules when 'make modules_install' is run; gzip or
- xz depending on "Compression algorithm" below.
+ External modules will also be compressed in the same way during the
+ installation.

- module-init-tools MAY support gzip, and kmod MAY support gzip and xz.
+ For modules inside an initrd or initramfs, it's more efficient to
+ compress the whole initrd or initramfs instead.

- Out-of-tree kernel modules installed using Kbuild will also be
- compressed upon installation.
+ This is fully compatible with signed modules.

- Note: for modules inside an initrd or initramfs, it's more efficient
- to compress the whole initrd or initramfs instead.
+ Please note that the tool used to load modules needs to support the
+ corresponding algorithm. module-init-tools MAY support gzip, and kmod
+ MAY support gzip and xz.

- Note: This is fully compatible with signed modules.
+ Your build system needs to provide the appropriate compression tool
+ to compress the modules.

- If in doubt, say N.
+ If in doubt, select 'None'.

-choice
- prompt "Compression algorithm"
- depends on MODULE_COMPRESS
- default MODULE_COMPRESS_GZIP
+config MODULE_COMPRESS_NONE
+ bool "None"
help
- This determines which sort of compression will be used during
- 'make modules_install'.
-
- GZIP (default) and XZ are supported.
+ Do not compress modules. The installed modules are suffixed
+ with .ko.

config MODULE_COMPRESS_GZIP
bool "GZIP"
+ help
+ Compress modules with XZ. The installed modules are suffixed
+ with .ko.gz.

config MODULE_COMPRESS_XZ
bool "XZ"
+ help
+ Compress modules with XZ. The installed modules are suffixed
+ with .ko.xz.

endchoice

--
2.27.0

2021-03-31 17:56:15

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 5/9] kbuild: rename extmod-prefix to extmod_prefix

On Wed, Mar 31, 2021 at 6:38 AM Masahiro Yamada <[email protected]> wrote:
>
> This seems to be useful in sub-make as well. As a preparation of
> exporting it, rename extmod-prefix to extmod_prefix because exported
> variables cannot contain hyphens.
>
> Signed-off-by: Masahiro Yamada <[email protected]>

Reviewed-by: Nick Desaulniers <[email protected]>

> ---
>
> Makefile | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b5ff4753eba8..e3c2bd1b6f42 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -919,7 +919,7 @@ endif
> ifdef CONFIG_LTO_CLANG
> ifdef CONFIG_LTO_CLANG_THIN
> CC_FLAGS_LTO := -flto=thin -fsplit-lto-unit
> -KBUILD_LDFLAGS += --thinlto-cache-dir=$(extmod-prefix).thinlto-cache
> +KBUILD_LDFLAGS += --thinlto-cache-dir=$(extmod_prefix).thinlto-cache
> else
> CC_FLAGS_LTO := -flto
> endif
> @@ -1141,9 +1141,9 @@ endif # CONFIG_BPF
>
> PHONY += prepare0
>
> -extmod-prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
> -export MODORDER := $(extmod-prefix)modules.order
> -export MODULES_NSDEPS := $(extmod-prefix)modules.nsdeps
> +extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
> +export MODORDER := $(extmod_prefix)modules.order
> +export MODULES_NSDEPS := $(extmod_prefix)modules.nsdeps
>
> ifeq ($(KBUILD_EXTMOD),)
> core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
> @@ -1742,7 +1742,7 @@ build-dirs := $(KBUILD_EXTMOD)
> $(MODORDER): descend
> @:
>
> -compile_commands.json: $(extmod-prefix)compile_commands.json
> +compile_commands.json: $(extmod_prefix)compile_commands.json
> PHONY += compile_commands.json
>
> clean-dirs := $(KBUILD_EXTMOD)
> @@ -1832,12 +1832,12 @@ endif
>
> PHONY += single_modpost
> single_modpost: $(single-no-ko) modules_prepare
> - $(Q){ $(foreach m, $(single-ko), echo $(extmod-prefix)$m;) } > $(MODORDER)
> + $(Q){ $(foreach m, $(single-ko), echo $(extmod_prefix)$m;) } > $(MODORDER)
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
>
> KBUILD_MODULES := 1
>
> -export KBUILD_SINGLE_TARGETS := $(addprefix $(extmod-prefix), $(single-no-ko))
> +export KBUILD_SINGLE_TARGETS := $(addprefix $(extmod_prefix), $(single-no-ko))
>
> # trim unrelated directories
> build-dirs := $(foreach d, $(build-dirs), \
> @@ -1906,12 +1906,12 @@ nsdeps: modules
> quiet_cmd_gen_compile_commands = GEN $@
> cmd_gen_compile_commands = $(PYTHON3) $< -a $(AR) -o $@ $(filter-out $<, $(real-prereqs))
>
> -$(extmod-prefix)compile_commands.json: scripts/clang-tools/gen_compile_commands.py \
> +$(extmod_prefix)compile_commands.json: scripts/clang-tools/gen_compile_commands.py \
> $(if $(KBUILD_EXTMOD),,$(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)) \
> $(if $(CONFIG_MODULES), $(MODORDER)) FORCE
> $(call if_changed,gen_compile_commands)
>
> -targets += $(extmod-prefix)compile_commands.json
> +targets += $(extmod_prefix)compile_commands.json
>
> PHONY += clang-tidy clang-analyzer
>
> @@ -1919,7 +1919,7 @@ ifdef CONFIG_CC_IS_CLANG
> quiet_cmd_clang_tools = CHECK $<
> cmd_clang_tools = $(PYTHON3) $(srctree)/scripts/clang-tools/run-clang-tools.py $@ $<
>
> -clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
> +clang-tidy clang-analyzer: $(extmod_prefix)compile_commands.json
> $(call cmd,clang_tools)
> else
> clang-tidy clang-analyzer:
> --
> 2.27.0
>


--
Thanks,
~Nick Desaulniers

2021-03-31 18:03:03

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 9/9] kbuild: remove CONFIG_MODULE_COMPRESS

On Wed, Mar 31, 2021 at 6:39 AM Masahiro Yamada <[email protected]> wrote:

Should the online be Kconfig rather than Kbuild, for a commit that
only changes Kconfigs?

>
> CONFIG_MODULE_COMPRESS is only used to activate the choice for module
> compression algorithm. It will be simpler to make the choice visible
> all the time by adding CONFIG_MODULE_COMPRESS_NONE to allow the user to
> disable module compression.
>
> This is more consistent with the "Kernel compression mode" and "Built-in
> initramfs compression mode" choices.
>
> CONFIG_KERNEL_UNCOMPRESSED and CONFIG_INITRAMFS_COMPRESSION_NONE are
> available to choose to not compress the kernel, initrd, respectively.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> init/Kconfig | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 019c1874e609..3ca1ffd219c4 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2225,40 +2225,47 @@ config MODULE_SIG_HASH
> default "sha384" if MODULE_SIG_SHA384
> default "sha512" if MODULE_SIG_SHA512
>
> -config MODULE_COMPRESS

The top level Makefile has comments and code that refer to this choice
which is now removed. I think you'll want to fix that up in this
change as well? Ah, patch 7 in the series does that:
https://lore.kernel.org/linux-kbuild/[email protected]/

Ok then this LGTM.
Reviewed-by: Nick Desaulniers <[email protected]>

> - bool "Compress modules on installation"
> +choice
> + prompt "Module compression mode"
> help
> + This option allows you to choose the algorithm which will be used to
> + compress modules when 'make modules_install' is run. (or, you can
> + choose to not compress modules at all.)
>
> - Compresses kernel modules when 'make modules_install' is run; gzip or
> - xz depending on "Compression algorithm" below.
> + External modules will also be compressed in the same way during the
> + installation.
>
> - module-init-tools MAY support gzip, and kmod MAY support gzip and xz.
> + For modules inside an initrd or initramfs, it's more efficient to
> + compress the whole initrd or initramfs instead.
>
> - Out-of-tree kernel modules installed using Kbuild will also be
> - compressed upon installation.
> + This is fully compatible with signed modules.
>
> - Note: for modules inside an initrd or initramfs, it's more efficient
> - to compress the whole initrd or initramfs instead.
> + Please note that the tool used to load modules needs to support the
> + corresponding algorithm. module-init-tools MAY support gzip, and kmod
> + MAY support gzip and xz.
>
> - Note: This is fully compatible with signed modules.
> + Your build system needs to provide the appropriate compression tool
> + to compress the modules.
>
> - If in doubt, say N.
> + If in doubt, select 'None'.
>
> -choice
> - prompt "Compression algorithm"
> - depends on MODULE_COMPRESS
> - default MODULE_COMPRESS_GZIP
> +config MODULE_COMPRESS_NONE
> + bool "None"
> help
> - This determines which sort of compression will be used during
> - 'make modules_install'.
> -
> - GZIP (default) and XZ are supported.
> + Do not compress modules. The installed modules are suffixed
> + with .ko.
>
> config MODULE_COMPRESS_GZIP
> bool "GZIP"
> + help
> + Compress modules with XZ. The installed modules are suffixed
> + with .ko.gz.
>
> config MODULE_COMPRESS_XZ
> bool "XZ"
> + help
> + Compress modules with XZ. The installed modules are suffixed
> + with .ko.xz.
>
> endchoice
>
> --
> 2.27.0
>


--
Thanks,
~Nick Desaulniers

2021-04-07 21:04:44

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 9/9] kbuild: remove CONFIG_MODULE_COMPRESS

On Wed, Mar 31, 2021 at 10:39 PM Masahiro Yamada <[email protected]> wrote:
>
> CONFIG_MODULE_COMPRESS is only used to activate the choice for module
> compression algorithm. It will be simpler to make the choice visible
> all the time by adding CONFIG_MODULE_COMPRESS_NONE to allow the user to
> disable module compression.
>
> This is more consistent with the "Kernel compression mode" and "Built-in
> initramfs compression mode" choices.
>
> CONFIG_KERNEL_UNCOMPRESSED and CONFIG_INITRAMFS_COMPRESSION_NONE are
> available to choose to not compress the kernel, initrd, respectively.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---
>
> init/Kconfig | 45 ++++++++++++++++++++++++++-------------------
> 1 file changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 019c1874e609..3ca1ffd219c4 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2225,40 +2225,47 @@ config MODULE_SIG_HASH
> default "sha384" if MODULE_SIG_SHA384
> default "sha512" if MODULE_SIG_SHA512
>
> -config MODULE_COMPRESS
> - bool "Compress modules on installation"
> +choice
> + prompt "Module compression mode"
> help
> + This option allows you to choose the algorithm which will be used to
> + compress modules when 'make modules_install' is run. (or, you can
> + choose to not compress modules at all.)
>
> - Compresses kernel modules when 'make modules_install' is run; gzip or
> - xz depending on "Compression algorithm" below.
> + External modules will also be compressed in the same way during the
> + installation.
>
> - module-init-tools MAY support gzip, and kmod MAY support gzip and xz.
> + For modules inside an initrd or initramfs, it's more efficient to
> + compress the whole initrd or initramfs instead.
>
> - Out-of-tree kernel modules installed using Kbuild will also be
> - compressed upon installation.
> + This is fully compatible with signed modules.
>
> - Note: for modules inside an initrd or initramfs, it's more efficient
> - to compress the whole initrd or initramfs instead.
> + Please note that the tool used to load modules needs to support the
> + corresponding algorithm. module-init-tools MAY support gzip, and kmod
> + MAY support gzip and xz.
>
> - Note: This is fully compatible with signed modules.
> + Your build system needs to provide the appropriate compression tool
> + to compress the modules.
>
> - If in doubt, say N.
> + If in doubt, select 'None'.
>
> -choice
> - prompt "Compression algorithm"
> - depends on MODULE_COMPRESS
> - default MODULE_COMPRESS_GZIP
> +config MODULE_COMPRESS_NONE
> + bool "None"
> help
> - This determines which sort of compression will be used during
> - 'make modules_install'.
> -
> - GZIP (default) and XZ are supported.
> + Do not compress modules. The installed modules are suffixed
> + with .ko.
>
> config MODULE_COMPRESS_GZIP
> bool "GZIP"
> + help
> + Compress modules with XZ. The installed modules are suffixed


This should be "Compress modules with GZIP."


I will fix it when applied.







> + with .ko.gz.
>
> config MODULE_COMPRESS_XZ
> bool "XZ"
> + help
> + Compress modules with XZ. The installed modules are suffixed
> + with .ko.xz.
>
> endchoice
>
> --
> 2.27.0
>


--
Best Regards
Masahiro Yamada

2021-04-07 21:05:20

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 1/9] kbuild: remove unneeded mkdir for external modules_install

On Wed, Mar 31, 2021 at 10:38 PM Masahiro Yamada <[email protected]> wrote:
>
> scripts/Makefile.modinst creates directories as needed.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> ---

Applied to linux-kbuild.


>
> Makefile | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index ed8bd815e8a3..0e06db5ed9d8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1779,10 +1779,8 @@ $(MODORDER): descend
> PHONY += modules_install
> modules_install: _emodinst_ _emodinst_post
>
> -install-dir := $(if $(INSTALL_MOD_DIR),$(INSTALL_MOD_DIR),extra)
> PHONY += _emodinst_
> _emodinst_:
> - $(Q)mkdir -p $(MODLIB)/$(install-dir)
> $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modinst
>
> PHONY += _emodinst_post
> --
> 2.27.0
>


--
Best Regards
Masahiro Yamada

2021-05-12 14:25:25

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 6/9] kbuild: refactor scripts/Makefile.modinst

Hi,

So I'm not *entirely* sure if this caused it, but I noticed that doing

make -C linux M=/path/to/extmod/ modules_install

stopped working.

This is because here:
>
> -extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)
> +export extmod_prefix = $(if $(KBUILD_EXTMOD),$(KBUILD_EXTMOD)/)


(as before, of course) another trailing / is added to the M= argument,
and then

> +modules := $(patsubst $(extmod_prefix)%, $(dst)/%, $(modules))

this patsubst turns out to do nothing. So $(modules) contains the
original paths where the modules were compiled, and consequently nothing
happens.

Specifying

make -C linux M=/path/to/extmod modules_install

actually works.


Obviously I can work around it, but it hardly seems intentional?

Thanks,
johannes