dwarves.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>,
	Jiri Olsa <jolsa@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: [PATCH 2/2] btf_encoder: Fix function generation
Date: Mon, 16 Nov 2020 19:21:45 +0100	[thread overview]
Message-ID: <20201116182145.GF1081385@krava> (raw)
In-Reply-To: <20201116135016.GA509215@kernel.org>

On Mon, Nov 16, 2020 at 10:50:16AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Nov 13, 2020 at 01:43:47PM -0800, Andrii Nakryiko escreveu:
> > On Fri, Nov 13, 2020 at 1:29 PM Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > > On Fri, Nov 13, 2020 at 12:56:40PM -0800, Andrii Nakryiko wrote:
> > > > On Fri, Nov 13, 2020 at 7:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> 
> > > > > Current conditions for picking up function records break
> > > > > BTF data on some gcc versions.
> 
> > > > > Some function records can appear with no arguments but with
> > > > > declaration tag set, so moving the 'fn->declaration' in front
> > > > > of other checks.
> 
> > > > > Then checking if argument names are present and finally checking
> > > > > ftrace filter if it's present. If ftrace filter is not available,
> > > > > using the external tag to filter out non external functions.
> 
> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> > > > I tested locally, all seems to work fine. Left few suggestions below,
> > > > but those could be done in follow ups (or argued to not be done).
> 
> > > > Acked-by: Andrii Nakryiko <andrii@kernel.org>
> 
> > > > BTW, for some stats.
> 
> > > > BEFORE allowing static funcs:
> 
> 
> Nowhere in the last patchkit comments is some explanation for the
> inclusion of static functions :-\ After the first patch in the last
> series I get:
> 
>   $ llvm-objcopy --remove-section=.BTF vmlinux
>   $ readelf -SW vmlinux  | grep BTF
>   $ pahole -J vmlinux
>   $ bpftool btf dump file ./vmlinux | grep 'FUNC '| cut -d\' -f2 | sort > before.bpftool
>   $ cp vmlinux vmlinux.before.all
>   $ wc -l before.bpftool
>   28829 before.bpftool

I think you see the original number of functions, because without
the 'not merged' kernel patch, that added the special init section,
pahole will fail to detect vmlinux and fall back to checking dwarf
declarations

there's a verbose message for the fall back, but it is not displayed
at the moment ;-) with the fix below you should see it:

  $ LLVM_OBJCOPY=objcopy ./pahole -V -J vmlinux >out
  $ cat out | grep 'vmlinux not detected'
  vmlinux not detected, falling back to dwarf data

I'll check on the verbose setup and send full patch,
I did not expect it would not get printed, sry

so the new numebr ~41k functions is together static functions
and init functions

jirka


---
diff --git a/btf_encoder.c b/btf_encoder.c
index 9b93e9963727..7efd26de5815 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -584,6 +584,8 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 	struct tag *pos;
 	int err = 0;
 
+	btf_elf__verbose = verbose;
+
 	if (btfe && strcmp(btfe->filename, cu->filename)) {
 		err = btf_encoder__encode();
 		if (err)
@@ -623,7 +625,6 @@ int cu__encode_btf(struct cu *cu, int verbose, bool force,
 		}
 	}
 
-	btf_elf__verbose = verbose;
 	btf_elf__force = force;
 	type_id_off = btf__get_nr_types(btfe->btf);
 
diff --git a/lib/bpf b/lib/bpf
--- a/lib/bpf
+++ b/lib/bpf
@@ -1 +1 @@
-Subproject commit ff797cc905d9c5fe9acab92d2da127342b20f80f
+Subproject commit ff797cc905d9c5fe9acab92d2da127342b20f80f-dirty


  reply	other threads:[~2020-11-16 18:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13 15:12 [PATCHv2 0/2] btf_encoder: Fix functions BTF data generation Jiri Olsa
2020-11-13 15:12 ` [PATCH 1/2] btf_encoder: Generate also .init functions Jiri Olsa
2020-11-13 15:12 ` [PATCH 2/2] btf_encoder: Fix function generation Jiri Olsa
2020-11-13 17:28   ` Arnaldo Carvalho de Melo
2020-11-13 18:11     ` Jiri Olsa
2020-11-13 20:56   ` Andrii Nakryiko
2020-11-13 21:29     ` Jiri Olsa
2020-11-13 21:43       ` Andrii Nakryiko
2020-11-16 13:50         ` Arnaldo Carvalho de Melo
2020-11-16 18:21           ` Jiri Olsa [this message]
2020-11-16 19:15             ` Arnaldo Carvalho de Melo
2020-11-16 19:22               ` Arnaldo Carvalho de Melo
2020-11-16 19:40                 ` Jiri Olsa
2020-11-16 19:44                   ` Arnaldo Carvalho de Melo
2020-11-14 22:38 [PATCHv3 0/2] btf_encoder: Fix functions BTF data generation Jiri Olsa
2020-11-14 22:38 ` [PATCH 2/2] btf_encoder: Fix function generation Jiri Olsa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201116182145.GF1081385@krava \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=haoluo@google.com \
    --cc=jolsa@kernel.org \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).