From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933617AbdDFSNy (ORCPT ); Thu, 6 Apr 2017 14:13:54 -0400 Received: from mail.kernel.org ([198.145.29.136]:50184 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933243AbdDFSNe (ORCPT ); Thu, 6 Apr 2017 14:13:34 -0400 Date: Thu, 6 Apr 2017 14:13:29 -0400 From: Steven Rostedt To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Andrew Morton Subject: Re: [PATCH 1/4] ftrace: Add use of synchronize_rcu_tasks() with dynamic trampolines Message-ID: <20170406141329.31d85e93@gandalf.local.home> In-Reply-To: <20170406180553.GG1600@linux.vnet.ibm.com> References: <20170406164237.874767449@goodmis.org> <20170406164432.069467885@goodmis.org> <20170406180553.GG1600@linux.vnet.ibm.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 6 Apr 2017 11:05:53 -0700 "Paul E. McKenney" wrote: > On Thu, Apr 06, 2017 at 12:42:38PM -0400, Steven Rostedt wrote: > > From: "Steven Rostedt (VMware)" > > > > The function tracer needs to be more careful than other subsystems when it > > comes to freeing data. Especially if that data is actually executable code. > > When a single function is traced, a trampoline can be dynamically allocated > > which is called to jump to the function trace callback. When the callback is > > no longer needed, the dynamic allocated trampoline needs to be freed. This > > is where the issues arise. The dynamically allocated trampoline must not be > > used again. As function tracing can trace all subsystems, including > > subsystems that are used to serialize aspects of freeing (namely RCU), it > > must take extra care when doing the freeing. > > > > Before synchronize_rcu_tasks() was around, there was no way for the function > > tracer to know that nothing was using the dynamically allocated trampoline > > when CONFIG_PREEMPT was enabled. That's because a task could be indefinitely > > preempted while sitting on the trampoline. Now with synchronize_rcu_tasks(), > > it will wait till all tasks have either voluntarily scheduled (not on the > > trampoline) or goes into userspace (not on the trampoline). Then it is safe > > to free the trampoline even with CONFIG_PREEMPT set. > > > > Cc: "Paul E. McKenney" > > One question below. Other than that: > > Acked-by: "Paul E. McKenney" Thanks! > > > Signed-off-by: Steven Rostedt (VMware) > > --- > > kernel/trace/Kconfig | 3 ++- > > kernel/trace/ftrace.c | 42 ++++++++++++++++++------------------------ > > 2 files changed, 20 insertions(+), 25 deletions(-) > > > > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig > > index d4a06e714645..67b463b4f169 100644 > > --- a/kernel/trace/Kconfig > > +++ b/kernel/trace/Kconfig > > @@ -134,7 +134,8 @@ config FUNCTION_TRACER > > select KALLSYMS > > select GENERIC_TRACER > > select CONTEXT_SWITCH_TRACER > > - select GLOB > > + select GLOB > > Does GLOB really want to be selected in production environments? > I could understand "select GLOB if WE_ARE_TESTING" or some such. Note, this patch just fixes the whitespace issue for the "select GLOB". All the selects had a tab in front of them, where as GLOB had all spaces. As for your question, FUNCTION_TRACER depends on glob. It's what is used for the function filters. You can do: echo '*sync*rcu*' > /sys/kernel/tracing/set_ftrace_filter And get: synchronize_rcu_tasks synchronize_srcu synchronize_srcu_expedited sync_rcu_exp_select_cpus sync_rcu_exp_handler synchronize_rcu_bh.part.65 synchronize_rcu_expedited synchronize_rcu.part.67 synchronize_rcu synchronize_rcu_bh selected. So yes, production environments do want it selected. -- Steve > > > + select TASKS_RCU if PREEMPT > > help > > Enable the kernel to trace every kernel function. This is done > > by using a compiler feature to insert a small, 5-byte No-Operation