[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20181129232927.74ca5f294e97fc58b15bf8c6@kernel.org>
Date: Thu, 29 Nov 2018 23:29:27 +0900
From: Masami Hiramatsu <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...nel.org>,
Andrew Morton <akpm@...ux-foundation.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Josh Poimboeuf <jpoimboe@...hat.com>,
Frederic Weisbecker <frederic@...nel.org>,
Joel Fernandes <joel@...lfernandes.org>,
Andy Lutomirski <luto@...nel.org>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [RFC][PATCH 00/14] function_graph: Rewrite to allow multiple
users
Hi Steve,
On Mon, 26 Nov 2018 11:32:15 -0500
Steven Rostedt <rostedt@...dmis.org> wrote:
> On Mon, 26 Nov 2018 18:21:12 +0900
> Masami Hiramatsu <mhiramat@...nel.org> wrote:
>
>
> > > 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? :)
>
> One way to solve this is to also have a counter array that gets updated
> every time the index array gets updated. And save the counter to the
> shadow stack index as well. This way, we only call the return if the
> counter on the stack matches what's in the counter on the counter array
> for the index.
Hmm, but we already know the current stack "header" entry when calling
handlers, don't we? I thought we just calcurate out from curr_ret_stack.
> > 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.
>
> Yes, I remember you telling me about this at plumbers, and while I was
> writing this code I had that in mind. It wouldn't be too hard to
> implement, I just left it off for now. I also left it off because I
> have some questions about what exactly is needed. What size do you
> require to be stored. Especially if we want to keep the shadow stack
> smaller. I was going to find a way to implement some of the data that
> is already stored via the ret_stack with this instead, and make the
> ret_stack entry smaller. Should we allow just sizeof(long)*3? or just
> let user choose any size and if they run out of stack, then too bad. We
> just wont let it crash.
I need only sizeof(unsigned long). If the kretprobe user requires more,
it will be fall back to current method -- get an "instance" and store
its address to the entry :-)
Thank you,
--
Masami Hiramatsu <mhiramat@...nel.org>
Powered by blists - more mailing lists