From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161166AbaJ3RBX (ORCPT ); Thu, 30 Oct 2014 13:01:23 -0400 Received: from cdptpa-outbound-snat.email.rr.com ([107.14.166.226]:5926 "EHLO cdptpa-oedge-vip.email.rr.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S933193AbaJ3RBW (ORCPT ); Thu, 30 Oct 2014 13:01:22 -0400 Date: Thu, 30 Oct 2014 13:01:19 -0400 From: Steven Rostedt To: "H. Peter Anvin" Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Andrew Morton , Masami Hiramatsu , Jiri Kosina , Josh Poimboeuf , Vojtech Pavlik , Seth Jennings , "Paul E. McKenney" Subject: Re: [for-next][PATCH 3/4] ftrace/x86: Allow !CONFIG_PREEMPT dynamic ops to use allocated trampolines Message-ID: <20141030130119.465f1f0e@gandalf.local.home> In-Reply-To: <20141027182948.581142569@goodmis.org> References: <20141027182702.778680710@goodmis.org> <20141027182948.581142569@goodmis.org> X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-RR-Connecting-IP: 107.14.168.118:25 X-Cloudmark-Score: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org H. Peter, Can you give me your acked-by for this? Thanks, -- Steve On Mon, 27 Oct 2014 14:27:05 -0400 Steven Rostedt wrote: > From: "Steven Rostedt (Red Hat)" > > When the static ftrace_ops (like function tracer) enables tracing, and it > is the only callback that is referencing a function, a trampoline is > dynamically allocated to the function that calls the callback directly > instead of calling a loop function that iterates over all the registered > ftrace ops (if more than one ops is registered). > > But when it comes to dynamically allocated ftrace_ops, where they may be > freed, on a CONFIG_PREEMPT kernel there's no way to know when it is safe > to free the trampoline. If a task was preempted while executing on the > trampoline, there's currently no way to know when it will be off that > trampoline. > > But this is not true when it comes to !CONFIG_PREEMPT. The current method > of calling schedule_on_each_cpu() will force tasks off the trampoline, > becaues they can not schedule while on it (kernel preemption is not > configured). That means it is safe to free a dynamically allocated > ftrace ops trampoline when CONFIG_PREEMPT is not configured. > > Cc: H. Peter Anvin > Cc: Paul E. McKenney > Signed-off-by: Steven Rostedt > --- > arch/x86/kernel/ftrace.c | 8 ++++++++ > kernel/trace/ftrace.c | 18 ++++++++++++++++++ > 2 files changed, 26 insertions(+) > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c > index ca17c20a1010..4cfeca6ffe11 100644 > --- a/arch/x86/kernel/ftrace.c > +++ b/arch/x86/kernel/ftrace.c > @@ -913,6 +913,14 @@ void *arch_ftrace_trampoline_func(struct ftrace_ops *ops, struct dyn_ftrace *rec > return addr_from_call((void *)ops->trampoline + offset); > } > > +void arch_ftrace_trampoline_free(struct ftrace_ops *ops) > +{ > + if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP)) > + return; > + > + tramp_free((void *)ops->trampoline); > + ops->trampoline = 0; > +} > > #endif /* CONFIG_X86_64 */ > #endif /* CONFIG_DYNAMIC_FTRACE */ > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 422e1f8300b1..eab3123a1fbe 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -2324,6 +2324,10 @@ static void ftrace_run_modify_code(struct ftrace_ops *ops, int command, > static ftrace_func_t saved_ftrace_func; > static int ftrace_start_up; > > +void __weak arch_ftrace_trampoline_free(struct ftrace_ops *ops) > +{ > +} > + > static void control_ops_free(struct ftrace_ops *ops) > { > free_percpu(ops->disabled); > @@ -2475,6 +2479,8 @@ static int ftrace_shutdown(struct ftrace_ops *ops, int command) > if (ops->flags & (FTRACE_OPS_FL_DYNAMIC | FTRACE_OPS_FL_CONTROL)) { > schedule_on_each_cpu(ftrace_sync); > > + arch_ftrace_trampoline_free(ops); > + > if (ops->flags & FTRACE_OPS_FL_CONTROL) > control_ops_free(ops); > } > @@ -4725,9 +4731,21 @@ void __weak arch_ftrace_update_trampoline(struct ftrace_ops *ops) > > static void ftrace_update_trampoline(struct ftrace_ops *ops) > { > + > +/* > + * Currently there's no safe way to free a trampoline when the kernel > + * is configured with PREEMPT. That is because a task could be preempted > + * when it jumped to the trampoline, it may be preempted for a long time > + * depending on the system load, and currently there's no way to know > + * when it will be off the trampoline. If the trampoline is freed > + * too early, when the task runs again, it will be executing on freed > + * memory and crash. > + */ > +#ifdef CONFIG_PREEMPT > /* Currently, only non dynamic ops can have a trampoline */ > if (ops->flags & FTRACE_OPS_FL_DYNAMIC) > return; > +#endif > > arch_ftrace_update_trampoline(ops); > }