From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756762AbbLBH7P (ORCPT ); Wed, 2 Dec 2015 02:59:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35944 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755077AbbLBH7O (ORCPT ); Wed, 2 Dec 2015 02:59:14 -0500 Date: Wed, 2 Dec 2015 08:59:11 +0100 From: Jiri Olsa To: Steven Rostedt Cc: LKML , Ingo Molnar , Peter Zijlstra Subject: Re: [PATCH] ftrace: Remove use of control list and ops Message-ID: <20151202075911.GA26308@krava.brq.redhat.com> References: <20151130173640.2c45b429@gandalf.local.home> <20151201134213.GA14155@krava.brq.redhat.com> <20151201095554.7b56bff7@gandalf.local.home> <20151201155744.GA24786@krava.local> <20151201115655.1b7e79e6@gandalf.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151201115655.1b7e79e6@gandalf.local.home> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Dec 01, 2015 at 11:56:55AM -0500, Steven Rostedt wrote: > On Tue, 1 Dec 2015 16:57:44 +0100 > Jiri Olsa 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);