lkml.org 
[lkml]   [2015]   [Apr]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
>


\
 
 \ /
  Last update: 2015-04-03 12:01    [W:1.468 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site