All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
	davem@davemloft.net, daniel@iogearbox.net, x86@kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH bpf 1/3] ftrace: Fix function_graph tracer interaction with BPF trampoline
Date: Tue, 10 Dec 2019 15:49:33 -0800	[thread overview]
Message-ID: <20191210234931.mfaklfs7s4i4fsxo@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <20191210183519.41772e0f@gandalf.local.home>

On Tue, Dec 10, 2019 at 06:35:19PM -0500, Steven Rostedt wrote:
> On Sun, 8 Dec 2019 16:01:12 -0800
> Alexei Starovoitov <ast@kernel.org> wrote:
> 
> >  #ifndef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> > index 67e0c462b059..a2659735db73 100644
> > --- a/kernel/trace/fgraph.c
> > +++ b/kernel/trace/fgraph.c
> > @@ -101,6 +101,15 @@ int function_graph_enter(unsigned long ret, unsigned long func,
> >  {
> >  	struct ftrace_graph_ent trace;
> >  
> > +	/*
> > +	 * Skip graph tracing if the return location is served by direct trampoline,
> > +	 * since call sequence and return addresses is unpredicatable anymore.
> > +	 * Ex: BPF trampoline may call original function and may skip frame
> > +	 * depending on type of BPF programs attached.
> > +	 */
> > +	if (ftrace_direct_func_count &&
> > +	    ftrace_find_rec_direct(ret - MCOUNT_INSN_SIZE))
> 
> My only worry is that this may not work for all archs that implement
> it. But I figure we can cross that bridge when we get to it.

Right. Since bpf trampoline is going to be the only user in short term
it's not an issue, since trampoline is x86-64 only so far.

> > +		return -EBUSY;
> >  	trace.func = func;
> >  	trace.depth = ++current->curr_ret_depth;
> >  
> 
> I added this patch to my queue and it's about 70% done going through my
> test suite (takes around 10 - 13 hours).
> 
> As I'm about to send a pull request to Linus tomorrow, I could include
> this patch (as it will be fully tested), and then you could apply the
> other two when it hits Linus's tree.
> 
> Would that work for you?

Awesome. Much appreciate additional testing. I can certainly wait another day.
I was hoping to get patch 2 all the way to Linus's tree before rc2 to make sure
register_ftrace_direct() API is used for real in this kernel cycle. When
everything will land I'll backport to our production kernel and then the actual
stress testing begins :)


  reply	other threads:[~2019-12-10 23:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09  0:01 [PATCH bpf 0/3] bpf: Make BPF trampoline friendly to ftrace Alexei Starovoitov
2019-12-09  0:01 ` [PATCH bpf 1/3] ftrace: Fix function_graph tracer interaction with BPF trampoline Alexei Starovoitov
2019-12-10 16:19   ` Alexei Starovoitov
2019-12-10 16:30     ` Steven Rostedt
2019-12-10 23:35   ` Steven Rostedt
2019-12-10 23:49     ` Alexei Starovoitov [this message]
2019-12-09  0:01 ` [PATCH bpf 2/3] bpf: Make BPF trampoline use register_ftrace_direct() API Alexei Starovoitov
2019-12-09  0:01 ` [PATCH bpf 3/3] selftests/bpf: test function_graph tracer and bpf trampoline together Alexei Starovoitov
2019-12-09 18:42 ` [PATCH bpf 0/3] bpf: Make BPF trampoline friendly to ftrace Martin Lau

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=20191210234931.mfaklfs7s4i4fsxo@ast-mbp.dhcp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=x86@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.