Re: [PATCHv3 0/2] btf_encoder: Fix functions BTF data generation

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>,
	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>
Subject: Re: [PATCHv3 0/2] btf_encoder: Fix functions BTF data generation
Date: Sun, 22 Nov 2020 16:25:38 -0800	[thread overview]
Message-ID: <CAEf4BzYu5OuF-5K91ofEE4Uk3nQWZubgYZWAfVnMGxUb0w7rEw@mail.gmail.com> (raw)
In-Reply-To: <20201122225617.GA1902740@krava>

On Sun, Nov 22, 2020 at 2:56 PM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Fri, Nov 20, 2020 at 05:13:24PM -0800, Andrii Nakryiko wrote:
> > On Sat, Nov 14, 2020 at 2:39 PM Jiri Olsa <jolsa@kernel.org> wrote:
> > >
> > > hi,
> > > recent btf encoder's changes brakes BTF data for some gcc
> > > versions. The problem is that some functions can appear
> > > in dwarf data in some instances without arguments, while
> > > they are defined with some.
> >
> > Hey Jiri,
> >
> > So this approach with __start_mcount_loc/__stop_mcount_loc works for
> > vmlinux only, but it doesn't work for kernel modules. For kernel
> > modules there is a dedicated "__mcount_loc" section, but no
> > __start/__stop symbols. I'm working around for now by making sure
> > functions that I need are global, but it would be nice to have this
> > working for modules out of the box as well.
>
> hi,
> I checked and it's bit more tricky than with vmlinux,
> addresses are in __mcount_loc, but it's all zeros and
> it gets filled after via relocation from .rela__mcount_loc
>
> I think we could do relocation of __mcount_loc section
> with zero base and get all base addresses.. and then
> continue from there with current code checks
>
> I'll check on it tomorrow

Thanks!

>
> >
> > If you get a chance to fix this soon, that would be great. If not,
> > I'll try to get to this ASAP as well, because it would be nice to have
> > this in the same version of pahole that got static function BTFs for
> > vmlinux (if Arnaldo doesn't mind, of course).
>
> we're eagerly expecting the new pahole with the DWARF bug
> workaround, so we asked Arnaldo to release soon, how big
> problem is it for you if the modules fix is in the next one?
>

Not a problem, I just hate remembering all the versions of all the
binaries/libraries/compilers and what each version added. 1.19 had a
chance to be the version which makes fentry/fexit work for all cases,
but I guess it won't happen :) No big deal.

> thanks,
> jirka
>
> >
> > >
> > > v3 changes:
> > >   - move 'generated' flag set out of should_generate_function
> > >   - rename should_generate_function to find_function
> > >   - added ack
> > >
> > > v2 changes:
> > >   - drop patch 3 logic and just change conditions
> > >     based on Andrii's suggestion
> > >   - drop patch 2
> > >   - add ack for patch 1
> > >
> > > thanks,
> > > jirka
> > >
> > >
> > > ---
> > > Jiri Olsa (2):
> > >       btf_encoder: Generate also .init functions
> > >       btf_encoder: Fix function generation
> > >
> > >  btf_encoder.c | 86 +++++++++++++++++++++-----------------------------------------------------------------
> > >  1 file changed, 21 insertions(+), 65 deletions(-)
> > >
> >
>