diff mbox series

[kbuild,v4] kbuild: add an elfnote for whether vmlinux is built with lto

Message ID 20210401232723.3571287-1-yhs@fb.com (mailing list archive)
State New, archived
Headers show
Series [kbuild,v4] kbuild: add an elfnote for whether vmlinux is built with lto | expand

Commit Message

Yonghong Song April 1, 2021, 11:27 p.m. UTC
Currently, clang LTO built vmlinux won't work with pahole.
LTO introduced cross-cu dwarf tag references and broke
current pahole model which handles one cu as a time.
The solution is to merge all cu's as one pahole cu as in [1].
We would like to do this merging only if cross-cu dwarf
references happens. The LTO build mode is a pretty good
indication for that.

In earlier version of this patch ([2]), clang flag
-grecord-gcc-switches is proposed to add to compilation flags
so pahole could detect "-flto" and then merging cu's.
This will increate the binary size of 1% without LTO though.

Arnaldo suggested to use a note to indicate the vmlinux
is built with LTO. Such a cheap way to get whether the vmlinux
is built with LTO or not helps pahole but is also useful
for tracing as LTO may inline/delete/demote global functions,
promote static functions, etc.

So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
The owner of the note is "Linux".

With gcc 8.4.1 and clang trunk, without LTO, I got
  $ readelf -n vmlinux
  Displaying notes found in: .notes
    Owner                Data size        Description
  ...
    Linux                0x00000004       func
     description data: 00 00 00 00
  ...
With "readelf -x ".notes" vmlinux", I can verify the above "func"
with type code 0x101.

With clang thin-LTO, I got the same as above except the following:
     description data: 01 00 00 00
which indicates the vmlinux is built with LTO.

  [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
  [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/

Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/elfnote-lto.h | 14 ++++++++++++++
 init/version.c              |  2 ++
 scripts/mod/modpost.c       |  2 ++
 3 files changed, 18 insertions(+)
 create mode 100644 include/linux/elfnote-lto.h

Changelogs:
  v3 -> v4:
    . put new lto note in its own header file similar to
      build-salt.h. (Nick)
  v2 -> v3:
    . abandoned the approach of adding -grecord-gcc-switches,
      instead create a note to indicate whether it is a lto build
      or not. The note definition is in compiler.h. (Arnaldo)
  v1 -> v2:
    . limited to add -grecord-gcc-switches for LTO_CLANG
      instead of all clang build

Comments

Nick Desaulniers April 2, 2021, 6:07 p.m. UTC | #1
On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
>
> Currently, clang LTO built vmlinux won't work with pahole.
> LTO introduced cross-cu dwarf tag references and broke
> current pahole model which handles one cu as a time.
> The solution is to merge all cu's as one pahole cu as in [1].
> We would like to do this merging only if cross-cu dwarf
> references happens. The LTO build mode is a pretty good
> indication for that.
>
> In earlier version of this patch ([2]), clang flag
> -grecord-gcc-switches is proposed to add to compilation flags
> so pahole could detect "-flto" and then merging cu's.
> This will increate the binary size of 1% without LTO though.
>
> Arnaldo suggested to use a note to indicate the vmlinux
> is built with LTO. Such a cheap way to get whether the vmlinux
> is built with LTO or not helps pahole but is also useful
> for tracing as LTO may inline/delete/demote global functions,
> promote static functions, etc.
>
> So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> The owner of the note is "Linux".
>
> With gcc 8.4.1 and clang trunk, without LTO, I got
>   $ readelf -n vmlinux
>   Displaying notes found in: .notes
>     Owner                Data size        Description
>   ...
>     Linux                0x00000004       func
>      description data: 00 00 00 00
>   ...
> With "readelf -x ".notes" vmlinux", I can verify the above "func"
> with type code 0x101.
>
> With clang thin-LTO, I got the same as above except the following:
>      description data: 01 00 00 00
> which indicates the vmlinux is built with LTO.
>
>   [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
>   [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
>
> Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>

LGTM thanks Yonghong!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  include/linux/elfnote-lto.h | 14 ++++++++++++++
>  init/version.c              |  2 ++
>  scripts/mod/modpost.c       |  2 ++
>  3 files changed, 18 insertions(+)
>  create mode 100644 include/linux/elfnote-lto.h
>
> Changelogs:
>   v3 -> v4:
>     . put new lto note in its own header file similar to
>       build-salt.h. (Nick)
>   v2 -> v3:
>     . abandoned the approach of adding -grecord-gcc-switches,
>       instead create a note to indicate whether it is a lto build
>       or not. The note definition is in compiler.h. (Arnaldo)
>   v1 -> v2:
>     . limited to add -grecord-gcc-switches for LTO_CLANG
>       instead of all clang build
>
> diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
> new file mode 100644
> index 000000000000..d4635a3ecc4f
> --- /dev/null
> +++ b/include/linux/elfnote-lto.h
> @@ -0,0 +1,14 @@
> +#ifndef __ELFNOTE_LTO_H
> +#define __ELFNOTE_LTO_H
> +
> +#include <linux/elfnote.h>
> +
> +#define LINUX_ELFNOTE_LTO_INFO 0x101
> +
> +#ifdef CONFIG_LTO
> +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
> +#else
> +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
> +#endif
> +
> +#endif /* __ELFNOTE_LTO_H */
> diff --git a/init/version.c b/init/version.c
> index 92afc782b043..1a356f5493e8 100644
> --- a/init/version.c
> +++ b/init/version.c
> @@ -9,6 +9,7 @@
>
>  #include <generated/compile.h>
>  #include <linux/build-salt.h>
> +#include <linux/elfnote-lto.h>
>  #include <linux/export.h>
>  #include <linux/uts.h>
>  #include <linux/utsname.h>
> @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
>         " (" LINUX_COMPILER ") %s\n";
>
>  BUILD_SALT;
> +BUILD_LTO_INFO;
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 24725e50c7b4..98fb2bb024db 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
>          */
>         buf_printf(b, "#define INCLUDE_VERMAGIC\n");
>         buf_printf(b, "#include <linux/build-salt.h>\n");
> +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
>         buf_printf(b, "#include <linux/vermagic.h>\n");
>         buf_printf(b, "#include <linux/compiler.h>\n");
>         buf_printf(b, "\n");
>         buf_printf(b, "BUILD_SALT;\n");
> +       buf_printf(b, "BUILD_LTO_INFO;\n");
>         buf_printf(b, "\n");
>         buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
>         buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> --
> 2.30.2
>
Sedat Dilek April 2, 2021, 6:31 p.m. UTC | #2
On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> >
> > Currently, clang LTO built vmlinux won't work with pahole.
> > LTO introduced cross-cu dwarf tag references and broke
> > current pahole model which handles one cu as a time.
> > The solution is to merge all cu's as one pahole cu as in [1].
> > We would like to do this merging only if cross-cu dwarf
> > references happens. The LTO build mode is a pretty good
> > indication for that.
> >
> > In earlier version of this patch ([2]), clang flag
> > -grecord-gcc-switches is proposed to add to compilation flags
> > so pahole could detect "-flto" and then merging cu's.
> > This will increate the binary size of 1% without LTO though.
> >
> > Arnaldo suggested to use a note to indicate the vmlinux
> > is built with LTO. Such a cheap way to get whether the vmlinux
> > is built with LTO or not helps pahole but is also useful
> > for tracing as LTO may inline/delete/demote global functions,
> > promote static functions, etc.
> >
> > So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> > The owner of the note is "Linux".
> >
> > With gcc 8.4.1 and clang trunk, without LTO, I got
> >   $ readelf -n vmlinux
> >   Displaying notes found in: .notes
> >     Owner                Data size        Description
> >   ...
> >     Linux                0x00000004       func
> >      description data: 00 00 00 00
> >   ...
> > With "readelf -x ".notes" vmlinux", I can verify the above "func"
> > with type code 0x101.
> >
> > With clang thin-LTO, I got the same as above except the following:
> >      description data: 01 00 00 00
> > which indicates the vmlinux is built with LTO.
> >
> >   [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> >   [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> >
> > Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> > Signed-off-by: Yonghong Song <yhs@fb.com>
>
> LGTM thanks Yonghong!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> > ---
> >  include/linux/elfnote-lto.h | 14 ++++++++++++++
> >  init/version.c              |  2 ++
> >  scripts/mod/modpost.c       |  2 ++
> >  3 files changed, 18 insertions(+)
> >  create mode 100644 include/linux/elfnote-lto.h
> >
> > Changelogs:
> >   v3 -> v4:
> >     . put new lto note in its own header file similar to
> >       build-salt.h. (Nick)

That is a bit smarter (and smaller) than v3.
Queued up and building a new clang-lto kernel...
Will report later.

- Sedat -

> >   v2 -> v3:
> >     . abandoned the approach of adding -grecord-gcc-switches,
> >       instead create a note to indicate whether it is a lto build
> >       or not. The note definition is in compiler.h. (Arnaldo)
> >   v1 -> v2:
> >     . limited to add -grecord-gcc-switches for LTO_CLANG
> >       instead of all clang build
> >
> > diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
> > new file mode 100644
> > index 000000000000..d4635a3ecc4f
> > --- /dev/null
> > +++ b/include/linux/elfnote-lto.h
> > @@ -0,0 +1,14 @@
> > +#ifndef __ELFNOTE_LTO_H
> > +#define __ELFNOTE_LTO_H
> > +
> > +#include <linux/elfnote.h>
> > +
> > +#define LINUX_ELFNOTE_LTO_INFO 0x101
> > +
> > +#ifdef CONFIG_LTO
> > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
> > +#else
> > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
> > +#endif
> > +
> > +#endif /* __ELFNOTE_LTO_H */
> > diff --git a/init/version.c b/init/version.c
> > index 92afc782b043..1a356f5493e8 100644
> > --- a/init/version.c
> > +++ b/init/version.c
> > @@ -9,6 +9,7 @@
> >
> >  #include <generated/compile.h>
> >  #include <linux/build-salt.h>
> > +#include <linux/elfnote-lto.h>
> >  #include <linux/export.h>
> >  #include <linux/uts.h>
> >  #include <linux/utsname.h>
> > @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
> >         " (" LINUX_COMPILER ") %s\n";
> >
> >  BUILD_SALT;
> > +BUILD_LTO_INFO;
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 24725e50c7b4..98fb2bb024db 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
> >          */
> >         buf_printf(b, "#define INCLUDE_VERMAGIC\n");
> >         buf_printf(b, "#include <linux/build-salt.h>\n");
> > +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
> >         buf_printf(b, "#include <linux/vermagic.h>\n");
> >         buf_printf(b, "#include <linux/compiler.h>\n");
> >         buf_printf(b, "\n");
> >         buf_printf(b, "BUILD_SALT;\n");
> > +       buf_printf(b, "BUILD_LTO_INFO;\n");
> >         buf_printf(b, "\n");
> >         buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
> >         buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> > --
> > 2.30.2
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN%3DUBNdQiO3g%40mail.gmail.com.
Sedat Dilek April 2, 2021, 7:38 p.m. UTC | #3
On Fri, Apr 2, 2021 at 8:31 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> > >
> > > Currently, clang LTO built vmlinux won't work with pahole.
> > > LTO introduced cross-cu dwarf tag references and broke
> > > current pahole model which handles one cu as a time.
> > > The solution is to merge all cu's as one pahole cu as in [1].
> > > We would like to do this merging only if cross-cu dwarf
> > > references happens. The LTO build mode is a pretty good
> > > indication for that.
> > >
> > > In earlier version of this patch ([2]), clang flag
> > > -grecord-gcc-switches is proposed to add to compilation flags
> > > so pahole could detect "-flto" and then merging cu's.
> > > This will increate the binary size of 1% without LTO though.
> > >
> > > Arnaldo suggested to use a note to indicate the vmlinux
> > > is built with LTO. Such a cheap way to get whether the vmlinux
> > > is built with LTO or not helps pahole but is also useful
> > > for tracing as LTO may inline/delete/demote global functions,
> > > promote static functions, etc.
> > >
> > > So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> > > The owner of the note is "Linux".
> > >
> > > With gcc 8.4.1 and clang trunk, without LTO, I got
> > >   $ readelf -n vmlinux
> > >   Displaying notes found in: .notes
> > >     Owner                Data size        Description
> > >   ...
> > >     Linux                0x00000004       func
> > >      description data: 00 00 00 00
> > >   ...
> > > With "readelf -x ".notes" vmlinux", I can verify the above "func"
> > > with type code 0x101.
> > >
> > > With clang thin-LTO, I got the same as above except the following:
> > >      description data: 01 00 00 00
> > > which indicates the vmlinux is built with LTO.
> > >
> > >   [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> > >   [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> > >
> > > Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> > > Signed-off-by: Yonghong Song <yhs@fb.com>
> >
> > LGTM thanks Yonghong!
> > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> > > ---
> > >  include/linux/elfnote-lto.h | 14 ++++++++++++++
> > >  init/version.c              |  2 ++
> > >  scripts/mod/modpost.c       |  2 ++
> > >  3 files changed, 18 insertions(+)
> > >  create mode 100644 include/linux/elfnote-lto.h
> > >
> > > Changelogs:
> > >   v3 -> v4:
> > >     . put new lto note in its own header file similar to
> > >       build-salt.h. (Nick)
>
> That is a bit smarter (and smaller) than v3.
> Queued up and building a new clang-lto kernel...
> Will report later.
>

link="https://lore.kernel.org/bpf/3f29403d-4942-e362-c98a-4e2d20a3db88@fb.com/T/#t"
b4 -d am $link

Needs this fix for the pahole side?

$ git diff
diff --git a/dwarf_loader.c b/dwarf_loader.c
index 026d13789ff9..244816042c88 100644
--- a/dwarf_loader.c
+++ b/dwarf_loader.c
@@ -2501,8 +2501,8 @@ static int cus__load_debug_types(struct cus
*cus, struct conf_load *conf,
       return 0;
}

-/* Match the define in linux:include/linux/elfnote.h */
-#define LINUX_ELFNOTE_BUILD_LTO                0x101
+/* Match the define in linux:include/linux/elfnote-lto.h */
+#define LINUX_ELFNOTE_LTO_INFO         0x101

static bool cus__merging_cu(Dwarf *dw, Elf *elf)
{
@@ -2520,7 +2520,7 @@ static bool cus__merging_cu(Dwarf *dw, Elf *elf)
                       size_t name_off, desc_off, offset = 0;
                       GElf_Nhdr hdr;
                       while ((offset = gelf_getnote(data, offset,
&hdr, &name_off, &desc_off)) != 0) {
-                               if (hdr.n_type != LINUX_ELFNOTE_BUILD_LTO)
+                               if (hdr.n_type != LINUX_ELFNOTE_LTO_INFO)
                                       continue;

                               /* owner is Linux */

- Sedat -

>
> > >   v2 -> v3:
> > >     . abandoned the approach of adding -grecord-gcc-switches,
> > >       instead create a note to indicate whether it is a lto build
> > >       or not. The note definition is in compiler.h. (Arnaldo)
> > >   v1 -> v2:
> > >     . limited to add -grecord-gcc-switches for LTO_CLANG
> > >       instead of all clang build
> > >
> > > diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
> > > new file mode 100644
> > > index 000000000000..d4635a3ecc4f
> > > --- /dev/null
> > > +++ b/include/linux/elfnote-lto.h
> > > @@ -0,0 +1,14 @@
> > > +#ifndef __ELFNOTE_LTO_H
> > > +#define __ELFNOTE_LTO_H
> > > +
> > > +#include <linux/elfnote.h>
> > > +
> > > +#define LINUX_ELFNOTE_LTO_INFO 0x101
> > > +
> > > +#ifdef CONFIG_LTO
> > > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
> > > +#else
> > > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
> > > +#endif
> > > +
> > > +#endif /* __ELFNOTE_LTO_H */
> > > diff --git a/init/version.c b/init/version.c
> > > index 92afc782b043..1a356f5493e8 100644
> > > --- a/init/version.c
> > > +++ b/init/version.c
> > > @@ -9,6 +9,7 @@
> > >
> > >  #include <generated/compile.h>
> > >  #include <linux/build-salt.h>
> > > +#include <linux/elfnote-lto.h>
> > >  #include <linux/export.h>
> > >  #include <linux/uts.h>
> > >  #include <linux/utsname.h>
> > > @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
> > >         " (" LINUX_COMPILER ") %s\n";
> > >
> > >  BUILD_SALT;
> > > +BUILD_LTO_INFO;
> > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > index 24725e50c7b4..98fb2bb024db 100644
> > > --- a/scripts/mod/modpost.c
> > > +++ b/scripts/mod/modpost.c
> > > @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
> > >          */
> > >         buf_printf(b, "#define INCLUDE_VERMAGIC\n");
> > >         buf_printf(b, "#include <linux/build-salt.h>\n");
> > > +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
> > >         buf_printf(b, "#include <linux/vermagic.h>\n");
> > >         buf_printf(b, "#include <linux/compiler.h>\n");
> > >         buf_printf(b, "\n");
> > >         buf_printf(b, "BUILD_SALT;\n");
> > > +       buf_printf(b, "BUILD_LTO_INFO;\n");
> > >         buf_printf(b, "\n");
> > >         buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
> > >         buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> > > --
> > > 2.30.2
> > >
> >
> >
> > --
> > Thanks,
> > ~Nick Desaulniers
> >
> > --
> > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN%3DUBNdQiO3g%40mail.gmail.com.
Sedat Dilek April 2, 2021, 7:56 p.m. UTC | #4
On Fri, Apr 2, 2021 at 9:38 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Fri, Apr 2, 2021 at 8:31 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> > >
> > > On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> > > >
> > > > Currently, clang LTO built vmlinux won't work with pahole.
> > > > LTO introduced cross-cu dwarf tag references and broke
> > > > current pahole model which handles one cu as a time.
> > > > The solution is to merge all cu's as one pahole cu as in [1].
> > > > We would like to do this merging only if cross-cu dwarf
> > > > references happens. The LTO build mode is a pretty good
> > > > indication for that.
> > > >
> > > > In earlier version of this patch ([2]), clang flag
> > > > -grecord-gcc-switches is proposed to add to compilation flags
> > > > so pahole could detect "-flto" and then merging cu's.
> > > > This will increate the binary size of 1% without LTO though.
> > > >
> > > > Arnaldo suggested to use a note to indicate the vmlinux
> > > > is built with LTO. Such a cheap way to get whether the vmlinux
> > > > is built with LTO or not helps pahole but is also useful
> > > > for tracing as LTO may inline/delete/demote global functions,
> > > > promote static functions, etc.
> > > >
> > > > So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> > > > The owner of the note is "Linux".
> > > >
> > > > With gcc 8.4.1 and clang trunk, without LTO, I got
> > > >   $ readelf -n vmlinux
> > > >   Displaying notes found in: .notes
> > > >     Owner                Data size        Description
> > > >   ...
> > > >     Linux                0x00000004       func
> > > >      description data: 00 00 00 00
> > > >   ...
> > > > With "readelf -x ".notes" vmlinux", I can verify the above "func"
> > > > with type code 0x101.
> > > >
> > > > With clang thin-LTO, I got the same as above except the following:
> > > >      description data: 01 00 00 00
> > > > which indicates the vmlinux is built with LTO.
> > > >
> > > >   [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> > > >   [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> > > >
> > > > Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> > > > Signed-off-by: Yonghong Song <yhs@fb.com>
> > >
> > > LGTM thanks Yonghong!
> > > Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> > >
> > > > ---
> > > >  include/linux/elfnote-lto.h | 14 ++++++++++++++
> > > >  init/version.c              |  2 ++
> > > >  scripts/mod/modpost.c       |  2 ++
> > > >  3 files changed, 18 insertions(+)
> > > >  create mode 100644 include/linux/elfnote-lto.h
> > > >
> > > > Changelogs:
> > > >   v3 -> v4:
> > > >     . put new lto note in its own header file similar to
> > > >       build-salt.h. (Nick)
> >
> > That is a bit smarter (and smaller) than v3.
> > Queued up and building a new clang-lto kernel...
> > Will report later.
> >
>
> link="https://lore.kernel.org/bpf/3f29403d-4942-e362-c98a-4e2d20a3db88@fb.com/T/#t"
> b4 -d am $link
>
> Needs this fix for the pahole side?
>
> $ git diff
> diff --git a/dwarf_loader.c b/dwarf_loader.c
> index 026d13789ff9..244816042c88 100644
> --- a/dwarf_loader.c
> +++ b/dwarf_loader.c
> @@ -2501,8 +2501,8 @@ static int cus__load_debug_types(struct cus
> *cus, struct conf_load *conf,
>        return 0;
> }
>
> -/* Match the define in linux:include/linux/elfnote.h */
> -#define LINUX_ELFNOTE_BUILD_LTO                0x101
> +/* Match the define in linux:include/linux/elfnote-lto.h */
> +#define LINUX_ELFNOTE_LTO_INFO         0x101
>
> static bool cus__merging_cu(Dwarf *dw, Elf *elf)
> {
> @@ -2520,7 +2520,7 @@ static bool cus__merging_cu(Dwarf *dw, Elf *elf)
>                        size_t name_off, desc_off, offset = 0;
>                        GElf_Nhdr hdr;
>                        while ((offset = gelf_getnote(data, offset,
> &hdr, &name_off, &desc_off)) != 0) {
> -                               if (hdr.n_type != LINUX_ELFNOTE_BUILD_LTO)
> +                               if (hdr.n_type != LINUX_ELFNOTE_LTO_INFO)
>                                        continue;
>
>                                /* owner is Linux */
>

I applied above diff against [1] which includes v3:

dwarf_loader: Handle subprogram ret type with abstract_origin properly
dwarf_loader: Check .notes section for LTO build info
dwarf_loader: Check .debug_abbrev for cross-CU references

- Sedat -

[1] https://git.kernel.org/pub/scm/devel/pahole/pahole.git/log/?h=tmp.master

>
> >
> > > >   v2 -> v3:
> > > >     . abandoned the approach of adding -grecord-gcc-switches,
> > > >       instead create a note to indicate whether it is a lto build
> > > >       or not. The note definition is in compiler.h. (Arnaldo)
> > > >   v1 -> v2:
> > > >     . limited to add -grecord-gcc-switches for LTO_CLANG
> > > >       instead of all clang build
> > > >
> > > > diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
> > > > new file mode 100644
> > > > index 000000000000..d4635a3ecc4f
> > > > --- /dev/null
> > > > +++ b/include/linux/elfnote-lto.h
> > > > @@ -0,0 +1,14 @@
> > > > +#ifndef __ELFNOTE_LTO_H
> > > > +#define __ELFNOTE_LTO_H
> > > > +
> > > > +#include <linux/elfnote.h>
> > > > +
> > > > +#define LINUX_ELFNOTE_LTO_INFO 0x101
> > > > +
> > > > +#ifdef CONFIG_LTO
> > > > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
> > > > +#else
> > > > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
> > > > +#endif
> > > > +
> > > > +#endif /* __ELFNOTE_LTO_H */
> > > > diff --git a/init/version.c b/init/version.c
> > > > index 92afc782b043..1a356f5493e8 100644
> > > > --- a/init/version.c
> > > > +++ b/init/version.c
> > > > @@ -9,6 +9,7 @@
> > > >
> > > >  #include <generated/compile.h>
> > > >  #include <linux/build-salt.h>
> > > > +#include <linux/elfnote-lto.h>
> > > >  #include <linux/export.h>
> > > >  #include <linux/uts.h>
> > > >  #include <linux/utsname.h>
> > > > @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
> > > >         " (" LINUX_COMPILER ") %s\n";
> > > >
> > > >  BUILD_SALT;
> > > > +BUILD_LTO_INFO;
> > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > > index 24725e50c7b4..98fb2bb024db 100644
> > > > --- a/scripts/mod/modpost.c
> > > > +++ b/scripts/mod/modpost.c
> > > > @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
> > > >          */
> > > >         buf_printf(b, "#define INCLUDE_VERMAGIC\n");
> > > >         buf_printf(b, "#include <linux/build-salt.h>\n");
> > > > +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
> > > >         buf_printf(b, "#include <linux/vermagic.h>\n");
> > > >         buf_printf(b, "#include <linux/compiler.h>\n");
> > > >         buf_printf(b, "\n");
> > > >         buf_printf(b, "BUILD_SALT;\n");
> > > > +       buf_printf(b, "BUILD_LTO_INFO;\n");
> > > >         buf_printf(b, "\n");
> > > >         buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
> > > >         buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> > > > --
> > > > 2.30.2
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > > ~Nick Desaulniers
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN%3DUBNdQiO3g%40mail.gmail.com.
Sedat Dilek April 6, 2021, 7:05 a.m. UTC | #5
On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> >
> > Currently, clang LTO built vmlinux won't work with pahole.
> > LTO introduced cross-cu dwarf tag references and broke
> > current pahole model which handles one cu as a time.
> > The solution is to merge all cu's as one pahole cu as in [1].
> > We would like to do this merging only if cross-cu dwarf
> > references happens. The LTO build mode is a pretty good
> > indication for that.
> >
> > In earlier version of this patch ([2]), clang flag
> > -grecord-gcc-switches is proposed to add to compilation flags
> > so pahole could detect "-flto" and then merging cu's.
> > This will increate the binary size of 1% without LTO though.
> >
> > Arnaldo suggested to use a note to indicate the vmlinux
> > is built with LTO. Such a cheap way to get whether the vmlinux
> > is built with LTO or not helps pahole but is also useful
> > for tracing as LTO may inline/delete/demote global functions,
> > promote static functions, etc.
> >
> > So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> > The owner of the note is "Linux".
> >
> > With gcc 8.4.1 and clang trunk, without LTO, I got
> >   $ readelf -n vmlinux
> >   Displaying notes found in: .notes
> >     Owner                Data size        Description
> >   ...
> >     Linux                0x00000004       func
> >      description data: 00 00 00 00
> >   ...
> > With "readelf -x ".notes" vmlinux", I can verify the above "func"
> > with type code 0x101.
> >
> > With clang thin-LTO, I got the same as above except the following:
> >      description data: 01 00 00 00
> > which indicates the vmlinux is built with LTO.
> >
> >   [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> >   [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> >
> > Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> > Signed-off-by: Yonghong Song <yhs@fb.com>
>
> LGTM thanks Yonghong!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>

Thanks for the patch.

Feel free to add:

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v12.0.0-rc4 (x86-64)

As a note for the pahole side:
Recent patches require an adaptation of the define and its comment.

1. LINUX_ELFNOTE_BUILD_LTO -> LINUX_ELFNOTE_LTO_INFO
2. include/linux/elfnote.h -> include/linux/elfnote-lto.h

- Sedat -

> > ---
> >  include/linux/elfnote-lto.h | 14 ++++++++++++++
> >  init/version.c              |  2 ++
> >  scripts/mod/modpost.c       |  2 ++
> >  3 files changed, 18 insertions(+)
> >  create mode 100644 include/linux/elfnote-lto.h
> >
> > Changelogs:
> >   v3 -> v4:
> >     . put new lto note in its own header file similar to
> >       build-salt.h. (Nick)
> >   v2 -> v3:
> >     . abandoned the approach of adding -grecord-gcc-switches,
> >       instead create a note to indicate whether it is a lto build
> >       or not. The note definition is in compiler.h. (Arnaldo)
> >   v1 -> v2:
> >     . limited to add -grecord-gcc-switches for LTO_CLANG
> >       instead of all clang build
> >
> > diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
> > new file mode 100644
> > index 000000000000..d4635a3ecc4f
> > --- /dev/null
> > +++ b/include/linux/elfnote-lto.h
> > @@ -0,0 +1,14 @@
> > +#ifndef __ELFNOTE_LTO_H
> > +#define __ELFNOTE_LTO_H
> > +
> > +#include <linux/elfnote.h>
> > +
> > +#define LINUX_ELFNOTE_LTO_INFO 0x101
> > +
> > +#ifdef CONFIG_LTO
> > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
> > +#else
> > +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
> > +#endif
> > +
> > +#endif /* __ELFNOTE_LTO_H */
> > diff --git a/init/version.c b/init/version.c
> > index 92afc782b043..1a356f5493e8 100644
> > --- a/init/version.c
> > +++ b/init/version.c
> > @@ -9,6 +9,7 @@
> >
> >  #include <generated/compile.h>
> >  #include <linux/build-salt.h>
> > +#include <linux/elfnote-lto.h>
> >  #include <linux/export.h>
> >  #include <linux/uts.h>
> >  #include <linux/utsname.h>
> > @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
> >         " (" LINUX_COMPILER ") %s\n";
> >
> >  BUILD_SALT;
> > +BUILD_LTO_INFO;
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 24725e50c7b4..98fb2bb024db 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
> >          */
> >         buf_printf(b, "#define INCLUDE_VERMAGIC\n");
> >         buf_printf(b, "#include <linux/build-salt.h>\n");
> > +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
> >         buf_printf(b, "#include <linux/vermagic.h>\n");
> >         buf_printf(b, "#include <linux/compiler.h>\n");
> >         buf_printf(b, "\n");
> >         buf_printf(b, "BUILD_SALT;\n");
> > +       buf_printf(b, "BUILD_LTO_INFO;\n");
> >         buf_printf(b, "\n");
> >         buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
> >         buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> > --
> > 2.30.2
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN%3DUBNdQiO3g%40mail.gmail.com.
Yonghong Song April 6, 2021, 4:13 p.m. UTC | #6
Masahiro and Michal,

Friendly ping. Any comments on this patch?

The addition LTO .notes information emitted by kernel is used by pahole
in the following patch:
    https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
    (dwarf_loader: check .notes section for lto build info)

Thanks,

Yonghong

On 4/6/21 12:05 AM, Sedat Dilek wrote:
> On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
>>
>> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
>>>
>>> Currently, clang LTO built vmlinux won't work with pahole.
>>> LTO introduced cross-cu dwarf tag references and broke
>>> current pahole model which handles one cu as a time.
>>> The solution is to merge all cu's as one pahole cu as in [1].
>>> We would like to do this merging only if cross-cu dwarf
>>> references happens. The LTO build mode is a pretty good
>>> indication for that.
>>>
>>> In earlier version of this patch ([2]), clang flag
>>> -grecord-gcc-switches is proposed to add to compilation flags
>>> so pahole could detect "-flto" and then merging cu's.
>>> This will increate the binary size of 1% without LTO though.
>>>
>>> Arnaldo suggested to use a note to indicate the vmlinux
>>> is built with LTO. Such a cheap way to get whether the vmlinux
>>> is built with LTO or not helps pahole but is also useful
>>> for tracing as LTO may inline/delete/demote global functions,
>>> promote static functions, etc.
>>>
>>> So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
>>> The owner of the note is "Linux".
>>>
>>> With gcc 8.4.1 and clang trunk, without LTO, I got
>>>    $ readelf -n vmlinux
>>>    Displaying notes found in: .notes
>>>      Owner                Data size        Description
>>>    ...
>>>      Linux                0x00000004       func
>>>       description data: 00 00 00 00
>>>    ...
>>> With "readelf -x ".notes" vmlinux", I can verify the above "func"
>>> with type code 0x101.
>>>
>>> With clang thin-LTO, I got the same as above except the following:
>>>       description data: 01 00 00 00
>>> which indicates the vmlinux is built with LTO.
>>>
>>>    [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
>>>    [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
>>>
>>> Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>
>> LGTM thanks Yonghong!
>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>>
> 
> Thanks for the patch.
> 
> Feel free to add:
> 
> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v12.0.0-rc4 (x86-64)
> 
> As a note for the pahole side:
> Recent patches require an adaptation of the define and its comment.
> 
> 1. LINUX_ELFNOTE_BUILD_LTO -> LINUX_ELFNOTE_LTO_INFO
> 2. include/linux/elfnote.h -> include/linux/elfnote-lto.h
> 
> - Sedat -
> 
>>> ---
>>>   include/linux/elfnote-lto.h | 14 ++++++++++++++
>>>   init/version.c              |  2 ++
>>>   scripts/mod/modpost.c       |  2 ++
>>>   3 files changed, 18 insertions(+)
>>>   create mode 100644 include/linux/elfnote-lto.h
>>>
>>> Changelogs:
>>>    v3 -> v4:
>>>      . put new lto note in its own header file similar to
>>>        build-salt.h. (Nick)
>>>    v2 -> v3:
>>>      . abandoned the approach of adding -grecord-gcc-switches,
>>>        instead create a note to indicate whether it is a lto build
>>>        or not. The note definition is in compiler.h. (Arnaldo)
>>>    v1 -> v2:
>>>      . limited to add -grecord-gcc-switches for LTO_CLANG
>>>        instead of all clang build
>>>
>>> diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
>>> new file mode 100644
>>> index 000000000000..d4635a3ecc4f
>>> --- /dev/null
>>> +++ b/include/linux/elfnote-lto.h
>>> @@ -0,0 +1,14 @@
>>> +#ifndef __ELFNOTE_LTO_H
>>> +#define __ELFNOTE_LTO_H
>>> +
>>> +#include <linux/elfnote.h>
>>> +
>>> +#define LINUX_ELFNOTE_LTO_INFO 0x101
>>> +
>>> +#ifdef CONFIG_LTO
>>> +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
>>> +#else
>>> +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
>>> +#endif
>>> +
>>> +#endif /* __ELFNOTE_LTO_H */
>>> diff --git a/init/version.c b/init/version.c
>>> index 92afc782b043..1a356f5493e8 100644
>>> --- a/init/version.c
>>> +++ b/init/version.c
>>> @@ -9,6 +9,7 @@
>>>
>>>   #include <generated/compile.h>
>>>   #include <linux/build-salt.h>
>>> +#include <linux/elfnote-lto.h>
>>>   #include <linux/export.h>
>>>   #include <linux/uts.h>
>>>   #include <linux/utsname.h>
>>> @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
>>>          " (" LINUX_COMPILER ") %s\n";
>>>
>>>   BUILD_SALT;
>>> +BUILD_LTO_INFO;
>>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
>>> index 24725e50c7b4..98fb2bb024db 100644
>>> --- a/scripts/mod/modpost.c
>>> +++ b/scripts/mod/modpost.c
>>> @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
>>>           */
>>>          buf_printf(b, "#define INCLUDE_VERMAGIC\n");
>>>          buf_printf(b, "#include <linux/build-salt.h>\n");
>>> +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
>>>          buf_printf(b, "#include <linux/vermagic.h>\n");
>>>          buf_printf(b, "#include <linux/compiler.h>\n");
>>>          buf_printf(b, "\n");
>>>          buf_printf(b, "BUILD_SALT;\n");
>>> +       buf_printf(b, "BUILD_LTO_INFO;\n");
>>>          buf_printf(b, "\n");
>>>          buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
>>>          buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
>>> --
>>> 2.30.2
>>>
>>
>>
>> --
>> Thanks,
>> ~Nick Desaulniers
>>
>> --
>> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN=UBNdQiO3g@mail.gmail.com .
Sedat Dilek April 7, 2021, 3:01 a.m. UTC | #7
On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
>
>
> Masahiro and Michal,
>
> Friendly ping. Any comments on this patch?
>
> The addition LTO .notes information emitted by kernel is used by pahole
> in the following patch:
>     https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
>     (dwarf_loader: check .notes section for lto build info)
>

Hi Yonghong,

the above pahole patch has this define and comment:

-static bool cus__merging_cu(Dwarf *dw)
+/* Match the define in linux:include/linux/elfnote.h */
+#define LINUX_ELFNOTE_BUILD_LTO 0x101

...and does not fit with the define and comment in this kernel patch:

+#include <linux/elfnote.h>
+
+#define LINUX_ELFNOTE_LTO_INFO 0x101

Thanks.

- Sedat -


> Thanks,
>
> Yonghong
>
> On 4/6/21 12:05 AM, Sedat Dilek wrote:
> > On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
> > Linux <clang-built-linux@googlegroups.com> wrote:
> >>
> >> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> >>>
> >>> Currently, clang LTO built vmlinux won't work with pahole.
> >>> LTO introduced cross-cu dwarf tag references and broke
> >>> current pahole model which handles one cu as a time.
> >>> The solution is to merge all cu's as one pahole cu as in [1].
> >>> We would like to do this merging only if cross-cu dwarf
> >>> references happens. The LTO build mode is a pretty good
> >>> indication for that.
> >>>
> >>> In earlier version of this patch ([2]), clang flag
> >>> -grecord-gcc-switches is proposed to add to compilation flags
> >>> so pahole could detect "-flto" and then merging cu's.
> >>> This will increate the binary size of 1% without LTO though.
> >>>
> >>> Arnaldo suggested to use a note to indicate the vmlinux
> >>> is built with LTO. Such a cheap way to get whether the vmlinux
> >>> is built with LTO or not helps pahole but is also useful
> >>> for tracing as LTO may inline/delete/demote global functions,
> >>> promote static functions, etc.
> >>>
> >>> So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> >>> The owner of the note is "Linux".
> >>>
> >>> With gcc 8.4.1 and clang trunk, without LTO, I got
> >>>    $ readelf -n vmlinux
> >>>    Displaying notes found in: .notes
> >>>      Owner                Data size        Description
> >>>    ...
> >>>      Linux                0x00000004       func
> >>>       description data: 00 00 00 00
> >>>    ...
> >>> With "readelf -x ".notes" vmlinux", I can verify the above "func"
> >>> with type code 0x101.
> >>>
> >>> With clang thin-LTO, I got the same as above except the following:
> >>>       description data: 01 00 00 00
> >>> which indicates the vmlinux is built with LTO.
> >>>
> >>>    [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> >>>    [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> >>>
> >>> Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> >>> Signed-off-by: Yonghong Song <yhs@fb.com>
> >>
> >> LGTM thanks Yonghong!
> >> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >>
> >
> > Thanks for the patch.
> >
> > Feel free to add:
> >
> > Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v12.0.0-rc4 (x86-64)
> >
> > As a note for the pahole side:
> > Recent patches require an adaptation of the define and its comment.
> >
> > 1. LINUX_ELFNOTE_BUILD_LTO -> LINUX_ELFNOTE_LTO_INFO
> > 2. include/linux/elfnote.h -> include/linux/elfnote-lto.h
> >
> > - Sedat -
> >
> >>> ---
> >>>   include/linux/elfnote-lto.h | 14 ++++++++++++++
> >>>   init/version.c              |  2 ++
> >>>   scripts/mod/modpost.c       |  2 ++
> >>>   3 files changed, 18 insertions(+)
> >>>   create mode 100644 include/linux/elfnote-lto.h
> >>>
> >>> Changelogs:
> >>>    v3 -> v4:
> >>>      . put new lto note in its own header file similar to
> >>>        build-salt.h. (Nick)
> >>>    v2 -> v3:
> >>>      . abandoned the approach of adding -grecord-gcc-switches,
> >>>        instead create a note to indicate whether it is a lto build
> >>>        or not. The note definition is in compiler.h. (Arnaldo)
> >>>    v1 -> v2:
> >>>      . limited to add -grecord-gcc-switches for LTO_CLANG
> >>>        instead of all clang build
> >>>
> >>> diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
> >>> new file mode 100644
> >>> index 000000000000..d4635a3ecc4f
> >>> --- /dev/null
> >>> +++ b/include/linux/elfnote-lto.h
> >>> @@ -0,0 +1,14 @@
> >>> +#ifndef __ELFNOTE_LTO_H
> >>> +#define __ELFNOTE_LTO_H
> >>> +
> >>> +#include <linux/elfnote.h>
> >>> +
> >>> +#define LINUX_ELFNOTE_LTO_INFO 0x101
> >>> +
> >>> +#ifdef CONFIG_LTO
> >>> +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
> >>> +#else
> >>> +#define BUILD_LTO_INFO ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
> >>> +#endif
> >>> +
> >>> +#endif /* __ELFNOTE_LTO_H */
> >>> diff --git a/init/version.c b/init/version.c
> >>> index 92afc782b043..1a356f5493e8 100644
> >>> --- a/init/version.c
> >>> +++ b/init/version.c
> >>> @@ -9,6 +9,7 @@
> >>>
> >>>   #include <generated/compile.h>
> >>>   #include <linux/build-salt.h>
> >>> +#include <linux/elfnote-lto.h>
> >>>   #include <linux/export.h>
> >>>   #include <linux/uts.h>
> >>>   #include <linux/utsname.h>
> >>> @@ -45,3 +46,4 @@ const char linux_proc_banner[] =
> >>>          " (" LINUX_COMPILER ") %s\n";
> >>>
> >>>   BUILD_SALT;
> >>> +BUILD_LTO_INFO;
> >>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> >>> index 24725e50c7b4..98fb2bb024db 100644
> >>> --- a/scripts/mod/modpost.c
> >>> +++ b/scripts/mod/modpost.c
> >>> @@ -2191,10 +2191,12 @@ static void add_header(struct buffer *b, struct module *mod)
> >>>           */
> >>>          buf_printf(b, "#define INCLUDE_VERMAGIC\n");
> >>>          buf_printf(b, "#include <linux/build-salt.h>\n");
> >>> +       buf_printf(b, "#include <linux/elfnote-lto.h>\n");
> >>>          buf_printf(b, "#include <linux/vermagic.h>\n");
> >>>          buf_printf(b, "#include <linux/compiler.h>\n");
> >>>          buf_printf(b, "\n");
> >>>          buf_printf(b, "BUILD_SALT;\n");
> >>> +       buf_printf(b, "BUILD_LTO_INFO;\n");
> >>>          buf_printf(b, "\n");
> >>>          buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
> >>>          buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");
> >>> --
> >>> 2.30.2
> >>>
> >>
> >>
> >> --
> >> Thanks,
> >> ~Nick Desaulniers
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN=UBNdQiO3g@mail.gmail.com .
Yonghong Song April 7, 2021, 6:23 a.m. UTC | #8
On 4/6/21 8:01 PM, Sedat Dilek wrote:
> On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>> Masahiro and Michal,
>>
>> Friendly ping. Any comments on this patch?
>>
>> The addition LTO .notes information emitted by kernel is used by pahole
>> in the following patch:
>>      https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
>>      (dwarf_loader: check .notes section for lto build info)
>>
> 
> Hi Yonghong,
> 
> the above pahole patch has this define and comment:
> 
> -static bool cus__merging_cu(Dwarf *dw)
> +/* Match the define in linux:include/linux/elfnote.h */
> +#define LINUX_ELFNOTE_BUILD_LTO 0x101
> 
> ...and does not fit with the define and comment in this kernel patch:
> 
> +#include <linux/elfnote.h>
> +
> +#define LINUX_ELFNOTE_LTO_INFO 0x101

Thanks, Sedat. I am aware of this. I think we can wait in pahole
to make a change until the kernel patch is finalized and merged.
The kernel patch may still change as we haven't get
maintainer's comment. This will avoid unnecessary churn's
in pahole side.

> 
> Thanks.
> 
> - Sedat -
> 
> 
>> Thanks,
>>
>> Yonghong
>>
>> On 4/6/21 12:05 AM, Sedat Dilek wrote:
>>> On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
>>> Linux <clang-built-linux@googlegroups.com> wrote:
>>>>
>>>> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
>>>>>
>>>>> Currently, clang LTO built vmlinux won't work with pahole.
>>>>> LTO introduced cross-cu dwarf tag references and broke
>>>>> current pahole model which handles one cu as a time.
>>>>> The solution is to merge all cu's as one pahole cu as in [1].
>>>>> We would like to do this merging only if cross-cu dwarf
>>>>> references happens. The LTO build mode is a pretty good
>>>>> indication for that.
>>>>>
>>>>> In earlier version of this patch ([2]), clang flag
>>>>> -grecord-gcc-switches is proposed to add to compilation flags
>>>>> so pahole could detect "-flto" and then merging cu's.
>>>>> This will increate the binary size of 1% without LTO though.
>>>>>
>>>>> Arnaldo suggested to use a note to indicate the vmlinux
>>>>> is built with LTO. Such a cheap way to get whether the vmlinux
>>>>> is built with LTO or not helps pahole but is also useful
>>>>> for tracing as LTO may inline/delete/demote global functions,
>>>>> promote static functions, etc.
>>>>>
>>>>> So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
>>>>> The owner of the note is "Linux".
>>>>>
>>>>> With gcc 8.4.1 and clang trunk, without LTO, I got
>>>>>     $ readelf -n vmlinux
>>>>>     Displaying notes found in: .notes
>>>>>       Owner                Data size        Description
>>>>>     ...
>>>>>       Linux                0x00000004       func
>>>>>        description data: 00 00 00 00
>>>>>     ...
>>>>> With "readelf -x ".notes" vmlinux", I can verify the above "func"
>>>>> with type code 0x101.
>>>>>
>>>>> With clang thin-LTO, I got the same as above except the following:
>>>>>        description data: 01 00 00 00
>>>>> which indicates the vmlinux is built with LTO.
>>>>>
>>>>>     [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
>>>>>     [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
>>>>>
>>>>> Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
>>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>>>
>>>> LGTM thanks Yonghong!
>>>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>>>>
>>>
>>> Thanks for the patch.
>>>
>>> Feel free to add:
>>>
>>> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v12.0.0-rc4 (x86-64)
>>>
>>> As a note for the pahole side:
>>> Recent patches require an adaptation of the define and its comment.
>>>
>>> 1. LINUX_ELFNOTE_BUILD_LTO -> LINUX_ELFNOTE_LTO_INFO
>>> 2. include/linux/elfnote.h -> include/linux/elfnote-lto.h
>>>
>>> - Sedat -
>>>
[...]
Sedat Dilek April 7, 2021, 9:27 a.m. UTC | #9
On Wed, Apr 7, 2021 at 8:23 AM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/6/21 8:01 PM, Sedat Dilek wrote:
> > On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >>
> >> Masahiro and Michal,
> >>
> >> Friendly ping. Any comments on this patch?
> >>
> >> The addition LTO .notes information emitted by kernel is used by pahole
> >> in the following patch:
> >>      https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
> >>      (dwarf_loader: check .notes section for lto build info)
> >>
> >
> > Hi Yonghong,
> >
> > the above pahole patch has this define and comment:
> >
> > -static bool cus__merging_cu(Dwarf *dw)
> > +/* Match the define in linux:include/linux/elfnote.h */
> > +#define LINUX_ELFNOTE_BUILD_LTO 0x101
> >
> > ...and does not fit with the define and comment in this kernel patch:
> >
> > +#include <linux/elfnote.h>
> > +
> > +#define LINUX_ELFNOTE_LTO_INFO 0x101
>
> Thanks, Sedat. I am aware of this. I think we can wait in pahole
> to make a change until the kernel patch is finalized and merged.
> The kernel patch may still change as we haven't get
> maintainer's comment. This will avoid unnecessary churn's
> in pahole side.
>

I am OK with that.

- Sedat -

> >
> > Thanks.
> >
> > - Sedat -
> >
> >
> >> Thanks,
> >>
> >> Yonghong
> >>
> >> On 4/6/21 12:05 AM, Sedat Dilek wrote:
> >>> On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
> >>> Linux <clang-built-linux@googlegroups.com> wrote:
> >>>>
> >>>> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> >>>>>
> >>>>> Currently, clang LTO built vmlinux won't work with pahole.
> >>>>> LTO introduced cross-cu dwarf tag references and broke
> >>>>> current pahole model which handles one cu as a time.
> >>>>> The solution is to merge all cu's as one pahole cu as in [1].
> >>>>> We would like to do this merging only if cross-cu dwarf
> >>>>> references happens. The LTO build mode is a pretty good
> >>>>> indication for that.
> >>>>>
> >>>>> In earlier version of this patch ([2]), clang flag
> >>>>> -grecord-gcc-switches is proposed to add to compilation flags
> >>>>> so pahole could detect "-flto" and then merging cu's.
> >>>>> This will increate the binary size of 1% without LTO though.
> >>>>>
> >>>>> Arnaldo suggested to use a note to indicate the vmlinux
> >>>>> is built with LTO. Such a cheap way to get whether the vmlinux
> >>>>> is built with LTO or not helps pahole but is also useful
> >>>>> for tracing as LTO may inline/delete/demote global functions,
> >>>>> promote static functions, etc.
> >>>>>
> >>>>> So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> >>>>> The owner of the note is "Linux".
> >>>>>
> >>>>> With gcc 8.4.1 and clang trunk, without LTO, I got
> >>>>>     $ readelf -n vmlinux
> >>>>>     Displaying notes found in: .notes
> >>>>>       Owner                Data size        Description
> >>>>>     ...
> >>>>>       Linux                0x00000004       func
> >>>>>        description data: 00 00 00 00
> >>>>>     ...
> >>>>> With "readelf -x ".notes" vmlinux", I can verify the above "func"
> >>>>> with type code 0x101.
> >>>>>
> >>>>> With clang thin-LTO, I got the same as above except the following:
> >>>>>        description data: 01 00 00 00
> >>>>> which indicates the vmlinux is built with LTO.
> >>>>>
> >>>>>     [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> >>>>>     [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> >>>>>
> >>>>> Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> >>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
> >>>>
> >>>> LGTM thanks Yonghong!
> >>>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> >>>>
> >>>
> >>> Thanks for the patch.
> >>>
> >>> Feel free to add:
> >>>
> >>> Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # LLVM/Clang v12.0.0-rc4 (x86-64)
> >>>
> >>> As a note for the pahole side:
> >>> Recent patches require an adaptation of the define and its comment.
> >>>
> >>> 1. LINUX_ELFNOTE_BUILD_LTO -> LINUX_ELFNOTE_LTO_INFO
> >>> 2. include/linux/elfnote.h -> include/linux/elfnote-lto.h
> >>>
> >>> - Sedat -
> >>>
> [...]
Arnaldo Carvalho de Melo April 7, 2021, 1:13 p.m. UTC | #10
Em Fri, Apr 02, 2021 at 11:07:10AM -0700, Nick Desaulniers escreveu:
> On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <yhs@fb.com> wrote:
> > Currently, clang LTO built vmlinux won't work with pahole.
> > LTO introduced cross-cu dwarf tag references and broke
> > current pahole model which handles one cu as a time.
> > The solution is to merge all cu's as one pahole cu as in [1].
> > We would like to do this merging only if cross-cu dwarf
> > references happens. The LTO build mode is a pretty good
> > indication for that.

> > In earlier version of this patch ([2]), clang flag
> > -grecord-gcc-switches is proposed to add to compilation flags
> > so pahole could detect "-flto" and then merging cu's.
> > This will increate the binary size of 1% without LTO though.

> > Arnaldo suggested to use a note to indicate the vmlinux
> > is built with LTO. Such a cheap way to get whether the vmlinux
> > is built with LTO or not helps pahole but is also useful
> > for tracing as LTO may inline/delete/demote global functions,
> > promote static functions, etc.

> > So this patch added an elfnote with a new type LINUX_ELFNOTE_LTO_INFO.
> > The owner of the note is "Linux".

> > With gcc 8.4.1 and clang trunk, without LTO, I got
> >   $ readelf -n vmlinux
> >   Displaying notes found in: .notes
> >     Owner                Data size        Description
> >   ...
> >     Linux                0x00000004       func
> >      description data: 00 00 00 00
> >   ...
> > With "readelf -x ".notes" vmlinux", I can verify the above "func"
> > with type code 0x101.
> >
> > With clang thin-LTO, I got the same as above except the following:
> >      description data: 01 00 00 00
> > which indicates the vmlinux is built with LTO.
> >
> >   [1] https://lore.kernel.org/bpf/20210325065316.3121287-1-yhs@fb.com/
> >   [2] https://lore.kernel.org/bpf/20210331001623.2778934-1-yhs@fb.com/
> >
> > Suggested-by: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
> > Signed-off-by: Yonghong Song <yhs@fb.com>
> 
> LGTM thanks Yonghong!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>

Thanks!

- Arnaldo
Arnaldo Carvalho de Melo April 7, 2021, 1:46 p.m. UTC | #11
Em Tue, Apr 06, 2021 at 11:23:27PM -0700, Yonghong Song escreveu:
> On 4/6/21 8:01 PM, Sedat Dilek wrote:
> > On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
> > > Masahiro and Michal,

> > > Friendly ping. Any comments on this patch?

> > > The addition LTO .notes information emitted by kernel is used by pahole
> > > in the following patch:
> > >      https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
> > >      (dwarf_loader: check .notes section for lto build info)

> > the above pahole patch has this define and comment:

> > -static bool cus__merging_cu(Dwarf *dw)
> > +/* Match the define in linux:include/linux/elfnote.h */
> > +#define LINUX_ELFNOTE_BUILD_LTO 0x101

> > ...and does not fit with the define and comment in this kernel patch:

> > +#include <linux/elfnote.h>
> > +
> > +#define LINUX_ELFNOTE_LTO_INFO 0x101

> Thanks, Sedat. I am aware of this. I think we can wait in pahole
> to make a change until the kernel patch is finalized and merged.
> The kernel patch may still change as we haven't get
> maintainer's comment. This will avoid unnecessary churn's
> in pahole side.

So, I tested with clang 12 on fedora rawhide as well on fedora 33, and
I'm satisfied with the current state to release v1.21, Masahiro, have
you had the time to look at this?

Yonghong, as we have a fallback in case the ELF note isn't available, I
think we're safe even if the notes patch merge gets delayed, right?

- Arnaldo
Yonghong Song April 7, 2021, 2:49 p.m. UTC | #12
On 4/7/21 6:46 AM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Apr 06, 2021 at 11:23:27PM -0700, Yonghong Song escreveu:
>> On 4/6/21 8:01 PM, Sedat Dilek wrote:
>>> On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
>>>> Masahiro and Michal,
> 
>>>> Friendly ping. Any comments on this patch?
> 
>>>> The addition LTO .notes information emitted by kernel is used by pahole
>>>> in the following patch:
>>>>       https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
>>>>       (dwarf_loader: check .notes section for lto build info)
> 
>>> the above pahole patch has this define and comment:
> 
>>> -static bool cus__merging_cu(Dwarf *dw)
>>> +/* Match the define in linux:include/linux/elfnote.h */
>>> +#define LINUX_ELFNOTE_BUILD_LTO 0x101
> 
>>> ...and does not fit with the define and comment in this kernel patch:
> 
>>> +#include <linux/elfnote.h>
>>> +
>>> +#define LINUX_ELFNOTE_LTO_INFO 0x101
> 
>> Thanks, Sedat. I am aware of this. I think we can wait in pahole
>> to make a change until the kernel patch is finalized and merged.
>> The kernel patch may still change as we haven't get
>> maintainer's comment. This will avoid unnecessary churn's
>> in pahole side.
> 
> So, I tested with clang 12 on fedora rawhide as well on fedora 33, and
> I'm satisfied with the current state to release v1.21, Masahiro, have
> you had the time to look at this?
> 
> Yonghong, as we have a fallback in case the ELF note isn't available, I
> think we're safe even if the notes patch merge gets delayed, right?

Right. That is why I separated the notes patch from other patches.
We can revisit it once the kernel patch is settled.

> 
> - Arnaldo
>
Masahiro Yamada April 11, 2021, 12:31 p.m. UTC | #13
On Wed, Apr 7, 2021 at 11:49 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/7/21 6:46 AM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Apr 06, 2021 at 11:23:27PM -0700, Yonghong Song escreveu:
> >> On 4/6/21 8:01 PM, Sedat Dilek wrote:
> >>> On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
> >>>> Masahiro and Michal,
> >
> >>>> Friendly ping. Any comments on this patch?
> >
> >>>> The addition LTO .notes information emitted by kernel is used by pahole
> >>>> in the following patch:
> >>>>       https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
> >>>>       (dwarf_loader: check .notes section for lto build info)
> >
> >>> the above pahole patch has this define and comment:
> >
> >>> -static bool cus__merging_cu(Dwarf *dw)
> >>> +/* Match the define in linux:include/linux/elfnote.h */
> >>> +#define LINUX_ELFNOTE_BUILD_LTO 0x101
> >
> >>> ...and does not fit with the define and comment in this kernel patch:
> >
> >>> +#include <linux/elfnote.h>
> >>> +
> >>> +#define LINUX_ELFNOTE_LTO_INFO 0x101
> >
> >> Thanks, Sedat. I am aware of this. I think we can wait in pahole
> >> to make a change until the kernel patch is finalized and merged.
> >> The kernel patch may still change as we haven't get
> >> maintainer's comment. This will avoid unnecessary churn's
> >> in pahole side.
> >
> > So, I tested with clang 12 on fedora rawhide as well on fedora 33, and
> > I'm satisfied with the current state to release v1.21, Masahiro, have
> > you had the time to look at this?
> >
> > Yonghong, as we have a fallback in case the ELF note isn't available, I
> > think we're safe even if the notes patch merge gets delayed, right?
>
> Right. That is why I separated the notes patch from other patches.
> We can revisit it once the kernel patch is settled.
>
> >
> > - Arnaldo
> >


Applied to linux-kbuild. Thanks.
Yonghong Song April 11, 2021, 5:43 p.m. UTC | #14
On 4/11/21 5:31 AM, Masahiro Yamada wrote:
> On Wed, Apr 7, 2021 at 11:49 PM Yonghong Song <yhs@fb.com> wrote:
>>
>>
>>
>> On 4/7/21 6:46 AM, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Apr 06, 2021 at 11:23:27PM -0700, Yonghong Song escreveu:
>>>> On 4/6/21 8:01 PM, Sedat Dilek wrote:
>>>>> On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
>>>>>> Masahiro and Michal,
>>>
>>>>>> Friendly ping. Any comments on this patch?
>>>
>>>>>> The addition LTO .notes information emitted by kernel is used by pahole
>>>>>> in the following patch:
>>>>>>        https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
>>>>>>        (dwarf_loader: check .notes section for lto build info)
>>>
>>>>> the above pahole patch has this define and comment:
>>>
>>>>> -static bool cus__merging_cu(Dwarf *dw)
>>>>> +/* Match the define in linux:include/linux/elfnote.h */
>>>>> +#define LINUX_ELFNOTE_BUILD_LTO 0x101
>>>
>>>>> ...and does not fit with the define and comment in this kernel patch:
>>>
>>>>> +#include <linux/elfnote.h>
>>>>> +
>>>>> +#define LINUX_ELFNOTE_LTO_INFO 0x101
>>>
>>>> Thanks, Sedat. I am aware of this. I think we can wait in pahole
>>>> to make a change until the kernel patch is finalized and merged.
>>>> The kernel patch may still change as we haven't get
>>>> maintainer's comment. This will avoid unnecessary churn's
>>>> in pahole side.
>>>
>>> So, I tested with clang 12 on fedora rawhide as well on fedora 33, and
>>> I'm satisfied with the current state to release v1.21, Masahiro, have
>>> you had the time to look at this?
>>>
>>> Yonghong, as we have a fallback in case the ELF note isn't available, I
>>> think we're safe even if the notes patch merge gets delayed, right?
>>
>> Right. That is why I separated the notes patch from other patches.
>> We can revisit it once the kernel patch is settled.
>>
>>>
>>> - Arnaldo
>>>
> 
> 
> Applied to linux-kbuild. Thanks.

Thanks!

> 
>
Sedat Dilek April 11, 2021, 6:09 p.m. UTC | #15
On Sun, Apr 11, 2021 at 7:44 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 4/11/21 5:31 AM, Masahiro Yamada wrote:
> > On Wed, Apr 7, 2021 at 11:49 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >>
> >>
> >> On 4/7/21 6:46 AM, Arnaldo Carvalho de Melo wrote:
> >>> Em Tue, Apr 06, 2021 at 11:23:27PM -0700, Yonghong Song escreveu:
> >>>> On 4/6/21 8:01 PM, Sedat Dilek wrote:
> >>>>> On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <yhs@fb.com> wrote:
> >>>>>> Masahiro and Michal,
> >>>
> >>>>>> Friendly ping. Any comments on this patch?
> >>>
> >>>>>> The addition LTO .notes information emitted by kernel is used by pahole
> >>>>>> in the following patch:
> >>>>>>        https://lore.kernel.org/bpf/20210401025825.2254746-1-yhs@fb.com/
> >>>>>>        (dwarf_loader: check .notes section for lto build info)
> >>>
> >>>>> the above pahole patch has this define and comment:
> >>>
> >>>>> -static bool cus__merging_cu(Dwarf *dw)
> >>>>> +/* Match the define in linux:include/linux/elfnote.h */
> >>>>> +#define LINUX_ELFNOTE_BUILD_LTO 0x101
> >>>
> >>>>> ...and does not fit with the define and comment in this kernel patch:
> >>>
> >>>>> +#include <linux/elfnote.h>
> >>>>> +
> >>>>> +#define LINUX_ELFNOTE_LTO_INFO 0x101
> >>>
> >>>> Thanks, Sedat. I am aware of this. I think we can wait in pahole
> >>>> to make a change until the kernel patch is finalized and merged.
> >>>> The kernel patch may still change as we haven't get
> >>>> maintainer's comment. This will avoid unnecessary churn's
> >>>> in pahole side.
> >>>
> >>> So, I tested with clang 12 on fedora rawhide as well on fedora 33, and
> >>> I'm satisfied with the current state to release v1.21, Masahiro, have
> >>> you had the time to look at this?
> >>>
> >>> Yonghong, as we have a fallback in case the ELF note isn't available, I
> >>> think we're safe even if the notes patch merge gets delayed, right?
> >>
> >> Right. That is why I separated the notes patch from other patches.
> >> We can revisit it once the kernel patch is settled.
> >>
> >>>
> >>> - Arnaldo
> >>>
> >
> >
> > Applied to linux-kbuild. Thanks.
>
> Thanks!
>

Great to see this applied.
Thanks.

- Sedat -
diff mbox series

Patch

diff --git a/include/linux/elfnote-lto.h b/include/linux/elfnote-lto.h
new file mode 100644
index 000000000000..d4635a3ecc4f
--- /dev/null
+++ b/include/linux/elfnote-lto.h
@@ -0,0 +1,14 @@ 
+#ifndef __ELFNOTE_LTO_H
+#define __ELFNOTE_LTO_H
+
+#include <linux/elfnote.h>
+
+#define LINUX_ELFNOTE_LTO_INFO	0x101
+
+#ifdef CONFIG_LTO
+#define BUILD_LTO_INFO	ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 1)
+#else
+#define BUILD_LTO_INFO	ELFNOTE32("Linux", LINUX_ELFNOTE_LTO_INFO, 0)
+#endif
+
+#endif /* __ELFNOTE_LTO_H */
diff --git a/init/version.c b/init/version.c
index 92afc782b043..1a356f5493e8 100644
--- a/init/version.c
+++ b/init/version.c
@@ -9,6 +9,7 @@ 
 
 #include <generated/compile.h>
 #include <linux/build-salt.h>
+#include <linux/elfnote-lto.h>
 #include <linux/export.h>
 #include <linux/uts.h>
 #include <linux/utsname.h>
@@ -45,3 +46,4 @@  const char linux_proc_banner[] =
 	" (" LINUX_COMPILER ") %s\n";
 
 BUILD_SALT;
+BUILD_LTO_INFO;
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 24725e50c7b4..98fb2bb024db 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -2191,10 +2191,12 @@  static void add_header(struct buffer *b, struct module *mod)
 	 */
 	buf_printf(b, "#define INCLUDE_VERMAGIC\n");
 	buf_printf(b, "#include <linux/build-salt.h>\n");
+	buf_printf(b, "#include <linux/elfnote-lto.h>\n");
 	buf_printf(b, "#include <linux/vermagic.h>\n");
 	buf_printf(b, "#include <linux/compiler.h>\n");
 	buf_printf(b, "\n");
 	buf_printf(b, "BUILD_SALT;\n");
+	buf_printf(b, "BUILD_LTO_INFO;\n");
 	buf_printf(b, "\n");
 	buf_printf(b, "MODULE_INFO(vermagic, VERMAGIC_STRING);\n");
 	buf_printf(b, "MODULE_INFO(name, KBUILD_MODNAME);\n");