Module object files can contain an undefined reference to __this_module,
which isn't resolved until we link the final .ko. The kernel doesn't
export this symbol, so ignore it in gen_autoksyms.sh. This avoids an
unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
symbol list that already contains all the module dependencies.
Signed-off-by: Sami Tolvanen <[email protected]>
---
scripts/gen_autoksyms.sh | 3 +++
1 file changed, 3 insertions(+)
diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
index faacf7062122..653fadbad302 100755
--- a/scripts/gen_autoksyms.sh
+++ b/scripts/gen_autoksyms.sh
@@ -56,4 +56,7 @@ EOT
# point addresses.
sed -e 's/^\.//' |
sort -u |
+# Ignore __this_module. It's not an exported symbol, and will be resolved
+# when the final .ko's are linked.
+grep -v '^__this_module$' |
sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
--
2.36.1.476.g0c4daa206d-goog
On 6/16/22 12:57, Sami Tolvanen wrote:
> Module object files can contain an undefined reference to __this_module,
> which isn't resolved until we link the final .ko. The kernel doesn't
> export this symbol, so ignore it in gen_autoksyms.sh. This avoids an
> unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> symbol list that already contains all the module dependencies.
>
> Signed-off-by: Sami Tolvanen <[email protected]>
> ---
> scripts/gen_autoksyms.sh | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> index faacf7062122..653fadbad302 100755
> --- a/scripts/gen_autoksyms.sh
> +++ b/scripts/gen_autoksyms.sh
> @@ -56,4 +56,7 @@ EOT
> # point addresses.
> sed -e 's/^\.//' |
> sort -u |
> +# Ignore __this_module. It's not an exported symbol, and will be resolved
> +# when the final .ko's are linked.
> +grep -v '^__this_module$' |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
Tested-by: Steve Muckle <[email protected]>
+ Sedat
Re: https://lore.kernel.org/linux-kbuild/CAKwvOdmb5xdF70TzNp=4STCpzkGh16FnuKE1KbdzDhHt=OuRFA@mail.gmail.com/
In case this helps.
+ Ramji
Ramji, it sounds like you helped test this downstream? If that's the
case, mind supplying your tested-by tag for the record? Thanks for
help verifying this change. Thanks too, Steve!
On Thu, Jun 16, 2022 at 12:58 PM Sami Tolvanen <[email protected]> wrote:
>
> Module object files can contain an undefined reference to __this_module,
> which isn't resolved until we link the final .ko. The kernel doesn't
> export this symbol, so ignore it in gen_autoksyms.sh. This avoids an
> unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> symbol list that already contains all the module dependencies.
Worth mentioning that this also fixes a significant build time
regression made more painful by CONFIG_LTO_CLANG_FULL when using
CONFIG_UNUSED_KSYMS_WHITELIST.
Thanks for the patch!
Reviewed-by: Nick Desaulniers <[email protected]>
>
> Signed-off-by: Sami Tolvanen <[email protected]>
> ---
> scripts/gen_autoksyms.sh | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> index faacf7062122..653fadbad302 100755
> --- a/scripts/gen_autoksyms.sh
> +++ b/scripts/gen_autoksyms.sh
> @@ -56,4 +56,7 @@ EOT
> # point addresses.
> sed -e 's/^\.//' |
> sort -u |
> +# Ignore __this_module. It's not an exported symbol, and will be resolved
> +# when the final .ko's are linked.
> +grep -v '^__this_module$' |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
> --
> 2.36.1.476.g0c4daa206d-goog
>
--
Thanks,
~Nick Desaulniers
On Fri, Jun 17, 2022 at 10:28 AM Nick Desaulniers
<[email protected]> wrote:
>
> + Sedat
> Re: https://lore.kernel.org/linux-kbuild/CAKwvOdmb5xdF70TzNp=4STCpzkGh16FnuKE1KbdzDhHt=OuRFA@mail.gmail.com/
> In case this helps.
>
> + Ramji
> Ramji, it sounds like you helped test this downstream? If that's the
> case, mind supplying your tested-by tag for the record? Thanks for
> help verifying this change. Thanks too, Steve!
>
> On Thu, Jun 16, 2022 at 12:58 PM Sami Tolvanen <[email protected]> wrote:
> >
> > Module object files can contain an undefined reference to __this_module,
> > which isn't resolved until we link the final .ko. The kernel doesn't
> > export this symbol, so ignore it in gen_autoksyms.sh. This avoids an
> > unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> > symbol list that already contains all the module dependencies.
>
> Worth mentioning that this also fixes a significant build time
> regression made more painful by CONFIG_LTO_CLANG_FULL when using
> CONFIG_UNUSED_KSYMS_WHITELIST.
>
Yes, I tested it and it resolves the build time regression with modules.
Tested-by: Ramji Jiyani <[email protected]>
Thanks,
Ramji
> Thanks for the patch!
> Reviewed-by: Nick Desaulniers <[email protected]>
>
> >
> > Signed-off-by: Sami Tolvanen <[email protected]>
> > ---
> > scripts/gen_autoksyms.sh | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> > index faacf7062122..653fadbad302 100755
> > --- a/scripts/gen_autoksyms.sh
> > +++ b/scripts/gen_autoksyms.sh
> > @@ -56,4 +56,7 @@ EOT
> > # point addresses.
> > sed -e 's/^\.//' |
> > sort -u |
> > +# Ignore __this_module. It's not an exported symbol, and will be resolved
> > +# when the final .ko's are linked.
> > +grep -v '^__this_module$' |
> > sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
> > --
> > 2.36.1.476.g0c4daa206d-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
On Fri, Jun 17, 2022 at 7:28 PM Nick Desaulniers
<[email protected]> wrote:
>
> + Sedat
> Re: https://lore.kernel.org/linux-kbuild/CAKwvOdmb5xdF70TzNp=4STCpzkGh16FnuKE1KbdzDhHt=OuRFA@mail.gmail.com/
> In case this helps.
>
Thanks Nick for CCing me and this patch.
Usually, I build with CONFIG_LTO_CLANG_THIN=y and have not seen any
measurable build-time speedup.
My kernel-config is attached.
Feel free to add my...
Tested-by: Sedat Dilek <[email protected]> # LLVM-14 (x86-64)
-Sedat-
> + Ramji
> Ramji, it sounds like you helped test this downstream? If that's the
> case, mind supplying your tested-by tag for the record? Thanks for
> help verifying this change. Thanks too, Steve!
>
> On Thu, Jun 16, 2022 at 12:58 PM Sami Tolvanen <[email protected]> wrote:
> >
> > Module object files can contain an undefined reference to __this_module,
> > which isn't resolved until we link the final .ko. The kernel doesn't
> > export this symbol, so ignore it in gen_autoksyms.sh. This avoids an
> > unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> > symbol list that already contains all the module dependencies.
>
> Worth mentioning that this also fixes a significant build time
> regression made more painful by CONFIG_LTO_CLANG_FULL when using
> CONFIG_UNUSED_KSYMS_WHITELIST.
>
> Thanks for the patch!
> Reviewed-by: Nick Desaulniers <[email protected]>
>
> >
> > Signed-off-by: Sami Tolvanen <[email protected]>
> > ---
> > scripts/gen_autoksyms.sh | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> > index faacf7062122..653fadbad302 100755
> > --- a/scripts/gen_autoksyms.sh
> > +++ b/scripts/gen_autoksyms.sh
> > @@ -56,4 +56,7 @@ EOT
> > # point addresses.
> > sed -e 's/^\.//' |
> > sort -u |
> > +# Ignore __this_module. It's not an exported symbol, and will be resolved
> > +# when the final .ko's are linked.
> > +grep -v '^__this_module$' |
> > sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
> > --
> > 2.36.1.476.g0c4daa206d-goog
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
On Fri, Jun 17, 2022 at 4:58 AM Sami Tolvanen <[email protected]> wrote:
>
> Module object files can contain an undefined reference to __this_module,
> which isn't resolved until we link the final .ko. The kernel doesn't
> export this symbol, so ignore it in gen_autoksyms.sh.
OK, I understand these two sentences.
> This avoids an
> unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> symbol list that already contains all the module dependencies.
>
I do not understand how this can happen.
Can you provide me steps to reproduce it?
> Signed-off-by: Sami Tolvanen <[email protected]>
> ---
> scripts/gen_autoksyms.sh | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/scripts/gen_autoksyms.sh b/scripts/gen_autoksyms.sh
> index faacf7062122..653fadbad302 100755
> --- a/scripts/gen_autoksyms.sh
> +++ b/scripts/gen_autoksyms.sh
> @@ -56,4 +56,7 @@ EOT
> # point addresses.
> sed -e 's/^\.//' |
> sort -u |
> +# Ignore __this_module. It's not an exported symbol, and will be resolved
> +# when the final .ko's are linked.
> +grep -v '^__this_module$' |
> sed -e 's/\(.*\)/#define __KSYM_\1 1/' >> "$output_file"
> --
> 2.36.1.476.g0c4daa206d-goog
>
--
Best Regards
Masahiro Yamada
On Sat, Jun 18, 2022 at 4:01 PM Masahiro Yamada <[email protected]> wrote:
>
> On Fri, Jun 17, 2022 at 4:58 AM Sami Tolvanen <[email protected]> wrote:
> >
> > Module object files can contain an undefined reference to __this_module,
> > which isn't resolved until we link the final .ko. The kernel doesn't
> > export this symbol, so ignore it in gen_autoksyms.sh.
>
> OK, I understand these two sentences.
>
> > This avoids an
> > unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> > symbol list that already contains all the module dependencies.
> >
>
> I do not understand how this can happen.
>
>
> Can you provide me steps to reproduce it?
This issue only happens when we have a whitelist that already contains
all the symbols we need to export. As autoksyms.h contains all the
necessary symbols in the initial vmlinux build, there should be no
need to link vmlinux again. However, as the code is looking at
undefined symbols in module .o files before __this_module is resolved,
adjust_autoksyms.sh thinks that __this_module is still missing and
triggers a rebuild, without actually changing anything.
I suspect this isn't a common situation, but it does happen when
building Android kernels, which specify a complete list of exported
symbols. Linking vmlinux.o takes a while with LTO and we would like to
avoid performing this step more than once.
Sami
On Wed, Jun 22, 2022 at 12:37 AM Sami Tolvanen <[email protected]> wrote:
>
> On Sat, Jun 18, 2022 at 4:01 PM Masahiro Yamada <[email protected]> wrote:
> >
> > On Fri, Jun 17, 2022 at 4:58 AM Sami Tolvanen <[email protected]> wrote:
> > >
> > > Module object files can contain an undefined reference to __this_module,
> > > which isn't resolved until we link the final .ko. The kernel doesn't
> > > export this symbol, so ignore it in gen_autoksyms.sh.
> >
> > OK, I understand these two sentences.
> >
> > > This avoids an
> > > unnecessary vmlinux rebuild with UNUSED_KSYMS_WHITELIST when we have a
> > > symbol list that already contains all the module dependencies.
> > >
> >
> > I do not understand how this can happen.
> >
> >
> > Can you provide me steps to reproduce it?
>
> This issue only happens when we have a whitelist that already contains
> all the symbols we need to export. As autoksyms.h contains all the
> necessary symbols in the initial vmlinux build, there should be no
> need to link vmlinux again. However, as the code is looking at
> undefined symbols in module .o files before __this_module is resolved,
> adjust_autoksyms.sh thinks that __this_module is still missing and
> triggers a rebuild, without actually changing anything.
>
> I suspect this isn't a common situation, but it does happen when
> building Android kernels, which specify a complete list of exported
> symbols. Linking vmlinux.o takes a while with LTO and we would like to
> avoid performing this step more than once.
>
> Sami
Thanks, confirmed.
But, this patch does not shoot the root cause.
The relink of vmlinux should not happen irrespective of
UNUSED_KSYMS_WHITELIST.
I thought I had fixed this issue long time before, but
actually didn't due to my mistake in
commit 3fdc7d3fe4c0 ("kbuild: link vmlinux only once for
CONFIG_TRIM_UNUSED_KSYMS").
I will pick up this patch anyway, but the re-link of vmlinux
should be addressed by the other patch.
--
Best Regards
Masahiro Yamada