Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753758AbdGSNnL (ORCPT ); Wed, 19 Jul 2017 09:43:11 -0400 Received: from mail-wr0-f196.google.com ([209.85.128.196]:34033 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752760AbdGSNnJ (ORCPT ); Wed, 19 Jul 2017 09:43:09 -0400 Date: Wed, 19 Jul 2017 15:43:07 +0200 From: Frederic Weisbecker To: "Paul E. McKenney" Cc: Peter Zijlstra , Christoph Lameter , "Li, Aubrey" , Andi Kleen , Aubrey Li , tglx@linutronix.de, len.brown@intel.com, rjw@rjwysocki.net, tim.c.chen@linux.intel.com, arjan@linux.intel.com, yang.zhang.wz@gmail.com, x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH v1 00/11] Create fast idle path for short idle periods Message-ID: <20170719134304.GA15611@lerouge> References: <20170710144609.GD31832@tassilo.jf.intel.com> <20170710164206.5aon5kelbisxqyxq@hirez.programming.kicks-ass.net> <20170710172705.GA3441@tassilo.jf.intel.com> <20170711094157.5xcwkloxnjehieqv@hirez.programming.kicks-ass.net> <20170711160926.GA18805@lerouge> <20170711163422.etydkhhtgfthpfi5@hirez.programming.kicks-ass.net> <20170711180931.GP2393@linux.vnet.ibm.com> <20170712115451.xxz3jkau7ape76n5@hirez.programming.kicks-ass.net> <20170712155651.GX2393@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170712155651.GX2393@linux.vnet.ibm.com> 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 Content-Length: 2793 Lines: 78 On Wed, Jul 12, 2017 at 08:56:51AM -0700, Paul E. McKenney wrote: > On Wed, Jul 12, 2017 at 01:54:51PM +0200, Peter Zijlstra wrote: > > On Tue, Jul 11, 2017 at 11:09:31AM -0700, Paul E. McKenney wrote: > > > On Tue, Jul 11, 2017 at 06:34:22PM +0200, Peter Zijlstra wrote: > > > > > > But I think we can at the very least do this; it only gets called from > > > > kernel/sched/idle.c and both callsites have IRQs explicitly disabled by > > > > that point. > > > > > > > > > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > > > > index 51d4c3acf32d..dccf2dc8155a 100644 > > > > --- a/kernel/rcu/tree.c > > > > +++ b/kernel/rcu/tree.c > > > > @@ -843,13 +843,8 @@ static void rcu_eqs_enter(bool user) > > > > */ > > > > void rcu_idle_enter(void) > > > > { > > > > - unsigned long flags; > > > > - > > > > - local_irq_save(flags); > > > > > > With this addition, I am all for it: > > > > > > RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!"); > > > > > > If you are OK with this addition, may I please have your Signed-off-by? > > > > Sure, > > > > Signed-off-by: Peter Zijlstra (Intel) > > Very good, I have queued the patch below. I left out the removal of > the export as I need to work out why the export was there. If it turns > out not to be needed, I will remove the related ones as well. > > Fair enough? > > Thanx, Paul > > ------------------------------------------------------------------------ > > commit 95f3e587ce6388028a51f0c852800fca944e7032 > Author: Peter Zijlstra (Intel) > Date: Wed Jul 12 07:59:54 2017 -0700 > > rcu: Make rcu_idle_enter() rely on callers disabling irqs > > All callers to rcu_idle_enter() have irqs disabled, so there is no > point in rcu_idle_enter disabling them again. This commit therefore > replaces the irq disabling with a RCU_LOCKDEP_WARN(). > > Signed-off-by: Peter Zijlstra (Intel) > Signed-off-by: Paul E. McKenney > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 6de6b1c5ee53..c78b076653ce 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -840,11 +840,8 @@ static void rcu_eqs_enter(bool user) > */ > void rcu_idle_enter(void) > { > - unsigned long flags; > - > - local_irq_save(flags); > + RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_idle_enter() invoked with irqs enabled!!!"); Nice! I really need to make that LOCKDEP_ASSERT_IRQS_DISABLED() patchset because there are many places where either we have WARN_ON_ONCE(!irqs_disabled()) or we need it but we were too shy to do it for performance reason. > rcu_eqs_enter(false); > - local_irq_restore(flags); > } > EXPORT_SYMBOL_GPL(rcu_idle_enter); > >