On Wed, Feb 24, 2021 at 11:13 PM Jessica Yu <j...@kernel.org> 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
> >> <torva...@linux-foundation.org> 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 <j...@kernel.org>
> 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 <j...@kernel.org>
> ---
> 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
