Dwarves Archive mirror
 help / color / mirror / Atom feed
From: "Michal Suchánek" <msuchanek@suse.de>
To: Martin KaFai Lau <kafai@fb.com>
Cc: Nathan Chancellor <nathan@kernel.org>,
	Jiri Slaby <jirislaby@kernel.org>, Jiri Olsa <jolsa@kernel.org>,
	dwarves@vger.kernel.org,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	bpf@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v2 dwarves] btf: Remove ftrace filter
Date: Mon, 10 May 2021 20:08:58 +0200	[thread overview]
Message-ID: <20210510180858.GI12700@kitsune.suse.cz> (raw)
In-Reply-To: <20210510180059.tz527zrb6bcqjrl7@kafai-mbp.dhcp.thefacebook.com>

On Mon, May 10, 2021 at 11:00:59AM -0700, Martin KaFai Lau wrote:
> On Sat, May 08, 2021 at 12:53:01PM +0200, Michal Suchánek wrote:
> > On Fri, May 07, 2021 at 10:03:21PM -0700, Martin KaFai Lau wrote:
> > > On Thu, May 06, 2021 at 07:54:37PM -0700, Nathan Chancellor wrote:
> > > > On Thu, May 06, 2021 at 01:56:22PM -0700, Martin KaFai Lau wrote:
> > > > > BTF is currently generated for functions that are in ftrace list
> > > > > or extern.
> > > > > 
> > > > > A recent use case also needs BTF generated for functions included in
> > > > > allowlist.  In particular, the kernel
> > > > > commit e78aea8b2170 ("bpf: tcp: Put some tcp cong functions in allowlist for bpf-tcp-cc")
> > > > > allows bpf program to directly call a few tcp cc kernel functions. Those
> > > > > kernel functions are currently allowed only if CONFIG_DYNAMIC_FTRACE
> > > > > is set to ensure they are in the ftrace list but this kconfig dependency
> > > > > is unnecessary.
> > > > > 
> > > > > Those kernel functions are specified under an ELF section .BTF_ids.
> > > > > There was an earlier attempt [0] to add another filter for the functions in
> > > > > the .BTF_ids section.  That discussion concluded that the ftrace filter
> > > > > should be removed instead.
> > > > > 
> > > > > This patch is to remove the ftrace filter and its related functions.
> > > > > 
> > > > > Number of BTF FUNC with and without is_ftrace_func():
> > > > > My kconfig in x86: 40643 vs 46225
> > > > > Jiri reported on arm: 25022 vs 55812
> > > > > 
> > > > > [0]: https://lore.kernel.org/dwarves/20210423213728.3538141-1-kafai@fb.com/
> > > > > 
> > > > > Cc: Andrii Nakryiko <andrii@kernel.org>
> > > > > Cc: Jiri Olsa <jolsa@kernel.org>
> > > > > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > > > 
> > > > This fixes an issue with Fedora's s390x config that CKI noticed:
> > > > 
> > > > https://groups.google.com/g/clang-built-linux/c/IzthpckBJvc/m/MPWGDmXiAwAJ  
> > > > 
> > > > Tested-by: Nathan Chancellor <nathan@kernel.org> # build
> > > Thanks all for reviewing and testing.
> > > 
> > > In my cross compile ppc64 test, it does not solve the issue.
> > > The problem is the tcp-cc functions (e.g. "cublictcp_*")
> > > are not STT_FUNC in ppc64, so they are not collected in collect_function().
> > > The ".cubictcp_*" is STT_FUNC though.
> > > 
> > > Since only the x86 (64 and 32) bpf jit can call these tcp-cc functions now
> > > and there is no usage for adding them to .BTF_ids for other ARCHs,
> > > I have post a patch to limit them to x86:
> > > https://lore.kernel.org/bpf/20210508005011.3863757-1-kafai@fb.com/
> > 
> > That's probably not the way to go. If function symbols start with a dot
> > in ppc64 elfv1 abi then pahole should learn to add a dot for ppc64 elfv1
> > abi.
> > 
> > Or we can build ppc64 BE using the elfv2 abi and depend on elfv2 abi for
> > BTF on ppc64. The patch for enabling elfv2 for BE is currently under
> > discussion and I have the patch that adds the dependency ready as well.
> ic.  I just learned the elfv2/LE vs elfv1/BE difference in ppc64.
> elfv2 BE ppc64 support is on its way, so it is better to have DEBUG_INFO_BTF
> depending on the ppc64-elfv2 related kconfig(s) instead of making an exception
> handling in pahole.
> 
> If you have the	BTF ppc64-elfv2 dependency patch ready, does it make
> sense to land this patch first independent of the ppc64 elfv2/BE
> effort?

It kind of depends on the result of the config option name bikeshedding
so it has to be merged after.

Thanks

Michal

  reply	other threads:[~2021-05-10 18:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-06 20:56 [PATCH v2 dwarves] btf: Remove ftrace filter Martin KaFai Lau
2021-05-07  2:54 ` Nathan Chancellor
2021-05-08  5:03   ` Martin KaFai Lau
2021-05-08 10:53     ` Michal Suchánek
2021-05-10 18:00       ` Martin KaFai Lau
2021-05-10 18:08         ` Michal Suchánek [this message]
2021-05-07  6:05 ` Jiri Slaby
2021-05-07 10:39 ` Jiri Olsa
2021-05-08 14:41   ` Arnaldo Carvalho de Melo

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=20210510180858.GI12700@kitsune.suse.cz \
    --to=msuchanek@suse.de \
    --cc=acme@kernel.org \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=dwarves@vger.kernel.org \
    --cc=jirislaby@kernel.org \
    --cc=jolsa@kernel.org \
    --cc=kafai@fb.com \
    --cc=kernel-team@fb.com \
    --cc=nathan@kernel.org \
    /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 read-only IMAP folder(s) and NNTP newsgroup(s).