On Thu, Apr 06, 2017 at 02:13:29PM -0400, Steven Rostedt wrote:
> On Thu, 6 Apr 2017 11:05:53 -0700
> "Paul E. McKenney" <[email protected]> wrote:
>
> > On Thu, Apr 06, 2017 at 12:42:38PM -0400, Steven Rostedt wrote:
> > > From: "Steven Rostedt (VMware)" <[email protected]>
> > >
> > > 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" <[email protected]>
> >
> > One question below. Other than that:
> >
> > Acked-by: "Paul E. McKenney" <[email protected]>
>
> Thanks!
>
> >
> > > Signed-off-by: Steven Rostedt (VMware) <[email protected]>
> > > ---
> > > 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.
Now that you mention it, that does seem like it might be important. ;-)
Ah, I was mistakenly looking at GLOB_SELFTEST instead of just plain
GLOB -- sorry for the noise!
Thanx, Paul
> -- 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
>
