Message-ID: <CAK7LNASv-pWVdztiD1-VCHCnOqZ6j=J6ra5cEdiUkmM9E8fHWQ@mail.gmail.com>
Date:   Fri, 26 Feb 2021 00:49:27 +0900
From:   Masahiro Yamada <masahiroy@...nel.org>
To:     Rasmus Villemoes <rasmus.villemoes@...vas.dk>
Cc:     Jessica Yu <jeyu@...nel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Christoph Hellwig <hch@....de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Miroslav Benes <mbenes@...e.cz>,
        Emil Velikov <emil.l.velikov@...il.com>
Subject: Re: [GIT PULL] Modules updates for v5.12

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

Powered by blists - more mailing lists