Messages in this thread | | | Date | Wed, 2 Dec 2015 08:59:11 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH] ftrace: Remove use of control list and ops |
| |
On Tue, Dec 01, 2015 at 11:56:55AM -0500, Steven Rostedt wrote: > On Tue, 1 Dec 2015 16:57:44 +0100 > Jiri Olsa <jolsa@redhat.com> wrote: > > > > > Hmm, I thought that I forced the list function when RCU or PER_CPU > > > was set. Oh wait. I have CONFIG_PREEMPT set, which will change the > > > logic slightly. I'm guessing you have PREEMPT_VOLUNTARY set. I'll try > > > that out. > > > > yep, but the trampoline has separate code path > > to set the ops func > > Right, but that only gets called for DYNAMIC ops (which perf is) when > CONFIG_PREEMPT is not set. What about this patch instead? (on top of my > other one) > > It's a lot like yours, but instead of creating a new helper function, I > just reuse the recurs_func instead. > > -- Steve > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 8b22e681baf8..05ed87d06bb0 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -5230,20 +5230,29 @@ static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip) > > /* > * If there's only one function registered but it does not support > - * recursion, this function will be called by the mcount trampoline. > - * This function will handle recursion protection. > + * recursion, needs RCU protection and/or requires per cpu handling, then > + * this function will be called by the mcount trampoline. > */ > -static void ftrace_ops_recurs_func(unsigned long ip, unsigned long parent_ip, > +static void ftrace_ops_assist_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct pt_regs *regs) > { > int bit; > > + if ((op->flags & FTRACE_OPS_FL_RCU) && !rcu_is_watching()) > + return; > + > bit = trace_test_and_set_recursion(TRACE_LIST_START, TRACE_LIST_MAX); > if (bit < 0) > return; > > - op->func(ip, parent_ip, op, regs); > + preempt_disable_notrace();
I was wondering about not disabling preemption in the original ftrace_ops_recurs_func function.. thats why I added new one ;-)
I'll test the patch
thanks, jirka
> > + if (!(op->flags & FTRACE_OPS_FL_PER_CPU) || > + ftrace_function_local_disabled(op)) { > + op->func(ip, parent_ip, op, regs); > + } > + > + preempt_enable_notrace(); > trace_clear_recursion(bit);
| |