2022-09-28 06:45:23

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 0/8] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS


This patch set refactors EXPORT_SYMBOL, <linux/export.h> and <asm/export.h>.
Also, re-implement TRIM_UNUSED_KSYMS in one-pass.

You can still put EXPORT_SYMBOL() in *.S file, very close to the definition,
but you do not need to care about whether it is a function or a data.
Remove EXPORT_DATA_SYMBOL().

In v1, I broke ia64 because of missing distinction between functions and data.

V2 handles it correctly.
If the exported symbols is a function, KSYMTAB_FUNC is output.
Otherwise, KSYMTAB_DATA is output.


Changes in v3:
- Move to the head of the series
- New patch
- Move struct kernel_symbol to kernel/module/internal.h
- Some cleanups

Changes in v2:
- New patch
- Use KSYMTAB_FUNC and KSYMTAB_DATA for functions and data, respectively
This distinction is needed for ia64.
- New patch
- New patch

Masahiro Yamada (8):
kbuild: move modules.builtin(.modinfo) rules to Makefile.vmlinux_o
kbuild: rebuild .vmlinux.export.o when its prerequisite is updated
kbuild: generate KSYMTAB entries by modpost
ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL*
modpost: squash sym_update_namespace() into sym_add_exported()
modpost: use null string instead of NULL pointer for default namespace
modpost: squash report_sec_mismatch() and remove enum mismatch
kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion

.gitignore | 1 -
Makefile | 37 ++----
arch/ia64/include/asm/Kbuild | 1 +
arch/ia64/include/asm/export.h | 3 -
arch/ia64/kernel/head.S | 2 +-
arch/ia64/kernel/ivt.S | 2 +-
include/asm-generic/export.h | 83 +-----------
include/linux/export-internal.h | 49 +++++++
include/linux/export.h | 114 +++-------------
kernel/module/internal.h | 12 ++
scripts/Makefile.build | 15 +--
scripts/Makefile.modpost | 8 +-
scripts/Makefile.vmlinux | 21 ++-
scripts/Makefile.vmlinux_o | 26 +++-
scripts/adjust_autoksyms.sh | 73 ----------
scripts/basic/fixdep.c | 3 +-
scripts/check-local-export | 4 +-
scripts/gen_autoksyms.sh | 62 ---------
scripts/gen_ksymdeps.sh | 30 -----
scripts/link-vmlinux.sh | 12 --
scripts/mod/modpost.c | 228 ++++++++++++++++----------------
scripts/mod/modpost.h | 1 +
scripts/remove-stale-files | 2 +
23 files changed, 270 insertions(+), 519 deletions(-)
delete mode 100644 arch/ia64/include/asm/export.h
delete mode 100755 scripts/adjust_autoksyms.sh
delete mode 100755 scripts/gen_autoksyms.sh
delete mode 100755 scripts/gen_ksymdeps.sh

--
2.34.1


2022-09-28 06:45:54

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 7/8] modpost: squash report_sec_mismatch() and remove enum mismatch

Now report_sec_mismatch() prints the same warning message for all
possible cases. (fatal() is just a sanity check for unreachable code.)

Squash it into default_mismatch_handler().

enum mismatch is no longer used. Remove it as well.

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

(no changes since v2)

Changes in v2:
- New patch

scripts/mod/modpost.c | 61 +++----------------------------------------
1 file changed, 4 insertions(+), 57 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 29f30558a398..90733664a602 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -847,18 +847,6 @@ static const char *const linker_symbols[] =
{ "__init_begin", "_sinittext", "_einittext", NULL };
static const char *const optim_symbols[] = { "*.constprop.*", NULL };

-enum mismatch {
- TEXT_TO_ANY_INIT,
- DATA_TO_ANY_INIT,
- TEXT_TO_ANY_EXIT,
- DATA_TO_ANY_EXIT,
- XXXINIT_TO_SOME_INIT,
- XXXEXIT_TO_SOME_EXIT,
- ANY_INIT_TO_ANY_EXIT,
- ANY_EXIT_TO_ANY_INIT,
- EXTABLE_TO_NON_TEXT,
-};
-
/**
* Describe how to match sections on different criteria:
*
@@ -880,7 +868,6 @@ struct sectioncheck {
const char *fromsec[20];
const char *bad_tosec[20];
const char *good_tosec[20];
- enum mismatch mismatch;
void (*handler)(const char *modname, struct elf_info *elf,
const struct sectioncheck* const mismatch,
Elf_Rela *r, Elf_Sym *sym, const char *fromsec);
@@ -899,56 +886,46 @@ static const struct sectioncheck sectioncheck[] = {
{
.fromsec = { TEXT_SECTIONS, NULL },
.bad_tosec = { ALL_INIT_SECTIONS, NULL },
- .mismatch = TEXT_TO_ANY_INIT,
},
{
.fromsec = { DATA_SECTIONS, NULL },
.bad_tosec = { ALL_XXXINIT_SECTIONS, NULL },
- .mismatch = DATA_TO_ANY_INIT,
},
{
.fromsec = { DATA_SECTIONS, NULL },
.bad_tosec = { INIT_SECTIONS, NULL },
- .mismatch = DATA_TO_ANY_INIT,
},
{
.fromsec = { TEXT_SECTIONS, NULL },
.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
- .mismatch = TEXT_TO_ANY_EXIT,
},
{
.fromsec = { DATA_SECTIONS, NULL },
.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
- .mismatch = DATA_TO_ANY_EXIT,
},
/* Do not reference init code/data from meminit code/data */
{
.fromsec = { ALL_XXXINIT_SECTIONS, NULL },
.bad_tosec = { INIT_SECTIONS, NULL },
- .mismatch = XXXINIT_TO_SOME_INIT,
},
/* Do not reference exit code/data from memexit code/data */
{
.fromsec = { ALL_XXXEXIT_SECTIONS, NULL },
.bad_tosec = { EXIT_SECTIONS, NULL },
- .mismatch = XXXEXIT_TO_SOME_EXIT,
},
/* Do not use exit code/data from init code */
{
.fromsec = { ALL_INIT_SECTIONS, NULL },
.bad_tosec = { ALL_EXIT_SECTIONS, NULL },
- .mismatch = ANY_INIT_TO_ANY_EXIT,
},
/* Do not use init code/data from exit code */
{
.fromsec = { ALL_EXIT_SECTIONS, NULL },
.bad_tosec = { ALL_INIT_SECTIONS, NULL },
- .mismatch = ANY_EXIT_TO_ANY_INIT,
},
{
.fromsec = { ALL_PCI_INIT_SECTIONS, NULL },
.bad_tosec = { INIT_SECTIONS, NULL },
- .mismatch = ANY_INIT_TO_ANY_EXIT,
},
{
.fromsec = { "__ex_table", NULL },
@@ -957,7 +934,6 @@ static const struct sectioncheck sectioncheck[] = {
*/
.bad_tosec = { ".altinstr_replacement", NULL },
.good_tosec = {ALL_TEXT_SECTIONS , NULL},
- .mismatch = EXTABLE_TO_NON_TEXT,
.handler = extable_mismatch_handler,
}
};
@@ -1215,37 +1191,6 @@ static inline void get_pretty_name(int is_func, const char** name, const char**
}
}

-/*
- * Print a warning about a section mismatch.
- * Try to find symbols near it so user can find it.
- * Check whitelist before warning - it may be a false positive.
- */
-static void report_sec_mismatch(const char *modname,
- const struct sectioncheck *mismatch,
- const char *fromsec,
- const char *fromsym,
- const char *tosec, const char *tosym)
-{
- sec_mismatch_count++;
-
- switch (mismatch->mismatch) {
- case TEXT_TO_ANY_INIT:
- case DATA_TO_ANY_INIT:
- case TEXT_TO_ANY_EXIT:
- case DATA_TO_ANY_EXIT:
- case XXXINIT_TO_SOME_INIT:
- case XXXEXIT_TO_SOME_EXIT:
- case ANY_INIT_TO_ANY_EXIT:
- case ANY_EXIT_TO_ANY_INIT:
- warn("%s: section mismatch in reference: %s (section: %s) -> %s (section: %s)\n",
- modname, fromsym, fromsec, tosym, tosec);
- break;
- case EXTABLE_TO_NON_TEXT:
- fatal("There's a special handler for this mismatch type, we should never get here.\n");
- break;
- }
-}
-
static void default_mismatch_handler(const char *modname, struct elf_info *elf,
const struct sectioncheck* const mismatch,
Elf_Rela *r, Elf_Sym *sym, const char *fromsec)
@@ -1266,8 +1211,10 @@ static void default_mismatch_handler(const char *modname, struct elf_info *elf,
/* check whitelist - we may ignore it */
if (secref_whitelist(mismatch,
fromsec, fromsym, tosec, tosym)) {
- report_sec_mismatch(modname, mismatch,
- fromsec, fromsym, tosec, tosym);
+ sec_mismatch_count++;
+
+ warn("%s: section mismatch in reference: %s (section: %s) -> %s (section: %s)\n",
+ modname, fromsym, fromsec, tosym, tosec);
}
}

--
2.34.1

2022-09-28 06:46:12

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 8/8] kbuild: implement CONFIG_TRIM_UNUSED_KSYMS without recursion

When CONFIG_TRIM_UNUSED_KSYMS is enabled, Kbuild recursively traverses
the directory tree to determine which EXPORT_SYMBOL to trim. If an
EXPORT_SYMBOL turns out to be unused by anyone, Kbuild begins the
second traverse, where some source files are recompiled with their
EXPORT_SYMBOL() tuned into a no-op.

Linus stated a negative opinion about this slowness in commits:

- 5cf0fd591f2e ("Kbuild: disable TRIM_UNUSED_KSYMS option")
- a555bdd0c58c ("Kbuild: enable TRIM_UNUSED_KSYMS again, with some guarding")

We can do this better now. The final data structures of EXPORT_SYMBOL
are generated by the modpost stage, so modpost can selectively emit
KSYMTAB entries that are really used by modules.

Commit 2cce989f8461 ("kbuild: unify two modpost invocations") is another
ground-work to do this in a one-pass algorithm. With the list of modules,
modpost sets sym->used if it is used by a module. modpost emits KSYMTAB
only for symbols with sym->used==true.

BTW, Nicolas explained why the trimming was implemented with recursion:

https://lore.kernel.org/all/[email protected]/

Actually, we never achieved that level of optimization where the chain
reaction of trimming comes into play because:

- CONFIG_LTO_CLANG cannot remove any unused symbols
- CONFIG_LD_DEAD_CODE_DATA_ELIMINATION is enabled only for vmlinux,
but not modules

If deeper trimming is required, we need to revisit this, but I guess
that is unlikely to happen.

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

Changes in v3:
- Some cleanups

Changes in v2:
- New patch

.gitignore | 1 -
Makefile | 19 +---------
include/linux/export.h | 41 ---------------------
scripts/Makefile.build | 7 ----
scripts/Makefile.modpost | 8 +++-
scripts/adjust_autoksyms.sh | 73 -------------------------------------
scripts/basic/fixdep.c | 3 +-
scripts/gen_autoksyms.sh | 62 -------------------------------
scripts/gen_ksymdeps.sh | 30 ---------------
scripts/mod/modpost.c | 54 ++++++++++++++++++++++++---
scripts/remove-stale-files | 2 +
11 files changed, 61 insertions(+), 239 deletions(-)
delete mode 100755 scripts/adjust_autoksyms.sh
delete mode 100755 scripts/gen_autoksyms.sh
delete mode 100755 scripts/gen_ksymdeps.sh

diff --git a/.gitignore b/.gitignore
index 265959544978..a9b44cd36066 100644
--- a/.gitignore
+++ b/.gitignore
@@ -103,7 +103,6 @@ modules.order
#
/include/config/
/include/generated/
-/include/ksym/
/arch/*/include/generated/

# stgit generated dirs
diff --git a/Makefile b/Makefile
index 79488f155fae..31dcde4c7fc5 100644
--- a/Makefile
+++ b/Makefile
@@ -1120,28 +1120,13 @@ endif
export KBUILD_VMLINUX_LIBS
export KBUILD_LDS := arch/$(SRCARCH)/kernel/vmlinux.lds

-# Recurse until adjust_autoksyms.sh is satisfied
-PHONY += autoksyms_recursive
ifdef CONFIG_TRIM_UNUSED_KSYMS
# For the kernel to actually contain only the needed exported symbols,
# we have to build modules as well to determine what those symbols are.
# (this can be evaluated only once include/config/auto.conf has been included)
KBUILD_MODULES := 1
-
-autoksyms_recursive: $(build-dir) modules.order
- $(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh \
- "$(MAKE) -f $(srctree)/Makefile autoksyms_recursive"
endif

-autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
-
-quiet_cmd_autoksyms_h = GEN $@
- cmd_autoksyms_h = mkdir -p $(dir $@); \
- $(CONFIG_SHELL) $(srctree)/scripts/gen_autoksyms.sh $@
-
-$(autoksyms_h):
- $(call cmd,autoksyms_h)
-
# '$(AR) mPi' needs 'T' to workaround the bug of llvm-ar <= 14
quiet_cmd_ar_vmlinux.a = AR $@
cmd_ar_vmlinux.a = \
@@ -1154,7 +1139,7 @@ vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt FORCE
$(call if_changed,ar_vmlinux.a)

PHONY += vmlinux_o
-vmlinux_o: autoksyms_recursive vmlinux.a $(KBUILD_VMLINUX_LIBS)
+vmlinux_o: vmlinux.a $(KBUILD_VMLINUX_LIBS)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vmlinux_o

vmlinux.o modules.builtin.modinfo modules.builtin: vmlinux_o
@@ -1191,7 +1176,7 @@ scripts: scripts_basic scripts_dtc
PHONY += prepare archprepare

archprepare: outputmakefile archheaders archscripts scripts include/config/kernel.release \
- asm-generic $(version_h) $(autoksyms_h) include/generated/utsrelease.h \
+ asm-generic $(version_h) include/generated/utsrelease.h \
include/generated/compile.h include/generated/autoconf.h remove-stale-files

prepare0: archprepare
diff --git a/include/linux/export.h b/include/linux/export.h
index 0c71577cf8bb..d9fb1ebb535c 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -63,47 +63,6 @@ extern struct module __this_module;
*/
#define __EXPORT_SYMBOL(sym, sec, ns)

-#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
-
-#include <generated/autoksyms.h>
-
-/*
- * For fine grained build dependencies, we want to tell the build system
- * about each possible exported symbol even if they're not actually exported.
- * We use a symbol pattern __ksym_marker_<symbol> that the build system filters
- * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
- * discarded in the final link stage.
- */
-
-#ifdef __ASSEMBLY__
-
-#define __ksym_marker(sym) \
- .section ".discard.ksym","a" ; \
-__ksym_marker_##sym: ; \
- .previous
-
-#else
-
-#define __ksym_marker(sym) \
- static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
-
-#endif
-
-#define __EXPORT_SYMBOL(sym, sec, ns) \
- __ksym_marker(sym); \
- __cond_export_sym(sym, sec, ns, __is_defined(__KSYM_##sym))
-#define __cond_export_sym(sym, sec, ns, conf) \
- ___cond_export_sym(sym, sec, ns, conf)
-#define ___cond_export_sym(sym, sec, ns, enabled) \
- __cond_export_sym_##enabled(sym, sec, ns)
-#define __cond_export_sym_1(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
-
-#ifdef __GENKSYMS__
-#define __cond_export_sym_0(sym, sec, ns) __GENKSYMS_EXPORT_SYMBOL(sym)
-#else
-#define __cond_export_sym_0(sym, sec, ns) /* nothing */
-#endif
-
#else

#define __EXPORT_SYMBOL(sym, sec, ns) ___EXPORT_SYMBOL(sym, sec, ns)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 4c1d0bd1bc03..64f652b03d29 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -218,16 +218,10 @@ is-standard-object = $(if $(filter-out y%, $(OBJECT_FILES_NON_STANDARD_$(basetar

$(obj)/%.o: objtool-enabled = $(if $(is-standard-object),$(if $(delay-objtool),$(is-single-obj-m),y))

-ifdef CONFIG_TRIM_UNUSED_KSYMS
-cmd_gen_ksymdeps = \
- $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
-endif
-
cmd_check_local_export = $(srctree)/scripts/check-local-export $@

define rule_cc_o_c
$(call cmd_and_fixdep,cc_o_c)
- $(call cmd,gen_ksymdeps)
$(call cmd,check_local_export)
$(call cmd,checksrc)
$(call cmd,checkdoc)
@@ -238,7 +232,6 @@ endef

define rule_as_o_S
$(call cmd_and_fixdep,as_o_S)
- $(call cmd,gen_ksymdeps)
$(call cmd,check_local_export)
$(call cmd,gen_objtooldep)
$(call cmd,gen_symversions_S)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 7740ce3b29e8..555b9e741a01 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -86,6 +86,12 @@ output-symdump := $(if $(vmlinux.o-if-present), Module.symvers, modules-only.sym
missing-input := $(filter-out $(vmlinux.o-if-present),vmlinux.o)
endif

+ifdef CONFIG_TRIM_UNUSED_KSYMS
+ksym-wl := $(CONFIG_UNUSED_KSYMS_WHITELIST)
+ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(srctree)/)$(ksym-wl)
+modpost-args += -t $(addprefix -W, $(ksym-wl))
+endif
+
else

# set src + obj - they may be used in the modules's Makefile
@@ -122,7 +128,7 @@ quiet_cmd_modpost = MODPOST $@
sed 's/ko$$/o/' $(or $(modorder-if-needed), /dev/null) | $(MODPOST) $(modpost-args) $(vmlinux.o-if-present) -T -

targets += $(output-symdump)
-$(output-symdump): $(modorder-if-needed) $(vmlinux.o-if-present) $(moudle.symvers-if-present) $(MODPOST) FORCE
+$(output-symdump): $(modorder-if-needed) $(vmlinux.o-if-present) $(moudle.symvers-if-present) $(MODPOST) $(ksym-wl) FORCE
$(call if_changed,modpost)

__modpost: $(output-symdump)
diff --git a/scripts/adjust_autoksyms.sh b/scripts/adjust_autoksyms.sh
deleted file mode 100755
index f1b5ac818411..000000000000
--- a/scripts/adjust_autoksyms.sh
+++ /dev/null
@@ -1,73 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0-only
-
-# Script to update include/generated/autoksyms.h and dependency files
-#
-# Copyright: (C) 2016 Linaro Limited
-# Created by: Nicolas Pitre, January 2016
-#
-
-# Update the include/generated/autoksyms.h file.
-#
-# For each symbol being added or removed, the corresponding dependency
-# file's timestamp is updated to force a rebuild of the affected source
-# file. All arguments passed to this script are assumed to be a command
-# to be exec'd to trigger a rebuild of those files.
-
-set -e
-
-cur_ksyms_file="include/generated/autoksyms.h"
-new_ksyms_file="include/generated/autoksyms.h.tmpnew"
-
-info() {
- if [ "$quiet" != "silent_" ]; then
- printf " %-7s %s\n" "$1" "$2"
- fi
-}
-
-info "CHK" "$cur_ksyms_file"
-
-# Use "make V=1" to debug this script.
-case "$KBUILD_VERBOSE" in
-*1*)
- set -x
- ;;
-esac
-
-# Generate a new symbol list file
-$CONFIG_SHELL $srctree/scripts/gen_autoksyms.sh --modorder "$new_ksyms_file"
-
-# Extract changes between old and new list and touch corresponding
-# dependency files.
-changed=$(
-count=0
-sort "$cur_ksyms_file" "$new_ksyms_file" | uniq -u |
-sed -n 's/^#define __KSYM_\(.*\) 1/\1/p' |
-while read sympath; do
- if [ -z "$sympath" ]; then continue; fi
- depfile="include/ksym/${sympath}"
- mkdir -p "$(dirname "$depfile")"
- touch "$depfile"
- # Filesystems with coarse time precision may create timestamps
- # equal to the one from a file that was very recently built and that
- # needs to be rebuild. Let's guard against that by making sure our
- # dep files are always newer than the first file we created here.
- while [ ! "$depfile" -nt "$new_ksyms_file" ]; do
- touch "$depfile"
- done
- echo $((count += 1))
-done | tail -1 )
-changed=${changed:-0}
-
-if [ $changed -gt 0 ]; then
- # Replace the old list with tne new one
- old=$(grep -c "^#define __KSYM_" "$cur_ksyms_file" || true)
- new=$(grep -c "^#define __KSYM_" "$new_ksyms_file" || true)
- info "KSYMS" "symbols: before=$old, after=$new, changed=$changed"
- info "UPD" "$cur_ksyms_file"
- mv -f "$new_ksyms_file" "$cur_ksyms_file"
- # Then trigger a rebuild of affected source files
- exec $@
-else
- rm -f "$new_ksyms_file"
-fi
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c
index 2328f9a641da..e7db12140d87 100644
--- a/scripts/basic/fixdep.c
+++ b/scripts/basic/fixdep.c
@@ -243,8 +243,7 @@ static void *read_file(const char *filename)
/* Ignore certain dependencies */
static int is_ignored_file(const char *s, int len)
{
- return str_ends_with(s, len, "include/generated/autoconf.h") ||
- str_ends_with(s, len, "include/generated/autoksyms.h");
+ return str_ends_with(s, len, "include/generated/autoconf.h");
}

/*
diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
deleted file mode 100755
index 653fadbad302..000000000000
--- a/scripts/gen_autoksyms.sh
+++ /dev/null
@@ -1,62 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0-only
-
-# Create an autoksyms.h header file from the list of all module's needed symbols
-# as recorded in *.usyms files and the user-provided symbol whitelist.
-
-set -e
-
-# Use "make V=1" to debug this script.
-case "$KBUILD_VERBOSE" in
-*1*)
- set -x
- ;;
-esac
-
-read_modorder=
-
-if [ "$1" = --modorder ]; then
- shift
- read_modorder=1
-fi
-
-output_file="$1"
-
-needed_symbols=
-
-# Special case for modversions (see modpost.c)
-if grep -q "^CONFIG_MODVERSIONS=y$" include/config/auto.conf; then
- needed_symbols="$needed_symbols module_layout"
-fi
-
-ksym_wl=$(sed -n 's/^CONFIG_UNUSED_KSYMS_WHITELIST=\(.*\)$/\1/p' include/config/auto.conf)
-if [ -n "$ksym_wl" ]; then
- [ "${ksym_wl}" != "${ksym_wl#/}" ] || ksym_wl="$abs_srctree/$ksym_wl"
- if [ ! -f "$ksym_wl" ] || [ ! -r "$ksym_wl" ]; then
- echo "ERROR: '$ksym_wl' whitelist file not found" >&2
- exit 1
- fi
-fi
-
-# Generate a new ksym list file with symbols needed by the current
-# set of modules.
-cat > "$output_file" << EOT
-/*
- * Automatically generated file; DO NOT EDIT.
- */
-
-EOT
-
-{
- [ -n "${read_modorder}" ] && sed 's/ko$/usyms/' modules.order | xargs cat
- echo "$needed_symbols"
- [ -n "$ksym_wl" ] && cat "$ksym_wl"
-} | sed -e 's/ /\n/g' | sed -n -e '/^$/!p' |
-# Remove the dot prefix for ppc64; symbol names with a dot (.) hold entry
-# point addresses.
-sed -e 's/^\.//' |
-sort -u |
-# Ignore __this_module. It's not an exported symbol, and will be resolved
-# when the final .ko's are linked.
-grep -v '^__this_module$' |
-sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
diff --git a/scripts/gen_ksymdeps.sh b/scripts/gen_ksymdeps.sh
deleted file mode 100755
index 8ee533f33659..000000000000
--- a/scripts/gen_ksymdeps.sh
+++ /dev/null
@@ -1,30 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-set -e
-
-# List of exported symbols
-#
-# If the object has no symbol, $NM warns 'no symbols'.
-# Suppress the stderr.
-# TODO:
-# Use -q instead of 2>/dev/null when we upgrade the minimum version of
-# binutils to 2.37, llvm to 13.0.0.
-ksyms=$($NM $1 2>/dev/null | sed -n 's/.*__ksym_marker_\(.*\)/\1/p')
-
-if [ -z "$ksyms" ]; then
- exit 0
-fi
-
-echo
-echo "ksymdeps_$1 := \\"
-
-for s in $ksyms
-do
- printf ' $(wildcard include/ksym/%s) \\\n' "$s"
-done
-
-echo
-echo "$1: \$(ksymdeps_$1)"
-echo
-echo "\$(ksymdeps_$1):"
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 90733664a602..1e9eba0a6cec 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -34,6 +34,9 @@ static bool warn_unresolved;

static int sec_mismatch_count;
static bool sec_mismatch_warn_only = true;
+/* Trim EXPORT_SYMBOLs that are unused by in-tree modules */
+static bool trim_unused_exports;
+
/* ignore missing files */
static bool ignore_missing_files;
/* If set to 1, only warn (instead of error) about missing ns imports */
@@ -216,6 +219,7 @@ struct symbol {
bool weak;
bool is_func;
bool is_gpl_only; /* exported by EXPORT_SYMBOL_GPL */
+ bool used; /* there exists a user of this symbol */
char name[];
};

@@ -1872,6 +1876,7 @@ static void check_exports(struct module *mod)
continue;
}

+ exp->used = true;
s->module = exp->module;
s->crc_valid = exp->crc_valid;
s->crc = exp->crc;
@@ -1895,6 +1900,23 @@ static void check_exports(struct module *mod)
}
}

+static void handle_white_list_exports(const char *white_list)
+{
+ char *buf, *p, *name;
+
+ buf = read_text_file(white_list);
+ p = buf;
+
+ while ((name = strsep(&p, "\n"))) {
+ struct symbol *sym = find_symbol(name);
+
+ if (sym)
+ sym->used = true;
+ }
+
+ free(buf);
+}
+
static void check_modname_len(struct module *mod)
{
const char *mod_name;
@@ -1965,10 +1987,14 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)

/* generate struct for exported symbols */
buf_printf(buf, "\n");
- list_for_each_entry(sym, &mod->exported_symbols, list)
+ list_for_each_entry(sym, &mod->exported_symbols, list) {
+ if (trim_unused_exports && !sym->used)
+ continue;
+
buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
sym->is_func ? "FUNC" : "DATA", sym->name,
sym->is_gpl_only ? "_gpl" : "", sym->namespace);
+ }

if (!modversions)
return;
@@ -1976,6 +2002,9 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
/* record CRCs for exported symbols */
buf_printf(buf, "\n");
list_for_each_entry(sym, &mod->exported_symbols, list) {
+ if (trim_unused_exports && !sym->used)
+ continue;
+
if (!sym->crc_valid)
warn("EXPORT symbol \"%s\" [%s%s] version generation failed, symbol will not be versioned.\n"
"Is \"%s\" prototyped in <asm/asm-prototypes.h>?\n",
@@ -2139,9 +2168,6 @@ static void write_mod_c_file(struct module *mod)
char fname[PATH_MAX];
int ret;

- check_modname_len(mod);
- check_exports(mod);
-
add_header(&buf, mod);
add_exported_symbols(&buf, mod);
add_versions(&buf, mod);
@@ -2275,12 +2301,13 @@ int main(int argc, char **argv)
{
struct module *mod;
char *missing_namespace_deps = NULL;
+ char *unused_exports_white_list = NULL;
char *dump_write = NULL, *files_source = NULL;
int opt;
LIST_HEAD(dump_lists);
struct dump_list *dl, *dl2;

- while ((opt = getopt(argc, argv, "ei:mnT:o:awENd:")) != -1) {
+ while ((opt = getopt(argc, argv, "ei:mntT:tW:o:awENd:")) != -1) {
switch (opt) {
case 'e':
external_module = true;
@@ -2305,6 +2332,12 @@ int main(int argc, char **argv)
case 'T':
files_source = optarg;
break;
+ case 't':
+ trim_unused_exports = true;
+ break;
+ case 'W':
+ unused_exports_white_list = optarg;
+ break;
case 'w':
warn_unresolved = true;
break;
@@ -2334,6 +2367,17 @@ int main(int argc, char **argv)
if (files_source)
read_symbols_from_files(files_source);

+ list_for_each_entry(mod, &modules, list) {
+ if (mod->from_dump || mod->is_vmlinux)
+ continue;
+
+ check_modname_len(mod);
+ check_exports(mod);
+ }
+
+ if (unused_exports_white_list)
+ handle_white_list_exports(unused_exports_white_list);
+
list_for_each_entry(mod, &modules, list) {
if (mod->from_dump)
continue;
diff --git a/scripts/remove-stale-files b/scripts/remove-stale-files
index ccadfa3afb2b..d2009eda270a 100755
--- a/scripts/remove-stale-files
+++ b/scripts/remove-stale-files
@@ -47,3 +47,5 @@ rm -f arch/riscv/purgatory/kexec-purgatory.c
rm -f scripts/extract-cert

rm -f arch/x86/purgatory/kexec-purgatory.c
+
+rm -rf include/ksym
--
2.34.1

2022-09-28 06:46:22

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 2/8] kbuild: rebuild .vmlinux.export.o when its prerequisite is updated

When include/linux/export-internal.h is updated, .vmlinux.export.o
must be rebuilt, but it does not happen because its rule is hidden
behind scripts/link-vmlinux.sh.

Move it out of the shell script, so that Make can see the dependency
between vmlinux and .vmlinux.export.o.

Move the vmlinux rule to scripts/Makefile.vmlinux.

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

Changes in v3:
- New patch

Makefile | 16 ++++------------
scripts/Makefile.vmlinux | 21 ++++++++++++++++++++-
scripts/link-vmlinux.sh | 5 -----
3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/Makefile b/Makefile
index 83d8ff1d521a..79488f155fae 100644
--- a/Makefile
+++ b/Makefile
@@ -1160,17 +1160,9 @@ vmlinux_o: autoksyms_recursive vmlinux.a $(KBUILD_VMLINUX_LIBS)
vmlinux.o modules.builtin.modinfo modules.builtin: vmlinux_o
@:

-ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
-
-# Final link of vmlinux with optional arch pass after final link
-cmd_link-vmlinux = \
- $(CONFIG_SHELL) $< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)"; \
- $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
-
-vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) modpost FORCE
- +$(call if_changed_dep,link-vmlinux)
-
-targets += vmlinux
+PHONY += vmlinux
+vmlinux: vmlinux.o $(KBUILD_LDS) modpost
+ $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vmlinux

# The actual objects are generated when descending,
# make sure no implicit rule kicks in
@@ -1501,7 +1493,7 @@ endif # CONFIG_MODULES
# Directories & files removed with 'make clean'
CLEAN_FILES += include/ksym vmlinux.symvers modules-only.symvers \
modules.builtin modules.builtin.modinfo modules.nsdeps \
- compile_commands.json .thinlto-cache .vmlinux.objs
+ compile_commands.json .thinlto-cache .vmlinux.objs .vmlinux.export.c

# Directories & files removed with 'make mrproper'
MRPROPER_FILES += include/config include/generated \
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 7a63abf22399..49946cb96844 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -1,18 +1,37 @@
# SPDX-License-Identifier: GPL-2.0-only

+PHONY := __default
+__default: vmlinux
+
include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include

# for c_flags
include $(srctree)/scripts/Makefile.lib

+targets :=
+
quiet_cmd_cc_o_c = CC $@
cmd_cc_o_c = $(CC) $(c_flags) -c -o $@ $<

%.o: %.c FORCE
$(call if_changed_dep,cc_o_c)

-targets := $(MAKECMDGOALS)
+ifdef CONFIG_MODULES
+targets += .vmlinux.export.o
+vmlinux: .vmlinux.export.o
+endif
+
+ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
+
+# Final link of vmlinux with optional arch pass after final link
+cmd_link_vmlinux = \
+ $< "$(LD)" "$(KBUILD_LDFLAGS)" "$(LDFLAGS_vmlinux)"; \
+ $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
+
+targets += vmlinux
+vmlinux: scripts/link-vmlinux.sh vmlinux.o $(KBUILD_LDS) FORCE
+ +$(call if_changed_dep,link_vmlinux)

# Add FORCE to the prequisites of a target to force it to be always rebuilt.
# ---------------------------------------------------------------------------
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index e3d42202e54c..918470d768e9 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -199,7 +199,6 @@ cleanup()
rm -f System.map
rm -f vmlinux
rm -f vmlinux.map
- rm -f .vmlinux.export.c
}

# Use "make V=1" to debug this script
@@ -214,10 +213,6 @@ if [ "$1" = "clean" ]; then
exit 0
fi

-if is_enabled CONFIG_MODULES; then
- ${MAKE} -f "${srctree}/scripts/Makefile.vmlinux" .vmlinux.export.o
-fi
-
${MAKE} -f "${srctree}/scripts/Makefile.build" obj=init init/version-timestamp.o

btf_vmlinux_bin_o=""
--
2.34.1

2022-09-28 06:52:35

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 6/8] modpost: use null string instead of NULL pointer for default namespace

The default namespace is the null string, "".

When set, the null string "" is converted to NULL:

s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;

When printed, the NULL pointer is get back to the null string:

sym->namespace ?: ""

This saves 1 byte memory allocated for "", but loses the readability.

In kernel-space, we strive to save memory, but modpost is a userspace
tool used to build the kernel. On modern systems, such small piece of
memory is not a big deal.

Handle the namespace string as is.

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

(no changes since v1)

scripts/mod/modpost.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 0bb5bbd176b4..29f30558a398 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -297,6 +297,13 @@ static bool contains_namespace(struct list_head *head, const char *namespace)
{
struct namespace_list *list;

+ /*
+ * The default namespace is null string "", which is always implicitly
+ * contained.
+ */
+ if (!namespace[0])
+ return true;
+
list_for_each_entry(list, head, list) {
if (!strcmp(list->namespace, namespace))
return true;
@@ -366,7 +373,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
s = alloc_symbol(name);
s->module = mod;
s->is_gpl_only = gpl_only;
- s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
+ s->namespace = NOFAIL(strdup(namespace));
list_add_tail(&s->list, &mod->exported_symbols);
hash_add_symbol(s);

@@ -1928,8 +1935,7 @@ static void check_exports(struct module *mod)
else
basename = mod->name;

- if (exp->namespace &&
- !contains_namespace(&mod->imported_namespaces, exp->namespace)) {
+ if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {
modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
"module %s uses symbol %s from namespace %s, but does not import it.\n",
basename, exp->name, exp->namespace);
@@ -2015,8 +2021,7 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
list_for_each_entry(sym, &mod->exported_symbols, list)
buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
sym->is_func ? "FUNC" : "DATA", sym->name,
- sym->is_gpl_only ? "_gpl" : "",
- sym->namespace ?: "");
+ sym->is_gpl_only ? "_gpl" : "", sym->namespace);

if (!modversions)
return;
@@ -2284,7 +2289,7 @@ static void write_dump(const char *fname)
buf_printf(&buf, "0x%08x\t%s\t%s\tEXPORT_SYMBOL%s\t%s\n",
sym->crc, sym->name, mod->name,
sym->is_gpl_only ? "_GPL" : "",
- sym->namespace ?: "");
+ sym->namespace);
}
}
write_buf(&buf, fname);
--
2.34.1

2022-09-28 07:01:30

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 4/8] ia64,export.h: replace EXPORT_DATA_SYMBOL* with EXPORT_SYMBOL*

With the previous refactoring, you can always use EXPORT_SYMBOL*.

Replace two instances in ia64, then remove EXPORT_DATA_SYMBOL*.

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

(no changes since v1)

arch/ia64/kernel/head.S | 2 +-
arch/ia64/kernel/ivt.S | 2 +-
include/asm-generic/export.h | 3 ---
3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/ia64/kernel/head.S b/arch/ia64/kernel/head.S
index f22469f1c1fc..c096500590e9 100644
--- a/arch/ia64/kernel/head.S
+++ b/arch/ia64/kernel/head.S
@@ -170,7 +170,7 @@ RestRR: \
__PAGE_ALIGNED_DATA

.global empty_zero_page
-EXPORT_DATA_SYMBOL_GPL(empty_zero_page)
+EXPORT_SYMBOL_GPL(empty_zero_page)
empty_zero_page:
.skip PAGE_SIZE

diff --git a/arch/ia64/kernel/ivt.S b/arch/ia64/kernel/ivt.S
index d6d4229b28db..7a418e324d30 100644
--- a/arch/ia64/kernel/ivt.S
+++ b/arch/ia64/kernel/ivt.S
@@ -87,7 +87,7 @@

.align 32768 // align on 32KB boundary
.global ia64_ivt
- EXPORT_DATA_SYMBOL(ia64_ivt)
+ EXPORT_SYMBOL(ia64_ivt)
ia64_ivt:
/////////////////////////////////////////////////////////////////////////////////////////
// 0x0000 Entry 0 (size 64 bundles) VHPT Translation (8,20,47)
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 0ae9f38a904c..570cd4da7210 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -8,7 +8,4 @@
*/
#include <linux/export.h>

-#define EXPORT_DATA_SYMBOL(name) EXPORT_SYMBOL(name)
-#define EXPORT_DATA_SYMBOL_GPL(name) EXPORT_SYMBOL_GPL(name)
-
#endif
--
2.34.1

2022-09-28 07:02:36

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v3 1/8] kbuild: move modules.builtin(.modinfo) rules to Makefile.vmlinux_o

Do not build modules.builtin(.modinfo) as a side-effect of vmlinux.

There are no good reason to rebuild them just because any of vmlinux's
prerequistes (vmlinux.lds, .vmlinux.export.c, etc.) has been updated.

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

Changes in v3:
- Move to the head of the series

Changes in v2:
- New patch

Makefile | 6 +++++-
scripts/Makefile.vmlinux_o | 26 +++++++++++++++++++++++++-
scripts/link-vmlinux.sh | 7 -------
3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 2b4980490ecb..83d8ff1d521a 100644
--- a/Makefile
+++ b/Makefile
@@ -1153,9 +1153,13 @@ targets += vmlinux.a
vmlinux.a: $(KBUILD_VMLINUX_OBJS) scripts/head-object-list.txt FORCE
$(call if_changed,ar_vmlinux.a)

-vmlinux.o: autoksyms_recursive vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE
+PHONY += vmlinux_o
+vmlinux_o: autoksyms_recursive vmlinux.a $(KBUILD_VMLINUX_LIBS)
$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vmlinux_o

+vmlinux.o modules.builtin.modinfo modules.builtin: vmlinux_o
+ @:
+
ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)

# Final link of vmlinux with optional arch pass after final link
diff --git a/scripts/Makefile.vmlinux_o b/scripts/Makefile.vmlinux_o
index 68c22879bade..0edfdb40364b 100644
--- a/scripts/Makefile.vmlinux_o
+++ b/scripts/Makefile.vmlinux_o
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only

PHONY := __default
-__default: vmlinux.o
+__default: vmlinux.o modules.builtin.modinfo modules.builtin

include include/config/auto.conf
include $(srctree)/scripts/Kbuild.include
@@ -62,6 +62,30 @@ vmlinux.o: $(initcalls-lds) vmlinux.a $(KBUILD_VMLINUX_LIBS) FORCE

targets += vmlinux.o

+# module.builtin.modinfo
+# ---------------------------------------------------------------------------
+
+OBJCOPYFLAGS_modules.builtin.modinfo := -j .modinfo -O binary
+
+targets += modules.builtin.modinfo
+modules.builtin.modinfo: vmlinux.o FORCE
+ $(call if_changed,objcopy)
+
+# module.builtin
+# ---------------------------------------------------------------------------
+
+# The second line aids cases where multiple modules share the same object.
+
+quiet_cmd_modules_builtin = GEN $@
+ cmd_modules_builtin = \
+ tr '\0' '\n' < $< | \
+ sed -n 's/^[[:alnum:]:_]*\.file=//p' | \
+ tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$$/.ko/' > $@
+
+targets += modules.builtin
+modules.builtin: modules.builtin.modinfo FORCE
+ $(call if_changed,modules_builtin)
+
# Add FORCE to the prequisites of a target to force it to be always rebuilt.
# ---------------------------------------------------------------------------

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 2782c5d1518b..e3d42202e54c 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -214,13 +214,6 @@ if [ "$1" = "clean" ]; then
exit 0
fi

-info MODINFO modules.builtin.modinfo
-${OBJCOPY} -j .modinfo -O binary vmlinux.o modules.builtin.modinfo
-info GEN modules.builtin
-# The second line aids cases where multiple modules share the same object.
-tr '\0' '\n' < modules.builtin.modinfo | sed -n 's/^[[:alnum:]:_]*\.file=//p' |
- tr ' ' '\n' | uniq | sed -e 's:^:kernel/:' -e 's/$/.ko/' > modules.builtin
-
if is_enabled CONFIG_MODULES; then
${MAKE} -f "${srctree}/scripts/Makefile.vmlinux" .vmlinux.export.o
fi
--
2.34.1

2022-09-28 09:59:58

by David Laight

[permalink] [raw]
Subject: RE: [PATCH v3 6/8] modpost: use null string instead of NULL pointer for default namespace

From: Masahiro Yamada
> Sent: 28 September 2022 07:40
>
> The default namespace is the null string, "".
>
> When set, the null string "" is converted to NULL:
>
> s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
>
> When printed, the NULL pointer is get back to the null string:
>
> sym->namespace ?: ""
>
> This saves 1 byte memory allocated for "", but loses the readability.

The code size changes are far larger that any data sizes.

> In kernel-space, we strive to save memory, but modpost is a userspace
> tool used to build the kernel. On modern systems, such small piece of
> memory is not a big deal.
>
> Handle the namespace string as is.
>
> Signed-off-by: Masahiro Yamada <[email protected]>
> Reviewed-by: Nick Desaulniers <[email protected]>
> ---
>
> (no changes since v1)
>
> scripts/mod/modpost.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 0bb5bbd176b4..29f30558a398 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -297,6 +297,13 @@ static bool contains_namespace(struct list_head *head, const char *namespace)
> {
> struct namespace_list *list;
>
> + /*
> + * The default namespace is null string "", which is always implicitly
> + * contained.
> + */
> + if (!namespace[0])
> + return true;
> +
> list_for_each_entry(list, head, list) {
> if (!strcmp(list->namespace, namespace))
> return true;
> @@ -366,7 +373,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
> s = alloc_symbol(name);
> s->module = mod;
> s->is_gpl_only = gpl_only;
> - s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
> + s->namespace = NOFAIL(strdup(namespace));
> list_add_tail(&s->list, &mod->exported_symbols);
> hash_add_symbol(s);
>
> @@ -1928,8 +1935,7 @@ static void check_exports(struct module *mod)
> else
> basename = mod->name;
>
> - if (exp->namespace &&
> - !contains_namespace(&mod->imported_namespaces, exp->namespace)) {
> + if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {

Do you still need to optimise/check for the default namespace?

David

> modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
> "module %s uses symbol %s from namespace %s, but does not import it.\n",
> basename, exp->name, exp->namespace);
> @@ -2015,8 +2021,7 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
> list_for_each_entry(sym, &mod->exported_symbols, list)
> buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
> sym->is_func ? "FUNC" : "DATA", sym->name,
> - sym->is_gpl_only ? "_gpl" : "",
> - sym->namespace ?: "");
> + sym->is_gpl_only ? "_gpl" : "", sym->namespace);
>
> if (!modversions)
> return;
> @@ -2284,7 +2289,7 @@ static void write_dump(const char *fname)
> buf_printf(&buf, "0x%08x\t%s\t%s\tEXPORT_SYMBOL%s\t%s\n",
> sym->crc, sym->name, mod->name,
> sym->is_gpl_only ? "_GPL" : "",
> - sym->namespace ?: "");
> + sym->namespace);
> }
> }
> write_buf(&buf, fname);
> --
> 2.34.1

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2022-09-28 14:47:50

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 6/8] modpost: use null string instead of NULL pointer for default namespace

On Wed, Sep 28, 2022 at 6:53 PM David Laight <[email protected]> wrote:
>
> From: Masahiro Yamada
> > Sent: 28 September 2022 07:40
> >
> > The default namespace is the null string, "".
> >
> > When set, the null string "" is converted to NULL:
> >
> > s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
> >
> > When printed, the NULL pointer is get back to the null string:
> >
> > sym->namespace ?: ""
> >
> > This saves 1 byte memory allocated for "", but loses the readability.
>
> The code size changes are far larger that any data sizes.
>
> > In kernel-space, we strive to save memory, but modpost is a userspace
> > tool used to build the kernel. On modern systems, such small piece of
> > memory is not a big deal.
> >
> > Handle the namespace string as is.
> >
> > Signed-off-by: Masahiro Yamada <[email protected]>
> > Reviewed-by: Nick Desaulniers <[email protected]>
> > ---
> >
> > (no changes since v1)
> >
> > scripts/mod/modpost.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 0bb5bbd176b4..29f30558a398 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -297,6 +297,13 @@ static bool contains_namespace(struct list_head *head, const char *namespace)
> > {
> > struct namespace_list *list;
> >
> > + /*
> > + * The default namespace is null string "", which is always implicitly
> > + * contained.
> > + */
> > + if (!namespace[0])
> > + return true;
> > +
> > list_for_each_entry(list, head, list) {
> > if (!strcmp(list->namespace, namespace))
> > return true;
> > @@ -366,7 +373,7 @@ static struct symbol *sym_add_exported(const char *name, struct module *mod,
> > s = alloc_symbol(name);
> > s->module = mod;
> > s->is_gpl_only = gpl_only;
> > - s->namespace = namespace[0] ? NOFAIL(strdup(namespace)) : NULL;
> > + s->namespace = NOFAIL(strdup(namespace));
> > list_add_tail(&s->list, &mod->exported_symbols);
> > hash_add_symbol(s);
> >
> > @@ -1928,8 +1935,7 @@ static void check_exports(struct module *mod)
> > else
> > basename = mod->name;
> >
> > - if (exp->namespace &&
> > - !contains_namespace(&mod->imported_namespaces, exp->namespace)) {
> > + if (!contains_namespace(&mod->imported_namespaces, exp->namespace)) {
>
> Do you still need to optimise/check for the default namespace?



I am not sure if I understood your comment, but
I changed the contains_namespace() body so that
contains_namespace(&mod->imported_namespaces, "")
returns true.











> David
>
> > modpost_log(allow_missing_ns_imports ? LOG_WARN : LOG_ERROR,
> > "module %s uses symbol %s from namespace %s, but does not import it.\n",
> > basename, exp->name, exp->namespace);
> > @@ -2015,8 +2021,7 @@ static void add_exported_symbols(struct buffer *buf, struct module *mod)
> > list_for_each_entry(sym, &mod->exported_symbols, list)
> > buf_printf(buf, "KSYMTAB_%s(%s, \"%s\", \"%s\");\n",
> > sym->is_func ? "FUNC" : "DATA", sym->name,
> > - sym->is_gpl_only ? "_gpl" : "",
> > - sym->namespace ?: "");
> > + sym->is_gpl_only ? "_gpl" : "", sym->namespace);
> >
> > if (!modversions)
> > return;
> > @@ -2284,7 +2289,7 @@ static void write_dump(const char *fname)
> > buf_printf(&buf, "0x%08x\t%s\t%s\tEXPORT_SYMBOL%s\t%s\n",
> > sym->crc, sym->name, mod->name,
> > sym->is_gpl_only ? "_GPL" : "",
> > - sym->namespace ?: "");
> > + sym->namespace);
> > }
> > }
> > write_buf(&buf, fname);
> > --
> > 2.34.1
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>


--
Best Regards
Masahiro Yamada

2022-09-28 21:39:21

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v3 0/8] Unify <linux/export.h> and <asm/export.h>, remove EXPORT_DATA_SYMBOL(), faster TRIM_UNUSED_KSYMS

On Wed, Sep 28, 2022 at 3:41 PM Masahiro Yamada <[email protected]> wrote:
>
>
> This patch set refactors EXPORT_SYMBOL, <linux/export.h> and <asm/export.h>.
> Also, re-implement TRIM_UNUSED_KSYMS in one-pass.
>
> You can still put EXPORT_SYMBOL() in *.S file, very close to the definition,
> but you do not need to care about whether it is a function or a data.
> Remove EXPORT_DATA_SYMBOL().
>
> In v1, I broke ia64 because of missing distinction between functions and data.
>
> V2 handles it correctly.
> If the exported symbols is a function, KSYMTAB_FUNC is output.
> Otherwise, KSYMTAB_DATA is output.
>



I noticed this patch set is broken in multiple ways.
No test is needed.
(0day may send various reports, but please ignore them)

I will fix the code when I have time.







Best Regards

Masahiro Yamada

2022-10-09 02:07:43

by kernel test robot

[permalink] [raw]
Subject: [kbuild] b3830bad81: System_halted

Greeting,

FYI, we noticed the following commit (built with gcc-11):

commit: b3830bad81e872632431363853c810c5f652a040 ("[PATCH v3 2/8] kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/Unify-linux-export-h-and-asm-export-h-remove-EXPORT_DATA_SYMBOL-faster-TRIM_UNUSED_KSYMS/20220928-144539
base: https://git.kernel.org/cgit/linux/kernel/git/masahiroy/linux-kbuild.git for-next
patch link: https://lore.kernel.org/linux-kbuild/[email protected]

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


early console in setup code
Probing EDD (edd=off to disable)... ok
No EFI environment detected.
early console in extract_kernel
input_data: 0x0000000002e5740d
input_len: 0x000000000099c37e
output: 0x0000000001000000
output_len: 0x000000000234aa00
kernel_total_size: 0x0000000002828000
needed_size: 0x0000000002a00000
trampoline_32bit: 0x000000000009d000

Decompressing Linux... Parsing ELF...

Alignment of LOAD segment isn't multiple of 2MB

-- System haltedBUG: kernel hang in boot stage



61682ee38a ("kbuild: move modules.builtin(.modinfo) rules to Makefile.vmlinux_o")
b3830bad81 ("kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")

+----------------+------------+------------+
| | 61682ee38a | b3830bad81 |
+----------------+------------+------------+
| boot_successes | 24 | 0 |
| boot_failures | 0 | 18 |
| System_halted | 0 | 18 |
+----------------+------------+------------+


If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/r/[email protected]


To reproduce:

# build kernel
cd linux
cp config-6.0.0-rc7-00038-gb3830bad81e8 .config
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
cd <mod-install-dir>
find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz


git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.


--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.64 kB)
config-6.0.0-rc7-00038-gb3830bad81e8 (166.42 kB)
job-script (4.85 kB)
dmesg.xz (1.36 kB)
Download all attachments

2022-10-10 19:34:44

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [kbuild] b3830bad81: System_halted

On Sun, Oct 9, 2022 at 10:21 AM kernel test robot <[email protected]> wrote:
>
> Greeting,
>
> FYI, we noticed the following commit (built with gcc-11):
>
> commit: b3830bad81e872632431363853c810c5f652a040 ("[PATCH v3 2/8] kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
> url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/Unify-linux-export-h-and-asm-export-h-remove-EXPORT_DATA_SYMBOL-faster-TRIM_UNUSED_KSYMS/20220928-144539
> base: https://git.kernel.org/cgit/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> patch link: https://lore.kernel.org/linux-kbuild/[email protected]
>
> in testcase: boot
>
> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


I think this is a false-positive alarm.

As I replied before [1], I know my patch set is broken.
I think 0day bot is testing the patch set I had already retracted.

I only picked up low-hanging fruits with fixes to my tree,
and did boot tests.

Please let me know if linux-next is broken.


[1] : https://lore.kernel.org/linux-kbuild/CAK7LNATcD6k+R66YFVg_mhe7-FGNc0nYaTPuORCcd34Qw3ra2g@mail.gmail.com/T/#t










>
> early console in setup code
> Probing EDD (edd=off to disable)... ok
> No EFI environment detected.
> early console in extract_kernel
> input_data: 0x0000000002e5740d
> input_len: 0x000000000099c37e
> output: 0x0000000001000000
> output_len: 0x000000000234aa00
> kernel_total_size: 0x0000000002828000
> needed_size: 0x0000000002a00000
> trampoline_32bit: 0x000000000009d000
>
> Decompressing Linux... Parsing ELF...
>
> Alignment of LOAD segment isn't multiple of 2MB
>
> -- System haltedBUG: kernel hang in boot stage
>
>
>
> 61682ee38a ("kbuild: move modules.builtin(.modinfo) rules to Makefile.vmlinux_o")
> b3830bad81 ("kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
>
> +----------------+------------+------------+
> | | 61682ee38a | b3830bad81 |
> +----------------+------------+------------+
> | boot_successes | 24 | 0 |
> | boot_failures | 0 | 18 |
> | System_halted | 0 | 18 |
> +----------------+------------+------------+
>
>
> If you fix the issue, kindly add following tag
> | Reported-by: kernel test robot <[email protected]>
> | Link: https://lore.kernel.org/r/[email protected]
>
>
> To reproduce:
>
> # build kernel
> cd linux
> cp config-6.0.0-rc7-00038-gb3830bad81e8 .config
> make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 olddefconfig prepare modules_prepare bzImage modules
> make HOSTCC=gcc-11 CC=gcc-11 ARCH=x86_64 INSTALL_MOD_PATH=<mod-install-dir> modules_install
> cd <mod-install-dir>
> find lib/ | cpio -o -H newc --quiet | gzip > modules.cgz
>
>
> git clone https://github.com/intel/lkp-tests.git
> cd lkp-tests
> bin/lkp qemu -k <bzImage> -m modules.cgz job-script # job-script is attached in this email
>
> # if come across any failure that blocks the test,
> # please remove ~/.lkp and /lkp dir to run from a clean state.
>
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp



--
Best Regards
Masahiro Yamada

2022-10-11 10:02:02

by kernel test robot

[permalink] [raw]
Subject: Re: [kbuild] b3830bad81: System_halted

On 10/11/2022 03:29, Masahiro Yamada wrote:
> On Sun, Oct 9, 2022 at 10:21 AM kernel test robot <[email protected]> wrote:
>>
>> Greeting,
>>
>> FYI, we noticed the following commit (built with gcc-11):
>>
>> commit: b3830bad81e872632431363853c810c5f652a040 ("[PATCH v3 2/8] kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
>> url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/Unify-linux-export-h-and-asm-export-h-remove-EXPORT_DATA_SYMBOL-faster-TRIM_UNUSED_KSYMS/20220928-144539
>> base: https://git.kernel.org/cgit/linux/kernel/git/masahiroy/linux-kbuild.git for-next
>> patch link: https://lore.kernel.org/linux-kbuild/[email protected]
>>
>> in testcase: boot
>>
>> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
>>
>> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
>
> I think this is a false-positive alarm.
>
> As I replied before [1], I know my patch set is broken.
> I think 0day bot is testing the patch set I had already retracted.
>
> I only picked up low-hanging fruits with fixes to my tree,
> and did boot tests.
>
> Please let me know if linux-next is broken.
>
>
> [1] : https://lore.kernel.org/linux-kbuild/CAK7LNATcD6k+R66YFVg_mhe7-FGNc0nYaTPuORCcd34Qw3ra2g@mail.gmail.com/T/#t
>

Sorry for this false-positive report.

Thanks for the info, we noticed that this patch has been merged into
linux-next, so we tested below commits:

b9f85101cad33 (tag: next-20221011, linux-next/master) Add linux-next specific files for 20221011
5d4aeffbf7092 kbuild: rebuild .vmlinux.export.o when its prerequisite is updated

They all passed the boot tests.

--
Best Regards,
Yujie

2022-10-11 15:35:13

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [kbuild] b3830bad81: System_halted

On Tue, Oct 11, 2022 at 6:37 PM Yujie Liu <[email protected]> wrote:
>
> On 10/11/2022 03:29, Masahiro Yamada wrote:
> > On Sun, Oct 9, 2022 at 10:21 AM kernel test robot <[email protected]> wrote:
> >>
> >> Greeting,
> >>
> >> FYI, we noticed the following commit (built with gcc-11):
> >>
> >> commit: b3830bad81e872632431363853c810c5f652a040 ("[PATCH v3 2/8] kbuild: rebuild .vmlinux.export.o when its prerequisite is updated")
> >> url: https://github.com/intel-lab-lkp/linux/commits/Masahiro-Yamada/Unify-linux-export-h-and-asm-export-h-remove-EXPORT_DATA_SYMBOL-faster-TRIM_UNUSED_KSYMS/20220928-144539
> >> base: https://git.kernel.org/cgit/linux/kernel/git/masahiroy/linux-kbuild.git for-next
> >> patch link: https://lore.kernel.org/linux-kbuild/[email protected]
> >>
> >> in testcase: boot
> >>
> >> on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
> >>
> >> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
> >
> >
> > I think this is a false-positive alarm.
> >
> > As I replied before [1], I know my patch set is broken.
> > I think 0day bot is testing the patch set I had already retracted.
> >
> > I only picked up low-hanging fruits with fixes to my tree,
> > and did boot tests.
> >
> > Please let me know if linux-next is broken.
> >
> >
> > [1] : https://lore.kernel.org/linux-kbuild/CAK7LNATcD6k+R66YFVg_mhe7-FGNc0nYaTPuORCcd34Qw3ra2g@mail.gmail.com/T/#t
> >
>
> Sorry for this false-positive report.
>
> Thanks for the info, we noticed that this patch has been merged into
> linux-next, so we tested below commits:
>
> b9f85101cad33 (tag: next-20221011, linux-next/master) Add linux-next specific files for 20221011
> 5d4aeffbf7092 kbuild: rebuild .vmlinux.export.o when its prerequisite is updated
>
> They all passed the boot tests.


Thank you for testing them!





--
Best Regards
Masahiro Yamada