On Thu, 2 Apr 2015, Steven Rostedt wrote:

> On Thu, 2 Apr 2015 13:11:16 +0200 (CEST)
> Miroslav Benes <mbe...@suse.cz> wrote:
> 
> > On Thu, 5 Mar 2015, Steven Rostedt wrote:
> > 
> > > On Thu, 5 Mar 2015 16:56:43 +0100 (CET)
> > > Miroslav Benes <mbe...@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 <mbe...@suse.cz>

Thanks a lot,
Miroslav


> -- Steve
> 
> From 00ccbf2f5b7580cd7dcdaeda84828d14f0cba3c9 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rost...@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.25...@pobox.suse.cz
> Link: 
> http://lkml.kernel.org/r/1424357773-13536-1-git-send-email-mbe...@suse.cz
> 
> Reported-by: Miroslav Benes <mbe...@suse.cz>
> Signed-off-by: Steven Rostedt <rost...@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
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to