From: Masami Hiramatsu <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Frederic Weisbecker <frederic@kernel.org>,
Joel Fernandes <joel@joelfernandes.org>,
Andy Lutomirski <luto@kernel.org>,
Mark Rutland <mark.rutland@arm.com>
Subject: Re: [RFC][PATCH 00/14] function_graph: Rewrite to allow multiple users
Date: Mon, 26 Nov 2018 18:21:12 +0900 [thread overview]
Message-ID: <20181126182112.422b914dd00ecb36e15f7b07@kernel.org> (raw)
In-Reply-To: <20181122012708.491151844@goodmis.org>
Hi Steve,
On Wed, 21 Nov 2018 20:27:08 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
>
> I talked with many of you at Plumbers about rewriting the function graph
> tracer. Well, this is it. I was originally going to produce just a
> proof of concept, but when I found that I had to fix a design flaw
> and that covered all the arch code anyway, I decided to do more of a
> RFC patch set.
Thank you for starting this work! This might be a good way to simplify
treating of shadow stacks in kretprobe and function-graph-tracers.
(And I hope this can help me to remove some kind of complexity in
kretprobes)
>
> I probably should add more comments to the code, and update the
> function graph design documentation, but I wanted to get this out
> before the US Turkey day for your enjoyment while you try to let your
> pants buckle again.
:)
Let me try to review and port kretprobe on it. On the way, I will find
issues if there are.
>
> Why the rewrite?
>
> Well the fuction graph tracer is arguably the strongest of the tracers.
> It shows both the entrance and exit of a function, can give the timings
> of a function, and shows the execution of the code quite nicely.
>
> But it has one major flaw.
>
> It can't let more than one user access it at a time. The function
> tracer has had that feature for years now, but due to the design of
> the function graph tracer it was difficult to implement. Why?
>
> Because you must maintain the state of a three-tuple.
>
> Task, Function, Callback
>
> The state is determined at by the entryfunc and must be passed to the
> retfunc when the function being traced returns. But this is not an
> easy task, as that state can be different for each task, each function
> and each callback.
>
> What's the solution? I use the shadow stack that is already being
> used to store the function return addresses.
>
> A big thanks to Masami Hiramatsu for suggesting this idea!
>
> For now, I only allow an 16 users of the function graph tracer at a time.
> That should be more than enough. I create an array of 16 fgraph_ops
> pointers. When a user registers their fgraph_ops to the function graph
> tracer, it is assigned an index into that array, which will hold a pointer
> to the fgraph_ops being registered.
>
> On entry of the function, the array is iterated and each entryfunc of
> the fgraph_ops in the array is called. If the entryfunc returns non-zero,
> then the index of that fgraph_ops is pushed on the shadow stack (along
> with the index to the "ret_stack entry" structure, for fast access
> to it). If the entryfunc returns zero, then it is ignored. If at least
> one function returned non-zero then the return of the traced function
> will also be traced.
>
> On the return of the function, the shadow stack is examined and all
> the indexes that were pushed on the stack is read, and each fgraph_ops
> retfunc is called in the reverse order.
>
> When a fgraph_ops is unregistered, its index in the array is set to point
> to a "stub" fgraph_ops that holds stub functions that just return
> "0" for the entryfunc and does nothing for the retfunc. This is because
> the retfunc may be called literally days after the entryfunc is called
> and we want to be able to free the fgraph_ops that is unregistered.
>
> Note, if another fgraph_ops is registered in the same location, its
> retfunc may be called that was set by a previous fgraph_ops. This
> is not a regression because that's what can happen today if you unregister
> a callback from the current function_graph tracer and register another
> one. If this is an issue, there are ways to solve it.
Yeah, I need the solution, maybe an API to get correct return address? :)
By the way, are there any way to hold a private data on each ret_stack entry?
Since kretprobe supports "entry data" passed from entry_handler to
return handler, we have to store the data or data-instance on the ret_stack.
This feature is used by systemtap to save the function entry data, like
function parameters etc., so that return handler analyzes the parameters
with return value.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
next prev parent reply other threads:[~2018-11-26 9:21 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-22 1:27 [RFC][PATCH 00/14] function_graph: Rewrite to allow multiple users Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 01/14] fgraph: Create a fgraph.c file to store function graph infrastructure Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 02/14] fgraph: Have set_graph_notrace only affect function_graph tracer Steven Rostedt
2018-11-23 0:01 ` Namhyung Kim
2018-11-23 17:37 ` Steven Rostedt
2018-11-24 5:49 ` Namhyung Kim
2018-11-24 18:41 ` Steven Rostedt
2018-11-26 4:54 ` Namhyung Kim
2018-11-22 1:27 ` [RFC][PATCH 03/14] arm64: function_graph: Remove use of FTRACE_NOTRACE_DEPTH Steven Rostedt
2018-11-27 19:31 ` Will Deacon
2018-11-27 19:50 ` Steven Rostedt
2018-12-05 21:50 ` [PATCH 03/14 v2] " Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 04/14] function_graph: Remove the " Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 05/14] ftrace: Create new ftrace-internal.h header Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 06/14] fgraph: Move function graph specific code into fgraph.c Steven Rostedt
2018-11-23 6:11 ` Joel Fernandes
2018-11-23 17:58 ` Steven Rostedt
2018-11-23 18:11 ` Steven Rostedt
2018-11-23 22:13 ` Joel Fernandes
2018-11-26 7:25 ` Masami Hiramatsu
2018-11-22 1:27 ` [RFC][PATCH 07/14] fgraph: Add new fgraph_ops structure to enable function graph hooks Steven Rostedt
2018-11-23 2:59 ` Joel Fernandes
2018-11-23 18:25 ` Steven Rostedt
2018-11-26 11:30 ` Masami Hiramatsu
2018-11-26 21:06 ` Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 08/14] function_graph: Remove unused task_curr_ret_stack() Steven Rostedt
2018-11-26 7:40 ` Masami Hiramatsu
2018-11-26 21:26 ` Steven Rostedt
2018-11-26 10:02 ` Joey Pabalinas
2018-11-26 21:27 ` Steven Rostedt
2018-11-26 21:37 ` Joey Pabalinas
2018-11-22 1:27 ` [RFC][PATCH 09/14] function_graph: Move ftrace_graph_get_addr() to fgraph.c Steven Rostedt
2018-11-23 3:13 ` Joel Fernandes
2018-11-23 19:25 ` Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 10/14] function_graph: Have profiler use new helper ftrace_graph_get_ret_stack() Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 11/14] function_graph: Convert ret_stack to a series of longs Steven Rostedt
2018-11-24 5:31 ` Joel Fernandes
2018-11-26 16:07 ` Masami Hiramatsu
2018-11-26 16:26 ` Steven Rostedt
2018-11-28 1:38 ` Joel Fernandes
2018-11-26 21:31 ` Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 12/14] function_graph: Add an array structure that will allow multiple callbacks Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 13/14] function_graph: Allow multiple users to attach to function graph Steven Rostedt
2018-11-22 1:27 ` [RFC][PATCH 14/14] function_graph: Allow for more than one callback to be registered Steven Rostedt
2018-11-22 10:08 ` [RFC][PATCH 00/14] function_graph: Rewrite to allow multiple users Peter Zijlstra
2018-11-22 12:46 ` Steven Rostedt
2018-11-22 13:42 ` Peter Zijlstra
2018-11-26 9:21 ` Masami Hiramatsu [this message]
2018-11-26 16:32 ` Steven Rostedt
2018-11-29 14:29 ` Masami Hiramatsu
2018-11-29 16:46 ` Steven Rostedt
2018-11-30 2:26 ` Masami Hiramatsu
2018-11-30 3:24 ` Steven Rostedt
2018-11-30 14:11 ` Masami Hiramatsu
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=20181126182112.422b914dd00ecb36e15f7b07@kernel.org \
--to=mhiramat@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=frederic@kernel.org \
--cc=joel@joelfernandes.org \
--cc=jpoimboe@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/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).