All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
@ 2021-04-01 23:27 Yonghong Song
  2021-04-02 18:07 ` Nick Desaulniers
  0 siblings, 1 reply; 16+ messages in thread
From: Yonghong Song @ 2021-04-01 23:27 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Alexei Starovoitov, Arnaldo Carvalho de Melo, bpf, kernel-team,
	Masahiro Yamada, Michal Marek, Nick Desaulniers

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

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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-01 23:27 [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto Yonghong Song
@ 2021-04-02 18:07 ` Nick Desaulniers
  2021-04-02 18:31   ` Sedat Dilek
                     ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Nick Desaulniers @ 2021-04-02 18:07 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Masahiro Yamada,
	Michal Marek, Bill Wendling, clang-built-linux

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
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-02 18:07 ` Nick Desaulniers
@ 2021-04-02 18:31   ` Sedat Dilek
  2021-04-02 19:38     ` Sedat Dilek
  2021-04-06  7:05   ` Sedat Dilek
  2021-04-07 13:13   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 16+ messages in thread
From: Sedat Dilek @ 2021-04-02 18:31 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Yonghong Song, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Masahiro Yamada,
	Michal Marek, Bill Wendling, clang-built-linux

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-02 18:31   ` Sedat Dilek
@ 2021-04-02 19:38     ` Sedat Dilek
  2021-04-02 19:56       ` Sedat Dilek
  0 siblings, 1 reply; 16+ messages in thread
From: Sedat Dilek @ 2021-04-02 19:38 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Yonghong Song, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Masahiro Yamada,
	Michal Marek, Bill Wendling, clang-built-linux

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.

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-02 19:38     ` Sedat Dilek
@ 2021-04-02 19:56       ` Sedat Dilek
  0 siblings, 0 replies; 16+ messages in thread
From: Sedat Dilek @ 2021-04-02 19:56 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Yonghong Song, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Masahiro Yamada,
	Michal Marek, Bill Wendling, clang-built-linux

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-02 18:07 ` Nick Desaulniers
  2021-04-02 18:31   ` Sedat Dilek
@ 2021-04-06  7:05   ` Sedat Dilek
  2021-04-06 16:13     ` Yonghong Song
  2021-04-07 13:13   ` Arnaldo Carvalho de Melo
  2 siblings, 1 reply; 16+ messages in thread
From: Sedat Dilek @ 2021-04-06  7:05 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Yonghong Song, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Masahiro Yamada,
	Michal Marek, Bill Wendling, clang-built-linux

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-06  7:05   ` Sedat Dilek
@ 2021-04-06 16:13     ` Yonghong Song
  2021-04-07  3:01       ` Sedat Dilek
  0 siblings, 1 reply; 16+ messages in thread
From: Yonghong Song @ 2021-04-06 16:13 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek
  Cc: Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Bill Wendling,
	clang-built-linux, Nick Desaulniers, Sedat Dilek


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 .

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-06 16:13     ` Yonghong Song
@ 2021-04-07  3:01       ` Sedat Dilek
  2021-04-07  6:23         ` Yonghong Song
  0 siblings, 1 reply; 16+ messages in thread
From: Sedat Dilek @ 2021-04-07  3:01 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list,
	Alexei Starovoitov, Arnaldo Carvalho de Melo, bpf, kernel-team,
	Bill Wendling, clang-built-linux, Nick Desaulniers

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 .

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-07  3:01       ` Sedat Dilek
@ 2021-04-07  6:23         ` Yonghong Song
  2021-04-07  9:27           ` Sedat Dilek
  2021-04-07 13:46           ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 16+ messages in thread
From: Yonghong Song @ 2021-04-07  6:23 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list,
	Alexei Starovoitov, Arnaldo Carvalho de Melo, bpf, kernel-team,
	Bill Wendling, clang-built-linux, Nick Desaulniers



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 -
>>>
[...]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-07  6:23         ` Yonghong Song
@ 2021-04-07  9:27           ` Sedat Dilek
  2021-04-07 13:46           ` Arnaldo Carvalho de Melo
  1 sibling, 0 replies; 16+ messages in thread
From: Sedat Dilek @ 2021-04-07  9:27 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Masahiro Yamada, Michal Marek, Linux Kbuild mailing list,
	Alexei Starovoitov, Arnaldo Carvalho de Melo, bpf, kernel-team,
	Bill Wendling, clang-built-linux, Nick Desaulniers

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 -
> >>>
> [...]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-02 18:07 ` Nick Desaulniers
  2021-04-02 18:31   ` Sedat Dilek
  2021-04-06  7:05   ` Sedat Dilek
@ 2021-04-07 13:13   ` Arnaldo Carvalho de Melo
  2 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-04-07 13:13 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Yonghong Song, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Masahiro Yamada,
	Michal Marek, Bill Wendling, clang-built-linux

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-07  6:23         ` Yonghong Song
  2021-04-07  9:27           ` Sedat Dilek
@ 2021-04-07 13:46           ` Arnaldo Carvalho de Melo
  2021-04-07 14:49             ` Yonghong Song
  1 sibling, 1 reply; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-04-07 13:46 UTC (permalink / raw)
  To: Yonghong Song, Masahiro Yamada, Michal Marek
  Cc: sedat.dilek, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Bill Wendling,
	clang-built-linux, Nick Desaulniers

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-07 13:46           ` Arnaldo Carvalho de Melo
@ 2021-04-07 14:49             ` Yonghong Song
  2021-04-11 12:31               ` Masahiro Yamada
  0 siblings, 1 reply; 16+ messages in thread
From: Yonghong Song @ 2021-04-07 14:49 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Masahiro Yamada, Michal Marek
  Cc: sedat.dilek, Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, kernel-team, Bill Wendling,
	clang-built-linux, Nick Desaulniers



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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-07 14:49             ` Yonghong Song
@ 2021-04-11 12:31               ` Masahiro Yamada
  2021-04-11 17:43                 ` Yonghong Song
  0 siblings, 1 reply; 16+ messages in thread
From: Masahiro Yamada @ 2021-04-11 12:31 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Arnaldo Carvalho de Melo, Michal Marek, Sedat Dilek,
	Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, Kernel Team, Bill Wendling,
	clang-built-linux, Nick Desaulniers

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.


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-11 12:31               ` Masahiro Yamada
@ 2021-04-11 17:43                 ` Yonghong Song
  2021-04-11 18:09                   ` Sedat Dilek
  0 siblings, 1 reply; 16+ messages in thread
From: Yonghong Song @ 2021-04-11 17:43 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Arnaldo Carvalho de Melo, Michal Marek, Sedat Dilek,
	Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, Kernel Team, Bill Wendling,
	clang-built-linux, Nick Desaulniers



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!

> 
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto
  2021-04-11 17:43                 ` Yonghong Song
@ 2021-04-11 18:09                   ` Sedat Dilek
  0 siblings, 0 replies; 16+ messages in thread
From: Sedat Dilek @ 2021-04-11 18:09 UTC (permalink / raw)
  To: Yonghong Song
  Cc: Masahiro Yamada, Arnaldo Carvalho de Melo, Michal Marek,
	Linux Kbuild mailing list, Alexei Starovoitov,
	Arnaldo Carvalho de Melo, bpf, Kernel Team, Bill Wendling,
	clang-built-linux, Nick Desaulniers

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 -

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-04-11 18:10 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-01 23:27 [PATCH kbuild v4] kbuild: add an elfnote for whether vmlinux is built with lto Yonghong Song
2021-04-02 18:07 ` Nick Desaulniers
2021-04-02 18:31   ` Sedat Dilek
2021-04-02 19:38     ` Sedat Dilek
2021-04-02 19:56       ` Sedat Dilek
2021-04-06  7:05   ` Sedat Dilek
2021-04-06 16:13     ` Yonghong Song
2021-04-07  3:01       ` Sedat Dilek
2021-04-07  6:23         ` Yonghong Song
2021-04-07  9:27           ` Sedat Dilek
2021-04-07 13:46           ` Arnaldo Carvalho de Melo
2021-04-07 14:49             ` Yonghong Song
2021-04-11 12:31               ` Masahiro Yamada
2021-04-11 17:43                 ` Yonghong Song
2021-04-11 18:09                   ` Sedat Dilek
2021-04-07 13:13   ` Arnaldo Carvalho de Melo

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.