All of lore.kernel.org
 help / color / mirror / Atom feed
From: Masahiro Yamada <masahiroy@kernel.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-modules <linux-modules@vger.kernel.org>,
	linux-s390 <linux-s390@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	clang-built-linux <llvm@lists.linux.dev>
Subject: Re: [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS
Date: Tue, 10 May 2022 22:03:31 +0900	[thread overview]
Message-ID: <CAK7LNASb40kWgvOM42WyS82p42HQ7Mqhwo3GDffg4yHcKgJk0w@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOdm_oSLHddWWSzF5UuYrLCsAwF8AwbUGotUnPvV+6JkSkg@mail.gmail.com>

On Tue, May 10, 2022 at 2:51 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
>  On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> > index 07a36a874dca..51ce72ce80fa 100644
> > --- a/include/asm-generic/export.h
> > +++ b/include/asm-generic/export.h
> > @@ -2,6 +2,14 @@
> >  #ifndef __ASM_GENERIC_EXPORT_H
> >  #define __ASM_GENERIC_EXPORT_H
> >
> > +/*
> > + * This comment block is used by fixdep. Please do not remove.
>
> I don't know much about fixdep. How does that work, if you could summarize?


You can find detailed explanation in scripts/basic/fixdep.c


In short, it works like this:

fixdep parses every source (including headers).
If it finds "CONFIG_MODVERSIONS", it adds a dependency
on $(wildcard include/config/MODVERSIONS)
to the .cmd files.

If CONFIG_MODVERSIONS is toggled in Kconfig,
it touches include/config/MODVERSIONS.    [1]

In the next run of Make, all the sources depending on
CONFIG_MODVERSIONS will be re-compiled because the
timestamp of include/config/MODVERSIONS is up-to-date.


[1]: https://github.com/torvalds/linux/blob/v5.17/scripts/kconfig/confdata.c#L141





> > + *
> > + * When CONFIG_MODVERSIONS is changed from n to y, all source files having
> > + * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a
> > + * side effect of the .o build rule.
> > + */
> > +
> >  #ifndef KSYM_FUNC
> >  #define KSYM_FUNC(x) x
> >  #endif
> > @@ -12,9 +20,6 @@
> >  #else
> >  #define KSYM_ALIGN 4
> >  #endif
> > -#ifndef KCRC_ALIGN
> > -#define KCRC_ALIGN 4
> > -#endif
>
> The #ifndef is there because arch/m68k/include/asm/export.h:1 defines
> KCRC_ALIGN. You should delete that, too.

Nice catch! I will clean it up too.




>
> > diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
> > index 4827c5abe5b7..6e6933ae7911 100644
> > --- a/scripts/genksyms/genksyms.c
> > +++ b/scripts/genksyms/genksyms.c
> > @@ -33,7 +33,7 @@ char *cur_filename;
> >  int in_source_file;
> >
> >  static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types,
> > -          flag_preserve, flag_warnings, flag_rel_crcs;
> > +          flag_preserve, flag_warnings;
> >
> >  static int errors;
> >  static int nsyms;
> > @@ -681,10 +681,7 @@ void export_symbol(const char *name)
> >                         fputs(">\n", debugfile);
> >
> >                 /* Used as a linker script. */
>
> ^ Does this comment still apply?

No.  From this commit going forward,
the genksyms output will not be used as a linker script.

08/14 will delete this comment anyway, but
it is possible to remove it in this commit.








>
> > -               printf(!flag_rel_crcs ? "__crc_%s = 0x%08lx;\n" :
> > -                      "SECTIONS { .rodata : ALIGN(4) { "
> > -                      "__crc_%s = .; LONG(0x%08lx); } }\n",
> > -                      name, crc);
> > +               printf("__crc_%s = 0x%08lx;\n", name, crc);
> >         }
> >  }
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index eceb3ee7ec06..6aee2401f3ad 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -88,11 +88,6 @@ modpost_link()
> >                 gen_initcalls
> >                 lds="-T .tmp_initcalls.lds"
> >
> > -               if is_enabled CONFIG_MODVERSIONS; then
> > -                       gen_symversions
>
> ^ this is the only caller of gen_symversions, right? Then
> gen_symversions can be cleaned up, too?


We can keep it in this commit.

The follow-up cleaning is done in 07/14.

To avoid too big commit,
I separated the build flow change and
trivial cleanups.



>
> > -                       lds="${lds} -T .tmp_symversions.lds"
> > -               fi
> > -
> >                 # This might take a while, so indicate that we're doing
> >                 # an LTO link
> >                 info LTO ${1}
> > @@ -183,6 +178,10 @@ vmlinux_link()
> >                 libs="${KBUILD_VMLINUX_LIBS}"
> >         fi
> >
> > +       if is_enabled CONFIG_MODULES; then
> > +               objs="${objs} .vmlinux.export.o"
> > +       fi
> > +
> >         if [ "${SRCARCH}" = "um" ]; then
> >                 wl=-Wl,
> >                 ld="${CC}"
> > @@ -312,6 +311,7 @@ cleanup()
> >         rm -f vmlinux.o
> >         rm -f .vmlinux.d
> >         rm -f .vmlinux.objs
> > +       rm -f .vmlinux.export.c
>
> Probably can drop the `rm -f .tmp_symversions.lds` here, too?

It will be cleaned up by 07/14.







> --
> Thanks,
> ~Nick Desaulniers



--
Best Regards
Masahiro Yamada

WARNING: multiple messages have this Message-ID
From: Masahiro Yamada <masahiroy@kernel.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: linux-s390 <linux-s390@vger.kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	clang-built-linux <llvm@lists.linux.dev>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	Ard Biesheuvel <ardb@kernel.org>,
	linux-modules <linux-modules@vger.kernel.org>
Subject: Re: [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS
Date: Tue, 10 May 2022 22:03:31 +0900	[thread overview]
Message-ID: <CAK7LNASb40kWgvOM42WyS82p42HQ7Mqhwo3GDffg4yHcKgJk0w@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOdm_oSLHddWWSzF5UuYrLCsAwF8AwbUGotUnPvV+6JkSkg@mail.gmail.com>

On Tue, May 10, 2022 at 2:51 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
>  On Sun, May 8, 2022 at 12:10 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> > index 07a36a874dca..51ce72ce80fa 100644
> > --- a/include/asm-generic/export.h
> > +++ b/include/asm-generic/export.h
> > @@ -2,6 +2,14 @@
> >  #ifndef __ASM_GENERIC_EXPORT_H
> >  #define __ASM_GENERIC_EXPORT_H
> >
> > +/*
> > + * This comment block is used by fixdep. Please do not remove.
>
> I don't know much about fixdep. How does that work, if you could summarize?


You can find detailed explanation in scripts/basic/fixdep.c


In short, it works like this:

fixdep parses every source (including headers).
If it finds "CONFIG_MODVERSIONS", it adds a dependency
on $(wildcard include/config/MODVERSIONS)
to the .cmd files.

If CONFIG_MODVERSIONS is toggled in Kconfig,
it touches include/config/MODVERSIONS.    [1]

In the next run of Make, all the sources depending on
CONFIG_MODVERSIONS will be re-compiled because the
timestamp of include/config/MODVERSIONS is up-to-date.


[1]: https://github.com/torvalds/linux/blob/v5.17/scripts/kconfig/confdata.c#L141





> > + *
> > + * When CONFIG_MODVERSIONS is changed from n to y, all source files having
> > + * EXPORT_SYMBOL variants must be re-compiled because genksyms is run as a
> > + * side effect of the .o build rule.
> > + */
> > +
> >  #ifndef KSYM_FUNC
> >  #define KSYM_FUNC(x) x
> >  #endif
> > @@ -12,9 +20,6 @@
> >  #else
> >  #define KSYM_ALIGN 4
> >  #endif
> > -#ifndef KCRC_ALIGN
> > -#define KCRC_ALIGN 4
> > -#endif
>
> The #ifndef is there because arch/m68k/include/asm/export.h:1 defines
> KCRC_ALIGN. You should delete that, too.

Nice catch! I will clean it up too.




>
> > diff --git a/scripts/genksyms/genksyms.c b/scripts/genksyms/genksyms.c
> > index 4827c5abe5b7..6e6933ae7911 100644
> > --- a/scripts/genksyms/genksyms.c
> > +++ b/scripts/genksyms/genksyms.c
> > @@ -33,7 +33,7 @@ char *cur_filename;
> >  int in_source_file;
> >
> >  static int flag_debug, flag_dump_defs, flag_reference, flag_dump_types,
> > -          flag_preserve, flag_warnings, flag_rel_crcs;
> > +          flag_preserve, flag_warnings;
> >
> >  static int errors;
> >  static int nsyms;
> > @@ -681,10 +681,7 @@ void export_symbol(const char *name)
> >                         fputs(">\n", debugfile);
> >
> >                 /* Used as a linker script. */
>
> ^ Does this comment still apply?

No.  From this commit going forward,
the genksyms output will not be used as a linker script.

08/14 will delete this comment anyway, but
it is possible to remove it in this commit.








>
> > -               printf(!flag_rel_crcs ? "__crc_%s = 0x%08lx;\n" :
> > -                      "SECTIONS { .rodata : ALIGN(4) { "
> > -                      "__crc_%s = .; LONG(0x%08lx); } }\n",
> > -                      name, crc);
> > +               printf("__crc_%s = 0x%08lx;\n", name, crc);
> >         }
> >  }
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index eceb3ee7ec06..6aee2401f3ad 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -88,11 +88,6 @@ modpost_link()
> >                 gen_initcalls
> >                 lds="-T .tmp_initcalls.lds"
> >
> > -               if is_enabled CONFIG_MODVERSIONS; then
> > -                       gen_symversions
>
> ^ this is the only caller of gen_symversions, right? Then
> gen_symversions can be cleaned up, too?


We can keep it in this commit.

The follow-up cleaning is done in 07/14.

To avoid too big commit,
I separated the build flow change and
trivial cleanups.



>
> > -                       lds="${lds} -T .tmp_symversions.lds"
> > -               fi
> > -
> >                 # This might take a while, so indicate that we're doing
> >                 # an LTO link
> >                 info LTO ${1}
> > @@ -183,6 +178,10 @@ vmlinux_link()
> >                 libs="${KBUILD_VMLINUX_LIBS}"
> >         fi
> >
> > +       if is_enabled CONFIG_MODULES; then
> > +               objs="${objs} .vmlinux.export.o"
> > +       fi
> > +
> >         if [ "${SRCARCH}" = "um" ]; then
> >                 wl=-Wl,
> >                 ld="${CC}"
> > @@ -312,6 +311,7 @@ cleanup()
> >         rm -f vmlinux.o
> >         rm -f .vmlinux.d
> >         rm -f .vmlinux.objs
> > +       rm -f .vmlinux.export.c
>
> Probably can drop the `rm -f .tmp_symversions.lds` here, too?

It will be cleaned up by 07/14.







> --
> Thanks,
> ~Nick Desaulniers



--
Best Regards
Masahiro Yamada

  reply	other threads:[~2022-05-10 13:19 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-08 19:06 [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada
2022-05-08 19:06 ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 01/14] modpost: remove left-over cross_compile declaration Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09 17:08   ` Nick Desaulniers
2022-05-09 17:08     ` Nick Desaulniers
2022-05-12  4:35   ` Masahiro Yamada
2022-05-12  4:35     ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 02/14] modpost: change the license of EXPORT_SYMBOL to bool type Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-12  4:36   ` Masahiro Yamada
2022-05-12  4:36     ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 03/14] modpost: split the section mismatch checks into section-check.c Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09 17:19   ` Nick Desaulniers
2022-05-09 17:19     ` Nick Desaulniers
2022-05-10  6:55     ` Masahiro Yamada
2022-05-10  6:55       ` Masahiro Yamada
2022-05-11 18:47       ` Nick Desaulniers
2022-05-11 18:47         ` Nick Desaulniers
2022-05-11 19:27         ` Masahiro Yamada
2022-05-11 19:27           ` Masahiro Yamada
2022-05-11 19:51           ` Jeff Johnson
2022-05-11 19:51             ` Jeff Johnson
2022-05-11 20:01             ` Nick Desaulniers
2022-05-11 20:01               ` Nick Desaulniers
2022-05-08 19:06 ` [PATCH v4 04/14] modpost: add sym_find_with_module() helper Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 05/14] modpost: extract symbol versions from *.cmd files Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09 21:52   ` Sami Tolvanen
2022-05-09 21:52     ` Sami Tolvanen
2022-05-08 19:06 ` [PATCH v4 06/14] kbuild: link symbol CRCs at final link, removing CONFIG_MODULE_REL_CRCS Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09 17:50   ` Nick Desaulniers
2022-05-09 17:50     ` Nick Desaulniers
2022-05-10 13:03     ` Masahiro Yamada [this message]
2022-05-10 13:03       ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 07/14] kbuild: stop merging *.symversions Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 08/14] genksyms: adjust the output format to modpost Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 09/14] kbuild: do not create *.prelink.o for Clang LTO or IBT Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09 23:13   ` Sami Tolvanen
2022-05-09 23:13     ` Sami Tolvanen
2022-05-08 19:06 ` [PATCH v4 10/14] kbuild: check static EXPORT_SYMBOL* by script instead of modpost Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09 18:05   ` Nick Desaulniers
2022-05-09 18:05     ` Nick Desaulniers
2022-05-10 13:53     ` Masahiro Yamada
2022-05-10 13:53       ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 11/14] kbuild: make built-in.a rule robust against too long argument error Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 12/14] kbuild: make *.mod " Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 13/14] kbuild: add cmd_and_savecmd macro Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-08 19:06 ` [PATCH v4 14/14] kbuild: rebuild multi-object modules when objtool is updated Masahiro Yamada
2022-05-08 19:06   ` Masahiro Yamada
2022-05-09  4:24 ` [PATCH v4 00/14] kbuild: yet another series of cleanups (modpost, LTO, MODULE_REL_CRCS, export.h) Masahiro Yamada
2022-05-09  4:24   ` Masahiro Yamada
2022-05-09 22:13   ` Nathan Chancellor
2022-05-09 22:13     ` Nathan Chancellor
2022-05-10  6:55     ` Masahiro Yamada
2022-05-10  6:55       ` Masahiro Yamada
2022-05-09 22:19   ` Nathan Chancellor
2022-05-09 22:19     ` Nathan Chancellor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAK7LNASb40kWgvOM42WyS82p42HQ7Mqhwo3GDffg4yHcKgJk0w@mail.gmail.com \
    --to=masahiroy@kernel.org \
    --cc=ardb@kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=llvm@lists.linux.dev \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.