linux-kernel - Re: [PATCH RFC tip/perf/core] ftrace/x86: Let dynamic trampolines call ops->func even for dynamic fops

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.00.1504031053110.19873@pobox.suse.cz>
Date:	Fri, 3 Apr 2015 11:29:04 +0200 (CEST)
From:	Miroslav Benes <mbenes@...e.cz>
To:	Steven Rostedt <rostedt@...dmis.org>
cc:	mingo@...hat.com, tglx@...utronix.de, hpa@...or.com,
	linux-kernel@...r.kernel.org, x86@...nel.org, jkosina@...e.cz
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@...e.cz> wrote:
> 
> > On Thu, 5 Mar 2015, Steven Rostedt wrote:
> > 
> > > On Thu, 5 Mar 2015 16:56:43 +0100 (CET)
> > > Miroslav Benes <mbenes@...e.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@...e.cz>

Thanks a lot,
Miroslav


> -- Steve
> 
> From 00ccbf2f5b7580cd7dcdaeda84828d14f0cba3c9 Mon Sep 17 00:00:00 2001
> From: "Steven Rostedt (Red Hat)" <rostedt@...dmis.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@...e.cz>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.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 majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/