Hi Linus,
Please pull below to receive modules updates for the v5.12 merge window.
A summary can be found in the signed tag.
Thank you,
Jessica
---
The following changes since commit 19c329f6808995b142b3966301f217c831e7cf31:
Linux 5.11-rc4 (2021-01-17 16:37:05 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux.git tags/modules-for-v5.12
for you to fetch changes up to 1e80d9cb579ed7edd121753eeccce82ff82521b4:
module: potential uninitialized return in module_kallsyms_on_each_symbol() (2021-02-10 16:57:04 +0100)
----------------------------------------------------------------
Modules updates for v5.12
Summary of modules changes for the 5.12 merge window:
- Retire EXPORT_UNUSED_SYMBOL() and EXPORT_SYMBOL_GPL_FUTURE(). These export
types were introduced between 2006 - 2008. All the of the unused symbols have
been long removed and gpl future symbols were converted to gpl quite a long
time ago, and I don't believe these export types have been used ever since.
So, I think it should be safe to retire those export types now. (Christoph Hellwig)
- Refactor and clean up some aged code cruft in the module loader (Christoph Hellwig)
- Build {,module_}kallsyms_on_each_symbol only when livepatching is enabled, as
it is the only caller (Christoph Hellwig)
- Unexport find_module() and module_mutex and fix the last module
callers to not rely on these anymore. Make module_mutex internal to
the module loader. (Christoph Hellwig)
- Harden ELF checks on module load and validate ELF structures before checking
the module signature (Frank van der Linden)
- Fix undefined symbol warning for clang (Fangrui Song)
- Fix smatch warning (Dan Carpenter)
Signed-off-by: Jessica Yu <[email protected]>
----------------------------------------------------------------
Christoph Hellwig (13):
powerpc/powernv: remove get_cxl_module
drm: remove drm_fb_helper_modinit
module: unexport find_module and module_mutex
module: use RCU to synchronize find_module
kallsyms: refactor {,module_}kallsyms_on_each_symbol
kallsyms: only build {,module_}kallsyms_on_each_symbol when required
module: mark module_mutex static
module: remove each_symbol_in_section
module: merge each_symbol_section into find_symbol
module: pass struct find_symbol_args to find_symbol
module: move struct symsearch to module.c
module: remove EXPORT_SYMBOL_GPL_FUTURE
module: remove EXPORT_UNUSED_SYMBOL*
Dan Carpenter (1):
module: potential uninitialized return in module_kallsyms_on_each_symbol()
Fangrui Song (1):
module: Ignore _GLOBAL_OFFSET_TABLE_ when warning for undefined symbols
Frank van der Linden (1):
module: harden ELF info handling
arch/arm/configs/bcm2835_defconfig | 1 -
arch/arm/configs/mxs_defconfig | 1 -
arch/mips/configs/nlm_xlp_defconfig | 1 -
arch/mips/configs/nlm_xlr_defconfig | 1 -
arch/parisc/configs/generic-32bit_defconfig | 1 -
arch/parisc/configs/generic-64bit_defconfig | 1 -
arch/powerpc/configs/ppc6xx_defconfig | 1 -
arch/powerpc/platforms/powernv/pci-cxl.c | 22 --
arch/s390/configs/debug_defconfig | 1 -
arch/s390/configs/defconfig | 1 -
arch/sh/configs/edosk7760_defconfig | 1 -
arch/sh/configs/sdk7780_defconfig | 1 -
arch/x86/configs/i386_defconfig | 1 -
arch/x86/configs/x86_64_defconfig | 1 -
arch/x86/tools/relocs.c | 4 +-
drivers/gpu/drm/drm_crtc_helper_internal.h | 10 -
drivers/gpu/drm/drm_fb_helper.c | 21 --
drivers/gpu/drm/drm_kms_helper_common.c | 25 +-
include/asm-generic/vmlinux.lds.h | 42 ---
include/linux/export.h | 9 -
include/linux/kallsyms.h | 17 +-
include/linux/module.h | 48 +--
init/Kconfig | 17 -
kernel/kallsyms.c | 8 +-
kernel/livepatch/core.c | 7 +-
kernel/module.c | 481 ++++++++++++++--------------
kernel/module_signature.c | 2 +-
kernel/module_signing.c | 2 +-
kernel/trace/trace_kprobe.c | 4 +-
lib/bug.c | 3 -
scripts/checkpatch.pl | 6 +-
scripts/mod/modpost.c | 50 +--
scripts/mod/modpost.h | 3 -
scripts/module.lds.S | 6 -
tools/include/linux/export.h | 3 -
35 files changed, 287 insertions(+), 516 deletions(-)
On Tue, Feb 23, 2021 at 7:42 AM Jessica Yu <[email protected]> wrote:
>
> Please pull below to receive modules updates for the v5.12 merge window.
"struct symsearch is only used inside of module.h, so move the definition
out of module.h"
Whaa?
The first module.h should be module.c. Oh well.
Pulled.
Linus
On Tue, Feb 23, 2021 at 11:55:50AM -0800, Linus Torvalds wrote:
> On Tue, Feb 23, 2021 at 10:42 AM Linus Torvalds
> <[email protected]> wrote:
> >
> > I think there is something horribly wrong in my tree, and my build
> > process is now about 30% slower. It went from 5+ minutes to 8+
> > minutes. The main suspect would be some lack of parallelism.
>
> I don't see quite what is wrong, but bisection is clear, and points
> the finger at
>
> 367948220fce "module: remove EXPORT_UNUSED_SYMBOL*"
>
> which looks entirely trivial, but clearly isn't.
>
> It's repeatable. That commit slows down my build hugely.
Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
chance?
>
> Linus
---end quoted text---
On Tue, Feb 23, 2021 at 11:55 AM Linus Torvalds
<[email protected]> wrote:
>
> I don't see quite what is wrong, but bisection is clear, and points
> the finger at
>
> 367948220fce "module: remove EXPORT_UNUSED_SYMBOL*"
>
> which looks entirely trivial, but clearly isn't.
>
> It's repeatable. That commit slows down my build hugely.
Hmm. I'm starting to suspect that the problem is that the removal of
CONFIG_UNUSED_SYMBOLS now means that we always trigger
CONFIG_TRIM_UNUSED_KSYMS instead.
And then that CONFIG_TRIM_UNUSED_KSYMS is some hugely expensive operation.
Linus
On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
<[email protected]> wrote:
>
> This is unacceptably slow. If that symbol trimming takes 30% of the
> whole kernel build time, it needs to be fixed or removed.
I think I'm going to mark TRIM_UNUSED_KSYMS as "depends on BROKEN".
There's no way I can accept that horrible overhead, and the rationale
for that config option is questionable at best, and no distro will
ever enable it anyway since they all accept external modules.
If somebody cares about it, they can fix the horrendous build costs.
Linus
On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <[email protected]> wrote:
>
> Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
> chance?
Crossed emails.
This is plain "make allmodconfig", so yes, now it will enable TRIM_UNUSED_KSYMS.
This is unacceptably slow. If that symbol trimming takes 30% of the
whole kernel build time, it needs to be fixed or removed.
Linus
The pull request you sent on Tue, 23 Feb 2021 16:42:36 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux.git tags/modules-for-v5.12
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/21a6ab2131ab0644eeef70507e20273338bf065c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On Tue, Feb 23, 2021 at 12:07:39PM -0800, Linus Torvalds wrote:
> On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
> <[email protected]> wrote:
> >
> > This is unacceptably slow. If that symbol trimming takes 30% of the
> > whole kernel build time, it needs to be fixed or removed.
>
> I think I'm going to mark TRIM_UNUSED_KSYMS as "depends on BROKEN".
> There's no way I can accept that horrible overhead, and the rationale
> for that config option is questionable at best,
I think it is pretty useful for embedded setups.
BROKEN seems pretty strong for something that absolutely works as
intendended. I guess to make you (and possibly others) not grumpy
we just need to ensure it doesn't get pulled in by allmodconfig.
So maybe just invert the symbol, default the KEEP_UNUSED_SYMBOL, and
add a message to the helptext explaining the slowdown?
+++ Linus Torvalds [23/02/21 12:03 -0800]:
>On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <[email protected]> wrote:
>>
>> Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
>> chance?
>
>Crossed emails.
>
>This is plain "make allmodconfig", so yes, now it will enable TRIM_UNUSED_KSYMS.
>
>This is unacceptably slow. If that symbol trimming takes 30% of the
>whole kernel build time, it needs to be fixed or removed.
[ Adding Masahiro to CC ]
It looks like CONFIG_TRIM_UNUSED_KSYMS had been hiding behind
CONFIG_UNUSED_SYMBOLS all this time, and once the EXPORT_UNUSED_SYMBOL
stuff was removed, it exposed that option to be selected by
allyesconfig. That option had previously caused build issues on
powerpc on linux-next, so I had temporarily marked that as BROKEN on
powerpc until Masahiro's fix landed in linux-next. I was not aware of
the additional build slowdown issue :/ In any case, Christoph's
suggestion to invert the option sounds reasonable, since the mips
defconfig selects it, it does not seem totally unused.
+++ Christoph Hellwig [24/02/21 08:52 +0100]:
>On Tue, Feb 23, 2021 at 12:07:39PM -0800, Linus Torvalds wrote:
>> On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
>> <[email protected]> wrote:
>> >
>> > This is unacceptably slow. If that symbol trimming takes 30% of the
>> > whole kernel build time, it needs to be fixed or removed.
>>
>> I think I'm going to mark TRIM_UNUSED_KSYMS as "depends on BROKEN".
>> There's no way I can accept that horrible overhead, and the rationale
>> for that config option is questionable at best,
>
>I think it is pretty useful for embedded setups.
>
>BROKEN seems pretty strong for something that absolutely works as
>intendended. I guess to make you (and possibly others) not grumpy
>we just need to ensure it doesn't get pulled in by allmodconfig.
>
>So maybe just invert the symbol, default the KEEP_UNUSED_SYMBOL, and
>add a message to the helptext explaining the slowdown?
Hm, something like this maybe? (untested)
---
From 08bc08229fc3801b1a580a07ce7ff3e806b3fe90 Mon Sep 17 00:00:00 2001
From: Jessica Yu <[email protected]>
Date: Wed, 24 Feb 2021 14:54:09 +0100
Subject: [PATCH] Kconfig: invert TRIM_UNUSED_SYMBOLS option and rename it to
KEEP_UNUSED_SYMBOLS
Removing CONFIG_UNUSED_SYMBOLS (commit 367948220fce "module: remove
EXPORT_UNUSED_SYMBOL*") unhid the CONFIG_TRIM_UNUSED_SYMBOLS option and
therefore it now gets automatically enabled with allyesconfig.
To prevent allyesconfig from enabling TRIM_UNUSED_SYMBOLS (which is known
to slow build times), invert the config option and name it
KEEP_UNUSED_SYMBOLS, which does the same thing as TRIM_UNUSED_SYMBOLS=n.
That way, allyesconfig will keep the previous behavior of not trimming
symbols.
Signed-off-by: Jessica Yu <[email protected]>
---
Makefile | 4 ++--
arch/mips/configs/generic_defconfig | 2 +-
include/asm-generic/export.h | 2 +-
include/linux/export.h | 2 +-
init/Kconfig | 21 +++++++++++----------
kernel/livepatch/Kconfig | 2 +-
scripts/Makefile.build | 2 +-
7 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/Makefile b/Makefile
index b18dbc634690..23f50521e97f 100644
--- a/Makefile
+++ b/Makefile
@@ -1164,7 +1164,7 @@ vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)
# Recurse until adjust_autoksyms.sh is satisfied
PHONY += autoksyms_recursive
-ifdef CONFIG_TRIM_UNUSED_KSYMS
+ifndef CONFIG_KEEP_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)
@@ -1175,7 +1175,7 @@ autoksyms_recursive: descend modules.order
"$(MAKE) -f $(srctree)/Makefile vmlinux"
endif
-autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
+autoksyms_h := $(if $(CONFIG_KEEP_UNUSED_KSYMS),,include/generated/autoksyms.h)
quiet_cmd_autoksyms_h = GEN $@
cmd_autoksyms_h = mkdir -p $(dir $@); \
diff --git a/arch/mips/configs/generic_defconfig b/arch/mips/configs/generic_defconfig
index 714169e411cf..d46da28ea032 100644
--- a/arch/mips/configs/generic_defconfig
+++ b/arch/mips/configs/generic_defconfig
@@ -29,7 +29,7 @@ CONFIG_MIPS_O32_FP64_SUPPORT=y
CONFIG_JUMP_LABEL=y
CONFIG_MODULES=y
CONFIG_MODULE_UNLOAD=y
-CONFIG_TRIM_UNUSED_KSYMS=y
+CONFIG_KEEP_UNUSED_KSYMS=n
CONFIG_NET=y
CONFIG_PACKET=y
CONFIG_UNIX=y
diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
index 07a36a874dca..06d401464195 100644
--- a/include/asm-generic/export.h
+++ b/include/asm-generic/export.h
@@ -57,7 +57,7 @@ __kstrtab_\name:
#endif
.endm
-#if defined(CONFIG_TRIM_UNUSED_KSYMS)
+#if !defined(CONFIG_KEEP_UNUSED_KSYMS)
#include <linux/kconfig.h>
#include <generated/autoksyms.h>
diff --git a/include/linux/export.h b/include/linux/export.h
index 6271a5d9c988..449f7d15e580 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -118,7 +118,7 @@ struct kernel_symbol {
*/
#define __EXPORT_SYMBOL(sym, sec, ns)
-#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
+#elif !defined(CONFIG_KEEP_UNUSED_KSYMS)
#include <generated/autoksyms.h>
diff --git a/init/Kconfig b/init/Kconfig
index ba8bd5256980..db5d00bfc239 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2272,29 +2272,30 @@ config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
If unsure, say N.
-config TRIM_UNUSED_KSYMS
- bool "Trim unused exported kernel symbols"
- depends on BROKEN
+config KEEP_UNUSED_KSYMS
+ bool "Keep unused exported kernel symbols"
+ default y
help
The kernel and some modules make many symbols available for
other modules to use via EXPORT_SYMBOL() and variants. Depending
on the set of modules being selected in your kernel configuration,
many of those exported symbols might never be used.
- This option allows for unused exported symbols to be dropped from
- the build. In turn, this provides the compiler more opportunities
- (especially when using LTO) for optimizing the code and reducing
- binary size. This might have some security advantages as well.
+ This option allows for unused exported symbols to be kept in the
+ build. Say N when you want to trim unused symbols from the build,
+ which provides the compiler more opportunities (especially when using LTO)
+ for optimizing the code and reducing binary size. This might have some
+ security advantages as well.
- If unsure, or if you need to build out-of-tree modules, say N.
+ If unsure, or if you need to build out-of-tree modules, say Y.
config UNUSED_KSYMS_WHITELIST
string "Whitelist of symbols to keep in ksymtab"
- depends on TRIM_UNUSED_KSYMS
+ depends on !KEEP_UNUSED_KSYMS
default "scripts/lto-used-symbollist.txt" if LTO_CLANG
help
By default, all unused exported symbols will be un-exported from the
- build when TRIM_UNUSED_KSYMS is selected.
+ build when KEEP_UNUSED_KSYMS is not selected.
UNUSED_KSYMS_WHITELIST allows to whitelist symbols that must be kept
exported at all times, even in absence of in-tree users. The value to
diff --git a/kernel/livepatch/Kconfig b/kernel/livepatch/Kconfig
index 53d51ed619a3..df8ebb7984e1 100644
--- a/kernel/livepatch/Kconfig
+++ b/kernel/livepatch/Kconfig
@@ -11,7 +11,7 @@ config LIVEPATCH
depends on SYSFS
depends on KALLSYMS_ALL
depends on HAVE_LIVEPATCH
- depends on !TRIM_UNUSED_KSYMS
+ depends on KEEP_UNUSED_KSYMS
help
Say Y here if you want to support kernel live patching.
This option has no runtime impact until a kernel "patch"
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 3f6bf0ea7c0e..e5e95a6948a7 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -242,7 +242,7 @@ objtool_dep = $(objtool_obj) \
$(wildcard include/config/orc/unwinder.h \
include/config/stack/validation.h)
-ifdef CONFIG_TRIM_UNUSED_KSYMS
+ifndef CONFIG_KEEP_UNUSED_KSYMS
cmd_gen_ksymdeps = \
$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <[email protected]> wrote:
>
> +++ Linus Torvalds [23/02/21 12:03 -0800]:
> >On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <[email protected]> wrote:
> >>
> >> Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
> >> chance?
> >
> >Crossed emails.
> >
> >This is plain "make allmodconfig", so yes, now it will enable TRIM_UNUSED_KSYMS.
> >
> >This is unacceptably slow. If that symbol trimming takes 30% of the
> >whole kernel build time, it needs to be fixed or removed.
>
> [ Adding Masahiro to CC ]
>
> It looks like CONFIG_TRIM_UNUSED_KSYMS had been hiding behind
> CONFIG_UNUSED_SYMBOLS all this time, and once the EXPORT_UNUSED_SYMBOL
> stuff was removed, it exposed that option to be selected by
> allyesconfig. That option had previously caused build issues on
> powerpc on linux-next, so I had temporarily marked that as BROKEN on
> powerpc until Masahiro's fix landed in linux-next. I was not aware of
> the additional build slowdown issue :/ In any case, Christoph's
> suggestion to invert the option sounds reasonable, since the mips
> defconfig selects it, it does not seem totally unused.
TRIM_UNUSED_KSYMS builds the tree twice by its concept.
[1] 1st build
At this point of time, we do not know which EXPORT_SYMBOL()
is needed. So, EXPORT_SYMBOL() is enabled, or noop'ed
based on the temporal guess.
(in the fresh build, EXPORT_SYMBOL() are all nooped.)
[2] Get the list of symbols needed to resolve all symbol references.
(this information is collected in include/generated/autoksyms.h)
[3] 2nd build
Rebuild the objects whose EXPORT_SYMBOL()
must be flipped.
The build system cleverly tracks which object needs rebuild.
So, building the tree twice does not mean
the build cost is twice.
But, 30% increase is reasonable.
In my understanding, TRIM_UNUSED_KSYMS is used
by Android. (Generic Kernel Image)
So, we should revive it.
--
Best Regards
Masahiro Yamada
On Wed, Feb 24, 2021 at 11:13 PM Jessica Yu <[email protected]> wrote:
>
> +++ Christoph Hellwig [24/02/21 08:52 +0100]:
> >On Tue, Feb 23, 2021 at 12:07:39PM -0800, Linus Torvalds wrote:
> >> On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
> >> <[email protected]> wrote:
> >> >
> >> > This is unacceptably slow. If that symbol trimming takes 30% of the
> >> > whole kernel build time, it needs to be fixed or removed.
> >>
> >> I think I'm going to mark TRIM_UNUSED_KSYMS as "depends on BROKEN".
> >> There's no way I can accept that horrible overhead, and the rationale
> >> for that config option is questionable at best,
> >
> >I think it is pretty useful for embedded setups.
> >
> >BROKEN seems pretty strong for something that absolutely works as
> >intendended. I guess to make you (and possibly others) not grumpy
> >we just need to ensure it doesn't get pulled in by allmodconfig.
> >
> >So maybe just invert the symbol, default the KEEP_UNUSED_SYMBOL, and
> >add a message to the helptext explaining the slowdown?
>
> Hm, something like this maybe? (untested)
I prefer a one-liner, 'depends on !COMPILE_TEST'.
!COMPILE_TEST is not super elegant, but
it is used in several spaces.
Inverting the CONFIG option is just a workaround.
A patch with many changes is not worth it.
> ---
> From 08bc08229fc3801b1a580a07ce7ff3e806b3fe90 Mon Sep 17 00:00:00 2001
> From: Jessica Yu <[email protected]>
> Date: Wed, 24 Feb 2021 14:54:09 +0100
> Subject: [PATCH] Kconfig: invert TRIM_UNUSED_SYMBOLS option and rename it to
> KEEP_UNUSED_SYMBOLS
>
> Removing CONFIG_UNUSED_SYMBOLS (commit 367948220fce "module: remove
> EXPORT_UNUSED_SYMBOL*") unhid the CONFIG_TRIM_UNUSED_SYMBOLS option and
> therefore it now gets automatically enabled with allyesconfig.
>
> To prevent allyesconfig from enabling TRIM_UNUSED_SYMBOLS (which is known
> to slow build times), invert the config option and name it
> KEEP_UNUSED_SYMBOLS, which does the same thing as TRIM_UNUSED_SYMBOLS=n.
> That way, allyesconfig will keep the previous behavior of not trimming
> symbols.
>
> Signed-off-by: Jessica Yu <[email protected]>
> ---
> Makefile | 4 ++--
> arch/mips/configs/generic_defconfig | 2 +-
> include/asm-generic/export.h | 2 +-
> include/linux/export.h | 2 +-
> init/Kconfig | 21 +++++++++++----------
> kernel/livepatch/Kconfig | 2 +-
> scripts/Makefile.build | 2 +-
> 7 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index b18dbc634690..23f50521e97f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1164,7 +1164,7 @@ vmlinux-deps := $(KBUILD_LDS) $(KBUILD_VMLINUX_OBJS) $(KBUILD_VMLINUX_LIBS)
>
> # Recurse until adjust_autoksyms.sh is satisfied
> PHONY += autoksyms_recursive
> -ifdef CONFIG_TRIM_UNUSED_KSYMS
> +ifndef CONFIG_KEEP_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)
> @@ -1175,7 +1175,7 @@ autoksyms_recursive: descend modules.order
> "$(MAKE) -f $(srctree)/Makefile vmlinux"
> endif
>
> -autoksyms_h := $(if $(CONFIG_TRIM_UNUSED_KSYMS), include/generated/autoksyms.h)
> +autoksyms_h := $(if $(CONFIG_KEEP_UNUSED_KSYMS),,include/generated/autoksyms.h)
>
> quiet_cmd_autoksyms_h = GEN $@
> cmd_autoksyms_h = mkdir -p $(dir $@); \
> diff --git a/arch/mips/configs/generic_defconfig b/arch/mips/configs/generic_defconfig
> index 714169e411cf..d46da28ea032 100644
> --- a/arch/mips/configs/generic_defconfig
> +++ b/arch/mips/configs/generic_defconfig
> @@ -29,7 +29,7 @@ CONFIG_MIPS_O32_FP64_SUPPORT=y
> CONFIG_JUMP_LABEL=y
> CONFIG_MODULES=y
> CONFIG_MODULE_UNLOAD=y
> -CONFIG_TRIM_UNUSED_KSYMS=y
> +CONFIG_KEEP_UNUSED_KSYMS=n
> CONFIG_NET=y
> CONFIG_PACKET=y
> CONFIG_UNIX=y
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index 07a36a874dca..06d401464195 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -57,7 +57,7 @@ __kstrtab_\name:
> #endif
> .endm
>
> -#if defined(CONFIG_TRIM_UNUSED_KSYMS)
> +#if !defined(CONFIG_KEEP_UNUSED_KSYMS)
>
> #include <linux/kconfig.h>
> #include <generated/autoksyms.h>
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 6271a5d9c988..449f7d15e580 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -118,7 +118,7 @@ struct kernel_symbol {
> */
> #define __EXPORT_SYMBOL(sym, sec, ns)
>
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +#elif !defined(CONFIG_KEEP_UNUSED_KSYMS)
>
> #include <generated/autoksyms.h>
>
> diff --git a/init/Kconfig b/init/Kconfig
> index ba8bd5256980..db5d00bfc239 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -2272,29 +2272,30 @@ config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
>
> If unsure, say N.
>
> -config TRIM_UNUSED_KSYMS
> - bool "Trim unused exported kernel symbols"
> - depends on BROKEN
> +config KEEP_UNUSED_KSYMS
> + bool "Keep unused exported kernel symbols"
> + default y
> help
> The kernel and some modules make many symbols available for
> other modules to use via EXPORT_SYMBOL() and variants. Depending
> on the set of modules being selected in your kernel configuration,
> many of those exported symbols might never be used.
>
> - This option allows for unused exported symbols to be dropped from
> - the build. In turn, this provides the compiler more opportunities
> - (especially when using LTO) for optimizing the code and reducing
> - binary size. This might have some security advantages as well.
> + This option allows for unused exported symbols to be kept in the
> + build. Say N when you want to trim unused symbols from the build,
> + which provides the compiler more opportunities (especially when using LTO)
> + for optimizing the code and reducing binary size. This might have some
> + security advantages as well.
>
> - If unsure, or if you need to build out-of-tree modules, say N.
> + If unsure, or if you need to build out-of-tree modules, say Y.
>
> config UNUSED_KSYMS_WHITELIST
> string "Whitelist of symbols to keep in ksymtab"
> - depends on TRIM_UNUSED_KSYMS
> + depends on !KEEP_UNUSED_KSYMS
> default "scripts/lto-used-symbollist.txt" if LTO_CLANG
> help
> By default, all unused exported symbols will be un-exported from the
> - build when TRIM_UNUSED_KSYMS is selected.
> + build when KEEP_UNUSED_KSYMS is not selected.
>
> UNUSED_KSYMS_WHITELIST allows to whitelist symbols that must be kept
> exported at all times, even in absence of in-tree users. The value to
> diff --git a/kernel/livepatch/Kconfig b/kernel/livepatch/Kconfig
> index 53d51ed619a3..df8ebb7984e1 100644
> --- a/kernel/livepatch/Kconfig
> +++ b/kernel/livepatch/Kconfig
> @@ -11,7 +11,7 @@ config LIVEPATCH
> depends on SYSFS
> depends on KALLSYMS_ALL
> depends on HAVE_LIVEPATCH
> - depends on !TRIM_UNUSED_KSYMS
> + depends on KEEP_UNUSED_KSYMS
> help
> Say Y here if you want to support kernel live patching.
> This option has no runtime impact until a kernel "patch"
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 3f6bf0ea7c0e..e5e95a6948a7 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -242,7 +242,7 @@ objtool_dep = $(objtool_obj) \
> $(wildcard include/config/orc/unwinder.h \
> include/config/stack/validation.h)
>
> -ifdef CONFIG_TRIM_UNUSED_KSYMS
> +ifndef CONFIG_KEEP_UNUSED_KSYMS
> cmd_gen_ksymdeps = \
> $(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
>
--
Best Regards
Masahiro Yamada
On Wed, Feb 24, 2021 at 11:46 PM Masahiro Yamada <[email protected]> wrote:
>
> On Wed, Feb 24, 2021 at 11:13 PM Jessica Yu <[email protected]> wrote:
> >
> > +++ Christoph Hellwig [24/02/21 08:52 +0100]:
> > >On Tue, Feb 23, 2021 at 12:07:39PM -0800, Linus Torvalds wrote:
> > >> On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
> > >> <[email protected]> wrote:
> > >> >
> > >> > This is unacceptably slow. If that symbol trimming takes 30% of the
> > >> > whole kernel build time, it needs to be fixed or removed.
> > >>
> > >> I think I'm going to mark TRIM_UNUSED_KSYMS as "depends on BROKEN".
> > >> There's no way I can accept that horrible overhead, and the rationale
> > >> for that config option is questionable at best,
> > >
> > >I think it is pretty useful for embedded setups.
> > >
> > >BROKEN seems pretty strong for something that absolutely works as
> > >intendended. I guess to make you (and possibly others) not grumpy
> > >we just need to ensure it doesn't get pulled in by allmodconfig.
> > >
> > >So maybe just invert the symbol, default the KEEP_UNUSED_SYMBOL, and
> > >add a message to the helptext explaining the slowdown?
> >
> > Hm, something like this maybe? (untested)
>
A patch attached, if Linus is OK to re-enable this.
--
Best Regards
Masahiro Yamada
On Wed, Feb 24, 2021 at 6:47 AM Masahiro Yamada <[email protected]> wrote:
>
> I prefer a one-liner, 'depends on !COMPILE_TEST'.
> !COMPILE_TEST is not super elegant, but
> it is used in several spaces.
Yeah, I'll do that.
I'll also make it an EXPERT-only question, which is what we tend to do
for a lot of special "only for people who really know what they are
doing" things.
Linus
On 24/02/2021 15.40, Masahiro Yamada wrote:
> On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <[email protected]> wrote:
>>
>> +++ Linus Torvalds [23/02/21 12:03 -0800]:
>>> On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <[email protected]> wrote:
>>>>
>>>> Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
>>>> chance?
>>>
>>> Crossed emails.
>>>
>>> This is plain "make allmodconfig", so yes, now it will enable TRIM_UNUSED_KSYMS.
>>>
>>> This is unacceptably slow. If that symbol trimming takes 30% of the
>>> whole kernel build time, it needs to be fixed or removed.
>>
>> [ Adding Masahiro to CC ]
>>
>> It looks like CONFIG_TRIM_UNUSED_KSYMS had been hiding behind
>> CONFIG_UNUSED_SYMBOLS all this time, and once the EXPORT_UNUSED_SYMBOL
>> stuff was removed, it exposed that option to be selected by
>> allyesconfig. That option had previously caused build issues on
>> powerpc on linux-next, so I had temporarily marked that as BROKEN on
>> powerpc until Masahiro's fix landed in linux-next. I was not aware of
>> the additional build slowdown issue :/ In any case, Christoph's
>> suggestion to invert the option sounds reasonable, since the mips
>> defconfig selects it, it does not seem totally unused.
>
>
> TRIM_UNUSED_KSYMS builds the tree twice by its concept.
>
> [1] 1st build
> At this point of time, we do not know which EXPORT_SYMBOL()
> is needed. So, EXPORT_SYMBOL() is enabled, or noop'ed
> based on the temporal guess.
> (in the fresh build, EXPORT_SYMBOL() are all nooped.)
>
> [2] Get the list of symbols needed to resolve all symbol references.
> (this information is collected in include/generated/autoksyms.h)
>
> [3] 2nd build
> Rebuild the objects whose EXPORT_SYMBOL()
> must be flipped.
I'm thinking we should be able to generate a linker script snippet from
[2] and use that when linking vmlinux, so there's no recursion and no
rebuild of individual .o files (and all the __cond_export_sym trickery
goes away).
The ksymtab entry for foo is already emitted in its own ___ksymtab+foo
section (or ___ksymtab_gpl+foo). So if the sorted list of undefined
symbols listed in the .mod files (plus the whitelist) consist of foo,
bar and baz, generate a header to be included by vmlinux.lds.h that says
#define KSYMTAB_SECTIONS \
___ksymtab+foo \
___ksymtab+bar \
___ksymtab+baz \
#define KSYMTAB_GPL_SECTIONS \
___ksymtab_gpl+foo \
___ksymtab_gpl+bar \
___ksymtab_gpl+baz \
with a !CONFIG_TRIM_UNUSED_KSYMS definition of these that just says
#define KSYMTAB_SECTIONS ___ksymtab+*
#define KSYMTAB_GPL_SECTIONS ___ksymtab_gpl+*
and use that
__ksymtab AT(ADDR(__ksymtab) - LOAD_OFFSET) { \
__start___ksymtab = .; \
KEEP(*(SORT(KSYMTAB_SECTIONS))) \
__stop___ksymtab = .; \
Only one of ___ksymtab+foo and ___ksymtab_gpl+foo will exist, but that
doesn't matter (it's really no different from the fact that many files
(i.e. the * before "(SORT") don't contain any section matching
___ksymtab_gpl+*).
We may then have to add another discard section to put the remaining
___ksymtab_gpl+* sections in, but that's fine as long as that stanza
just appears later in the linker script.
If LD_DEAD_CODE_DATA_ELIMINATION was more widely supported (and I was
surprised to see that it's not even available on arm or x86) one could
also play another game, dropping the KEEP()s and instead create a linker
script snippet containing EXTERN(__ksymtab_foo __ksymtab_bar ...),
referencing the "struct kernel_symbol" elements themselves rather than
the singleton sections they reside in.
Rasmus
On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 24/02/2021 15.40, Masahiro Yamada wrote:
> > On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <[email protected]> wrote:
> >>
> >> +++ Linus Torvalds [23/02/21 12:03 -0800]:
> >>> On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <[email protected]> wrote:
> >>>>
> >>>> Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
> >>>> chance?
> >>>
> >>> Crossed emails.
> >>>
> >>> This is plain "make allmodconfig", so yes, now it will enable TRIM_UNUSED_KSYMS.
> >>>
> >>> This is unacceptably slow. If that symbol trimming takes 30% of the
> >>> whole kernel build time, it needs to be fixed or removed.
> >>
> >> [ Adding Masahiro to CC ]
> >>
> >> It looks like CONFIG_TRIM_UNUSED_KSYMS had been hiding behind
> >> CONFIG_UNUSED_SYMBOLS all this time, and once the EXPORT_UNUSED_SYMBOL
> >> stuff was removed, it exposed that option to be selected by
> >> allyesconfig. That option had previously caused build issues on
> >> powerpc on linux-next, so I had temporarily marked that as BROKEN on
> >> powerpc until Masahiro's fix landed in linux-next. I was not aware of
> >> the additional build slowdown issue :/ In any case, Christoph's
> >> suggestion to invert the option sounds reasonable, since the mips
> >> defconfig selects it, it does not seem totally unused.
Good insight.
Actually, I came up with the same idea last night, and had started
the implementation background.
I needed sleep before completing the patch set, but
now it is working as far as I tested.
BTW,
KEEP(*(SORT(___ksymtab+foo ___ksymtab+bar ___ksymtab+baz))
is a syntax error.
KEEP(*(__ksymtab+foo))
KEEP(*(__ksymtab+bar))
KEEP(*(__ksymtab+baz))
works.
> >
> > TRIM_UNUSED_KSYMS builds the tree twice by its concept.
> >
> > [1] 1st build
> > At this point of time, we do not know which EXPORT_SYMBOL()
> > is needed. So, EXPORT_SYMBOL() is enabled, or noop'ed
> > based on the temporal guess.
> > (in the fresh build, EXPORT_SYMBOL() are all nooped.)
> >
> > [2] Get the list of symbols needed to resolve all symbol references.
> > (this information is collected in include/generated/autoksyms.h)
> >
> > [3] 2nd build
> > Rebuild the objects whose EXPORT_SYMBOL()
> > must be flipped.
>
> I'm thinking we should be able to generate a linker script snippet from
> [2] and use that when linking vmlinux, so there's no recursion and no
> rebuild of individual .o files (and all the __cond_export_sym trickery
> goes away).
>
> The ksymtab entry for foo is already emitted in its own ___ksymtab+foo
> section (or ___ksymtab_gpl+foo). So if the sorted list of undefined
> symbols listed in the .mod files (plus the whitelist) consist of foo,
> bar and baz, generate a header to be included by vmlinux.lds.h that says
>
> #define KSYMTAB_SECTIONS \
> ___ksymtab+foo \
> ___ksymtab+bar \
> ___ksymtab+baz \
>
> #define KSYMTAB_GPL_SECTIONS \
> ___ksymtab_gpl+foo \
> ___ksymtab_gpl+bar \
> ___ksymtab_gpl+baz \
>
> with a !CONFIG_TRIM_UNUSED_KSYMS definition of these that just says
>
> #define KSYMTAB_SECTIONS ___ksymtab+*
> #define KSYMTAB_GPL_SECTIONS ___ksymtab_gpl+*
>
> and use that
>
> __ksymtab AT(ADDR(__ksymtab) - LOAD_OFFSET) { \
> __start___ksymtab = .; \
> KEEP(*(SORT(KSYMTAB_SECTIONS))) \
> __stop___ksymtab = .; \
>
> Only one of ___ksymtab+foo and ___ksymtab_gpl+foo will exist, but that
> doesn't matter (it's really no different from the fact that many files
> (i.e. the * before "(SORT") don't contain any section matching
> ___ksymtab_gpl+*).
>
> We may then have to add another discard section to put the remaining
> ___ksymtab_gpl+* sections in, but that's fine as long as that stanza
> just appears later in the linker script.
>
> If LD_DEAD_CODE_DATA_ELIMINATION was more widely supported (and I was
> surprised to see that it's not even available on arm or x86) one could
> also play another game, dropping the KEEP()s and instead create a linker
> script snippet containing EXTERN(__ksymtab_foo __ksymtab_bar ...),
> referencing the "struct kernel_symbol" elements themselves rather than
> the singleton sections they reside in.
Do you mean LD_DEAD_CODE_DATA_ELIMINATION must be enabled by default
to do this?
>
> Rasmus
--
Best Regards
Masahiro Yamada
On Fri, Feb 26, 2021 at 12:49 AM Masahiro Yamada <[email protected]> wrote:
>
> On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
> <[email protected]> wrote:
> >
> > On 24/02/2021 15.40, Masahiro Yamada wrote:
> > > On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <[email protected]> wrote:
> > >>
> > >> +++ Linus Torvalds [23/02/21 12:03 -0800]:
> > >>> On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <[email protected]> wrote:
> > >>>>
> > >>>> Does your build now enable TRIM_UNUSED_KSYMS but previously didn't by
> > >>>> chance?
> > >>>
> > >>> Crossed emails.
> > >>>
> > >>> This is plain "make allmodconfig", so yes, now it will enable TRIM_UNUSED_KSYMS.
> > >>>
> > >>> This is unacceptably slow. If that symbol trimming takes 30% of the
> > >>> whole kernel build time, it needs to be fixed or removed.
> > >>
> > >> [ Adding Masahiro to CC ]
> > >>
> > >> It looks like CONFIG_TRIM_UNUSED_KSYMS had been hiding behind
> > >> CONFIG_UNUSED_SYMBOLS all this time, and once the EXPORT_UNUSED_SYMBOL
> > >> stuff was removed, it exposed that option to be selected by
> > >> allyesconfig. That option had previously caused build issues on
> > >> powerpc on linux-next, so I had temporarily marked that as BROKEN on
> > >> powerpc until Masahiro's fix landed in linux-next. I was not aware of
> > >> the additional build slowdown issue :/ In any case, Christoph's
> > >> suggestion to invert the option sounds reasonable, since the mips
> > >> defconfig selects it, it does not seem totally unused.
>
>
> Good insight.
> Actually, I came up with the same idea last night, and had started
> the implementation background.
> I needed sleep before completing the patch set, but
> now it is working as far as I tested.
>
> BTW,
> KEEP(*(SORT(___ksymtab+foo ___ksymtab+bar ___ksymtab+baz))
> is a syntax error.
>
> KEEP(*(__ksymtab+foo))
> KEEP(*(__ksymtab+bar))
> KEEP(*(__ksymtab+baz))
>
> works.
>
>
Sorry, I missed to CC you.
This patch set.
https://lore.kernel.org/patchwork/project/lkml/list/?series=486545
--
Best Regards
Masahiro Yamada
On 25/02/2021 16.49, Masahiro Yamada wrote:
> On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
> <[email protected]> wrote:
>>
>> On 24/02/2021 15.40, Masahiro Yamada wrote:
>
>
> Good insight.
> Actually, I came up with the same idea last night, and had started
> the implementation background.
> I needed sleep before completing the patch set, but
> now it is working as far as I tested.
>
> BTW,
> KEEP(*(SORT(___ksymtab+foo ___ksymtab+bar ___ksymtab+baz))
> is a syntax error.
>
ah, ok, didn't test anything, just threw it out there in case somebody
wanted to see if it was doable.
Is that a limitation of SORT? The ld docs say
There are two ways to include more than one section:
*(.text .rdata)
*(.text) *(.rdata)
The difference between these is the order in which the '.text' and
'.rdata' input sections will appear in the output section.
so there shouldn't be a problem mentioning more than one section name?
>> If LD_DEAD_CODE_DATA_ELIMINATION was more widely supported (and I was
>> surprised to see that it's not even available on arm or x86) one could
>> also play another game, dropping the KEEP()s and instead create a linker
>> script snippet containing EXTERN(__ksymtab_foo __ksymtab_bar ...),
>> referencing the "struct kernel_symbol" elements themselves rather than
>> the singleton sections they reside in.
>
> Do you mean LD_DEAD_CODE_DATA_ELIMINATION must be enabled by default
> to do this?
>
No, but without LD_DEAD_CODE_DATA_ELIMINATION, I don't see much point of
the TRIM_UNUSED_KSYMS. Yes, the export_symbol metadata itself vanishes,
but the actual functions remain in the image. Conversely, with modules
enabled, LD_DEAD_CODE_DATA_ELIMINATION can't do much when almost all of
the kernel can be built modular so almost extern interface is an
EXPORT_SYMBOL. At least, that's what I see for a ppc target with a
somewhat trimmed-down .config, combining the two gives much more space
saving than the sum of what each option does:
$ size vmlinux.{vanilla,trim,dead,trim-dead}
text data bss dec hex filename
6197380 1159488 121732 7478600 721d48 vmlinux.vanilla
6045906 1159440 121732 7327078 6fcd66 vmlinux.trim
6087316 1137284 120476 7345076 7013b4 vmlinux.dead
5675370 1101964 115180 6892514 692be2 vmlinux.trim-dead
Anyway, that was just an aside, probably the above ___ksymtab+foo thing
will work just fine.
Rasmus
On Fri, Feb 26, 2021 at 1:19 AM Rasmus Villemoes
<[email protected]> wrote:
>
> On 25/02/2021 16.49, Masahiro Yamada wrote:
> > On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
> > <[email protected]> wrote:
> >>
> >> On 24/02/2021 15.40, Masahiro Yamada wrote:
> >
> >
> > Good insight.
> > Actually, I came up with the same idea last night, and had started
> > the implementation background.
> > I needed sleep before completing the patch set, but
> > now it is working as far as I tested.
> >
> > BTW,
> > KEEP(*(SORT(___ksymtab+foo ___ksymtab+bar ___ksymtab+baz))
> > is a syntax error.
> >
>
> ah, ok, didn't test anything, just threw it out there in case somebody
> wanted to see if it was doable.
>
> Is that a limitation of SORT? The ld docs say
>
> There are two ways to include more than one section:
> *(.text .rdata)
> *(.text) *(.rdata)
> The difference between these is the order in which the '.text' and
> '.rdata' input sections will appear in the output section.
>
> so there shouldn't be a problem mentioning more than one section name?
KEEP(*(foo bar))
and
SORT(*(foo bar))
are both syntax error.
I think having multiple entries in KEEP() / SORT()
is sensible, but unfortunately the linker rejects this form.
> >> If LD_DEAD_CODE_DATA_ELIMINATION was more widely supported (and I was
> >> surprised to see that it's not even available on arm or x86) one could
> >> also play another game, dropping the KEEP()s and instead create a linker
> >> script snippet containing EXTERN(__ksymtab_foo __ksymtab_bar ...),
> >> referencing the "struct kernel_symbol" elements themselves rather than
> >> the singleton sections they reside in.
> >
> > Do you mean LD_DEAD_CODE_DATA_ELIMINATION must be enabled by default
> > to do this?
> >
>
> No, but without LD_DEAD_CODE_DATA_ELIMINATION, I don't see much point of
> the TRIM_UNUSED_KSYMS. Yes, the export_symbol metadata itself vanishes,
> but the actual functions remain in the image. Conversely, with modules
> enabled, LD_DEAD_CODE_DATA_ELIMINATION can't do much when almost all of
> the kernel can be built modular so almost extern interface is an
> EXPORT_SYMBOL. At least, that's what I see for a ppc target with a
> somewhat trimmed-down .config, combining the two gives much more space
> saving than the sum of what each option does:
>
> $ size vmlinux.{vanilla,trim,dead,trim-dead}
> text data bss dec hex filename
> 6197380 1159488 121732 7478600 721d48 vmlinux.vanilla
> 6045906 1159440 121732 7327078 6fcd66 vmlinux.trim
> 6087316 1137284 120476 7345076 7013b4 vmlinux.dead
> 5675370 1101964 115180 6892514 692be2 vmlinux.trim-dead
>
> Anyway, that was just an aside, probably the above ___ksymtab+foo thing
> will work just fine.
>
> Rasmus
Make sense.
The combination of LD_DEAD_CODE_DATA_ELIMINATION and
TRIM_UNUSED_KSYMS is more powerful than
the stand-alone use of TRIM_UNUSED_KSYMS.
I just expected the combination of LTO and TRIM_UNUSED_KSYMS
would be even more powerful...
Unfortunately, Clang LTO which lands in this MW
cannot trim any code. So, it is useless for the
purpose of eliminating unreachable code.
--
Best Regards
Masahiro Yamada