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

[Nick Desaulniers]

On Thu, Apr 1, 2021 at 4:27 PM Yonghong Song <y...@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...@fb.com/
> [2] https://lore.kernel.org/bpf/20210331001623...@fb.com/
>
> Suggested-by: Arnaldo Carvalho de Melo <arnald...@gmail.com>
> Signed-off-by: Yonghong Song <y...@fb.com>

LGTM thanks Yonghong!
Reviewed-by: Nick Desaulniers <ndesau...@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

[Sedat Dilek]

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

- Sedat -

> --
> 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-li...@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]

link="https://lore.kernel.org/bpf/3f29403d-4942-e362...@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 Dilek]

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

[Sedat Dilek]

I tried successfully with Clang-ThinLTO plus BTF and DWARF-v5
debug-info Kconfigs enabled.

### Toolchain

Selfmade LLVM/Clang/LLD version 12.0.0-rc4 (x86-64)

### Some relevant Linux Kconfigs

$ grep 'LTO_|_BTF|DWARF' .config | grep ^CONFIG | sort
CONFIG_ARCH_SUPPORTS_LTO_CLANG_THIN=y
CONFIG_ARCH_SUPPORTS_LTO_CLANG=y
CONFIG_DEBUG_INFO_BTF_MODULES=y
CONFIG_DEBUG_INFO_BTF=y
CONFIG_DEBUG_INFO_DWARF5=y
CONFIG_HAS_LTO_CLANG=y
CONFIG_LTO_CLANG_THIN=y
CONFIG_LTO_CLANG=y
CONFIG_PAHOLE_HAS_SPLIT_BTF=y

Note: Full linux-config attached

### make and make-options

/usr/bin/perf_5.10 stat make V=1 -j4 LLVM=1 LLVM_IAS=1
PAHOLE=/opt/pahole/bin/pahole LOCALVERSION=-11-amd64-clang12
-lto KBUILD_BUILD_HOST=iniza KBUILD_BUILD_USER=sedat...@gmail.com
KBUILD_BUILD_TIMESTAMP=2021-04-03 bindeb-pkg
KDEB_PKGVERSION=5.12.0~rc5-11~bullseye+dileks1

### Linux-kernel

base: v5.12-rc5-117-gd93a0d43e3d0 ("Merge tag 'block-5.12-2021-04-02'
of git://git.kernel.dk/linux-block")

patch: kbuild: add an elfnote for whether vmlinux is built with lto (v4)

extra-patch against kbuild-next (attached): kbuild: dwarf5: Drop
DEBUG_INFO_BTF dependency

### pahole

base: v1.20-11-g39227909db3c ("dwarf_loader: Permit merging all DWARF
CU's for clang LTO built binary")

patches from [1] on top of base:

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

extra patch (attached):
dwarf_loader: Fix LINUX_ELFNOTE_LTO_INFO define

### Binary sizes in MiB

525 vmlinux

### llvm-readelf

$ /opt/llvm-toolchain/bin/llvm-readelf-12 -n vmlinux
Linux 0x00000004 func
description data: 01 00 00 00

Note: "01" in description data: indicates it is an "LTO" build (see
"LINUX_ELFNOTE_LTO_INFO" define).

Thanks.

[Sedat Dilek]

On Fri, Apr 2, 2021 at 8:07 PM 'Nick Desaulniers' via Clang Built
Linux <clang-bu...@googlegroups.com> wrote:
>

Thanks for the patch.

Feel free to add:

Tested-by: Sedat Dilek <sedat...@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 -

[Yonghong Song]

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...@fb.com/
(dwarf_loader: check .notes section for lto build info)

Thanks,

Yonghong

>> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/CAKwvOdmX8d3XTzJFk5rN_PnOQYJ8bXMrh8DrhzqN=UBNd...@mail.gmail.com .

[Sedat Dilek]

On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <y...@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...@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 -

[Yonghong Song]

On 4/6/21 8:01 PM, Sedat Dilek wrote:
> On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <y...@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...@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.

[...]

[Sedat Dilek]

On Wed, Apr 7, 2021 at 8:23 AM Yonghong Song <y...@fb.com> wrote:
>
>
>
> On 4/6/21 8:01 PM, Sedat Dilek wrote:
> > On Tue, Apr 6, 2021 at 6:13 PM Yonghong Song <y...@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...@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 -

[Arnaldo Carvalho de Melo]

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

Thanks!

- Arnaldo

[Arnaldo Carvalho de Melo]

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 <y...@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...@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]

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]

Applied to linux-kbuild. Thanks.


--
Best Regards
Masahiro Yamada

[Yonghong Song]

Thanks!

>
>

[Sedat Dilek]

Great to see this applied.
Thanks.

- Sedat -