Messages in this thread | | | Date | Fri, 3 Apr 2015 11:29:04 +0200 (CEST) | From | Miroslav Benes <> | Subject | Re: [PATCH RFC tip/perf/core] ftrace/x86: Let dynamic trampolines call ops->func even for dynamic fops |
| |
On Thu, 2 Apr 2015, Steven Rostedt wrote:
> On Thu, 2 Apr 2015 13:11:16 +0200 (CEST) > Miroslav Benes <mbenes@suse.cz> wrote: > > > On Thu, 5 Mar 2015, Steven Rostedt wrote: > > > > > On Thu, 5 Mar 2015 16:56:43 +0100 (CET) > > > Miroslav Benes <mbenes@suse.cz> wrote: > > > > > > > I don't know if you plan to do something about this patch or if you just > > > > missed it in your e-mail pile. Should I resend it or have you already > > > > scratched that? > > > > > > Thanks for the reminder. It was marked as "todo" but fell in the noise. > > > It's worse that I was traveling when you sent this, which makes it more > > > likely to be missed :-/ > > > > > > Anyway, I'm still trying to catch up. If you don't hear from me by next > > > Thursday, please send me another reminder. > > > > ping... > > > > Thanks for the reminder. I modified your patch to this. Would this work > for you? It only needs to touch the generic ftrace.c file. I kept your > change log though.
Yes, this works but there is a slight difference to the previous behaviour which I tried to preserve in my patch. ftrace_ops_get_func in your patch does not check FTRACE_FORCE_LIST_FUNC. I do not know if that is a real problem. Anyway your approach looks much better and dynamic trampolines are now used for live patches. So in this context you can add
Tested-by: Miroslav Benes <mbenes@suse.cz>
Thanks a lot, Miroslav
> -- Steve > > From 00ccbf2f5b7580cd7dcdaeda84828d14f0cba3c9 Mon Sep 17 00:00:00 2001 > From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org> > Date: Thu, 19 Feb 2015 15:56:14 +0100 > Subject: [PATCH] ftrace: Let dynamic trampolines call ops->func even for > dynamic fops > > Dynamically allocated trampolines call ftrace_ops_get_func to get the > function which they should call. For dynamic fops (FTRACE_OPS_FL_DYNAMIC > flag is set) ftrace_ops_list_func is always returned. This is reasonable > for static trampolines but goes against the main advantage of dynamic > ones, that is avoidance of going through the list of all registered > callbacks for functions that are only being traced by a single callback. > > We can fix it by returning ops->func (or recursion safe version) from > ftrace_ops_get_func whenever it is possible for dynamic trampolines. > > Note that dynamic trampolines are not allowed for dynamic fops if > CONFIG_PREEMPT=y. > > Link: http://lkml.kernel.org/r/alpine.LNX.2.00.1501291023000.25445@pobox.suse.cz > Link: http://lkml.kernel.org/r/1424357773-13536-1-git-send-email-mbenes@suse.cz > > Reported-by: Miroslav Benes <mbenes@suse.cz> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > --- > kernel/trace/ftrace.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 4f228024055b..d01d238d8ef4 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -249,6 +249,19 @@ static void update_function_graph_func(void); > static inline void update_function_graph_func(void) { } > #endif > > + > +static ftrace_func_t ftrace_ops_get_list_func(struct ftrace_ops *ops) > +{ > + /* > + * If this is a dynamic ops or we force list func, > + * then it needs to call the list anyway. > + */ > + if (ops->flags & FTRACE_OPS_FL_DYNAMIC || FTRACE_FORCE_LIST_FUNC) > + return ftrace_ops_list_func; > + > + return ftrace_ops_get_func(ops); > +} > + > static void update_ftrace_function(void) > { > ftrace_func_t func; > @@ -270,7 +283,7 @@ static void update_ftrace_function(void) > * then have the mcount trampoline call the function directly. > */ > } else if (ftrace_ops_list->next == &ftrace_list_end) { > - func = ftrace_ops_get_func(ftrace_ops_list); > + func = ftrace_ops_get_list_func(ftrace_ops_list); > > } else { > /* Just use the default ftrace_ops */ > @@ -5209,13 +5222,6 @@ static void ftrace_ops_recurs_func(unsigned long ip, unsigned long parent_ip, > ftrace_func_t ftrace_ops_get_func(struct ftrace_ops *ops) > { > /* > - * If this is a dynamic ops or we force list func, > - * then it needs to call the list anyway. > - */ > - if (ops->flags & FTRACE_OPS_FL_DYNAMIC || FTRACE_FORCE_LIST_FUNC) > - return ftrace_ops_list_func; > - > - /* > * If the func handles its own recursion, call it directly. > * Otherwise call the recursion protected function that > * will call the ftrace ops function. > -- > 1.8.3.1 >
| |