Re: [PATCH 3/3] btf_encoder: Change functions check due to broken dwarf

From: Andrii Nakryiko <andrii.nakryiko@gmail.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	dwarves@vger.kernel.org, bpf <bpf@vger.kernel.org>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andriin@fb.com>, Yonghong Song <yhs@fb.com>,
	Hao Luo <haoluo@google.com>, "Frank Ch. Eigler" <fche@redhat.com>,
	Mark Wielaard <mjw@redhat.com>
Subject: Re: [PATCH 3/3] btf_encoder: Change functions check due to broken dwarf
Date: Wed, 11 Nov 2020 12:36:55 -0800	[thread overview]
Message-ID: <CAEf4BzZ089_ECxY_tFBMUc5c-rwtD9Mw8n7anUsdgpzgoipsPg@mail.gmail.com> (raw)
In-Reply-To: <20201111203130.GC619201@krava>

On Wed, Nov 11, 2020 at 12:31 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Wed, Nov 11, 2020 at 09:19:29PM +0100, Jiri Olsa wrote:
> > On Wed, Nov 11, 2020 at 11:59:20AM -0800, Andrii Nakryiko wrote:
> >
> > SNIP
> >
> > > > +       if (!fl->init_bpf_begin &&
> > > > +           !strcmp("__init_bpf_preserve_type_begin", elf_sym__name(sym, btfe->symtab)))
> > > > +               fl->init_bpf_begin = sym->st_value;
> > > > +
> > > > +       if (!fl->init_bpf_end &&
> > > > +           !strcmp("__init_bpf_preserve_type_end", elf_sym__name(sym, btfe->symtab)))
> > > > +               fl->init_bpf_end = sym->st_value;
> > > > +}
> > > > +
> > > > +static int has_all_symbols(struct funcs_layout *fl)
> > > > +{
> > > > +       return fl->mcount_start && fl->mcount_stop &&
> > > > +              fl->init_begin && fl->init_end &&
> > > > +              fl->init_bpf_begin && fl->init_bpf_end;
> > >
> > > See below for what seems to be the root cause for the immediate problem.
> > >
> > > But me, Alexei and Daniel had a discussion offline, and we concluded
> > > that this special bpf_preserve_init section is probably not the right
> > > approach overall. We should roll back the bpf patch and instead adjust
> > > pahole's approach. I think we should just drop the __init check and
> > > include all the __init functions into BTF. There could be cases where
> > > we'd need to attach BPF programs to __init functions (e.g., bpf_lsm
> > > security cases), so having BTFs for those FUNCs are necessary as well.
> > > Ftrace currently disallows that, but it's only because no user-space
> > > application has a way to attach probes early enough. This might change
> > > in the future, so there is no need to invent special mechanisms now
> > > for bpf_iter function preservation. Let's just include all __init
> > > functions in BTF. Can you please do that change and check how much
> > > more functions we get in BTF? Thanks!
> >
> > sure, not problem to keep all init functions, will give you the count
>
> with pahole change below (on top of current master) and kernel
> without the special init section, I'm getting over ~2000 functions
> more on my .config:
>
>   $ bpftool btf dump file ./vmlinux | grep 'FUNC ' | wc -l
>   41505
>   $ bpftool btf dump file /sys/kernel/btf/vmlinux | grep 'FUNC ' | wc -l
>   39256

That's a very small percentage increase, let's just do this.

>
> jirka
>
>

[...]