[GIT PULL] Modules updates for v5.12

[GIT PULL] Modules updates for v5.12

[Jessica Yu]

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 <jeyu@kernel.org>

----------------------------------------------------------------
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(-)

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Tue, Feb 23, 2021 at 7:42 AM Jessica Yu <jeyu@kernel.org> 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

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Tue, Feb 23, 2021 at 10:26 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Tue, Feb 23, 2021 at 7:42 AM Jessica Yu <jeyu@kernel.org> wrote:
> >
> > Please pull below to receive modules updates for the v5.12 merge window.
>
> Pulled.

Actually, I take that back.

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 can't see anything in the module pull that could possibly cause
this, but it seems to have coincided with that pull.

I'm investigating, but the output stopped for a long time at

  CHK     include/generated/autoksyms.h

Ring a bell? Did something change in here?

                Linus

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Tue, Feb 23, 2021 at 10:42 AM Linus Torvalds
<torvalds@linux-foundation.org> 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.

              Linus

Re: [GIT PULL] Modules updates for v5.12

[Christoph Hellwig]

On Tue, Feb 23, 2021 at 11:55:50AM -0800, Linus Torvalds wrote:
> On Tue, Feb 23, 2021 at 10:42 AM Linus Torvalds
> <torvalds@linux-foundation.org> 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---

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Tue, Feb 23, 2021 at 11:55 AM Linus Torvalds
<torvalds@linux-foundation.org> 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

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <hch@lst.de> 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

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
<torvalds@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, 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

Re: [GIT PULL] Modules updates for v5.12

[pr-tracker-bot]

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

Re: [GIT PULL] Modules updates for v5.12

[Christoph Hellwig]

On Tue, Feb 23, 2021 at 12:07:39PM -0800, Linus Torvalds wrote:
> On Tue, Feb 23, 2021 at 12:03 PM Linus Torvalds
> <torvalds@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?

Re: [GIT PULL] Modules updates for v5.12

[Jessica Yu]

+++ Linus Torvalds [23/02/21 12:03 -0800]:
>On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <hch@lst.de> 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.

Re: [GIT PULL] Modules updates for v5.12

[Jessica Yu]

+++ 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
>> <torvalds@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)

---
 From 08bc08229fc3801b1a580a07ce7ff3e806b3fe90 Mon Sep 17 00:00:00 2001
From: Jessica Yu <jeyu@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 <jeyu@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
  

Re: [GIT PULL] Modules updates for v5.12

[Masahiro Yamada]

On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <jeyu@kernel.org> wrote:
>
> +++ Linus Torvalds [23/02/21 12:03 -0800]:
> >On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <hch@lst.de> 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

Re: [GIT PULL] Modules updates for v5.12

[Masahiro Yamada]

On Wed, Feb 24, 2021 at 11:13 PM Jessica Yu <jeyu@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
> >> <torvalds@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 <jeyu@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 <jeyu@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

Re: [GIT PULL] Modules updates for v5.12

[Masahiro Yamada]

[-- Attachment #1: Type: text/plain, Size: 1303 bytes --]

On Wed, Feb 24, 2021 at 11:46 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Wed, Feb 24, 2021 at 11:13 PM Jessica Yu <jeyu@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
> > >> <torvalds@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)
>

A patch attached, if Linus is OK to re-enable this.



-- 
Best Regards
Masahiro Yamada

[-- Attachment #2: 0001-Kbuild-re-enable-TRIM_UNUSED_KSYMS-under-the-conditi.patch --]
[-- Type: text/x-patch, Size: 1247 bytes --]

From 2eb88648aff9547d1c10df841d1ac61ac2369df6 Mon Sep 17 00:00:00 2001
From: Masahiro Yamada <masahiroy@kernel.org>
Date: Wed, 24 Feb 2021 23:52:54 +0900
Subject: [PATCH] Kbuild: re-enable TRIM_UNUSED_KSYMS under the condition of
 !COMPILE_TEST

This option is useful for embedded systems, and acutally used by the
MIPS generic_defconfig. Commit 1518c633df78 ("kbuild: allow symbol
whitelisting with TRIM_UNUSED_KSYMS") also states this is used by
Android Generic Kernel Image. I guess somebody will start complaining.

Disable this only for COMPILE_TEST so it is hidden for all{yes,mod}config.

Fixes: 5cf0fd591f2e ("Kbuild: disable TRIM_UNUSED_KSYMS option")
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---
 init/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 2ff0b5a50736..135e2129e04b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -2273,7 +2273,7 @@ config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
 
 config TRIM_UNUSED_KSYMS
 	bool "Trim unused exported kernel symbols"
-	depends on BROKEN
+	depends on !COMPILE_TEST
 	help
 	  The kernel and some modules make many symbols available for
 	  other modules to use via EXPORT_SYMBOL() and variants. Depending
-- 
2.27.0

Re: [GIT PULL] Modules updates for v5.12

[Linus Torvalds]

On Wed, Feb 24, 2021 at 6:47 AM Masahiro Yamada <masahiroy@kernel.org> 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

Re: [GIT PULL] Modules updates for v5.12

[Rasmus Villemoes]

On 24/02/2021 15.40, Masahiro Yamada wrote:
> On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <jeyu@kernel.org> wrote:
>>
>> +++ Linus Torvalds [23/02/21 12:03 -0800]:
>>> On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <hch@lst.de> 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

Re: [GIT PULL] Modules updates for v5.12

[Masahiro Yamada]

On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 24/02/2021 15.40, Masahiro Yamada wrote:
> > On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <jeyu@kernel.org> wrote:
> >>
> >> +++ Linus Torvalds [23/02/21 12:03 -0800]:
> >>> On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <hch@lst.de> 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

Re: [GIT PULL] Modules updates for v5.12

[Masahiro Yamada]

On Fri, Feb 26, 2021 at 12:49 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
> >
> > On 24/02/2021 15.40, Masahiro Yamada wrote:
> > > On Wed, Feb 24, 2021 at 5:33 PM Jessica Yu <jeyu@kernel.org> wrote:
> > >>
> > >> +++ Linus Torvalds [23/02/21 12:03 -0800]:
> > >>> On Tue, Feb 23, 2021 at 12:01 PM Christoph Hellwig <hch@lst.de> 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

Re: [GIT PULL] Modules updates for v5.12

[Rasmus Villemoes]

On 25/02/2021 16.49, Masahiro Yamada wrote:
> On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> 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

Re: [GIT PULL] Modules updates for v5.12

[Masahiro Yamada]

On Fri, Feb 26, 2021 at 1:19 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 25/02/2021 16.49, Masahiro Yamada wrote:
> > On Thu, Feb 25, 2021 at 4:36 AM Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> 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