From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755137AbdKIRgB convert rfc822-to-8bit (ORCPT ); Thu, 9 Nov 2017 12:36:01 -0500 Received: from mail.kernel.org ([198.145.29.99]:47546 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754505AbdKIRf4 (ORCPT ); Thu, 9 Nov 2017 12:35:56 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 05C31218BB Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Thu, 9 Nov 2017 12:35:53 -0500 From: Steven Rostedt To: Ben Hutchings Cc: stable@vger.kernel.org, "Paul E. McKenney" , "gregkh@linuxfoundation.org" , LKML Subject: Re: [4.4,06/28] rcu: Allow for page faults in NMI handlers Message-ID: <20171109123553.364d640c@gandalf.local.home> In-Reply-To: <1510240648.2465.46.camel@codethink.co.uk> References: <20171016160923.860937466@linuxfoundation.org> <1510240648.2465.46.camel@codethink.co.uk> 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=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 09 Nov 2017 15:17:28 +0000 Ben Hutchings wrote: > On Mon, 2017-10-16 at 18:11 +0200, gregkh@linuxfoundation.org wrote: > > 4.4-stable review patch.  If anyone has any objections, please let me know. > > > > ------------------ > > > > From: Paul E. McKenney > > > > commit 28585a832602747cbfa88ad8934013177a3aae38 upstream. > > > > A number of architecture invoke rcu_irq_enter() on exception entry in > > order to allow RCU read-side critical sections in the exception handler > > when the exception is from an idle or nohz_full CPU.  This works, at > > least unless the exception happens in an NMI handler.  In that case, > > rcu_nmi_enter() would already have exited the extended quiescent state, > > which would mean that rcu_irq_enter() would (incorrectly) cause RCU > > to think that it is again in an extended quiescent state.  This will > > in turn result in lockdep splats in response to later RCU read-side > > critical sections. > > > > This commit therefore causes rcu_irq_enter() and rcu_irq_exit() to > > take no action if there is an rcu_nmi_enter() in effect, thus avoiding > > the unscheduled return to RCU quiescent state.  This in turn should > > make the kernel safe for on-demand RCU voyeurism. > > > > Link: http://lkml.kernel.org/r/20170922211022.GA18084@linux.vnet.ibm.com > > > > Cc: stable@vger.kernel.org > > Fixes: 0be964be0 ("module: Sanitize RCU usage and locking") > > > Reported-by: Steven Rostedt > > > Signed-off-by: Paul E. McKenney > > > Signed-off-by: Steven Rostedt (VMware) > > > Signed-off-by: Greg Kroah-Hartman > > --- > >  kernel/rcu/tree.c |   12 ++++++++++++ > >  1 file changed, 12 insertions(+) > > > > --- a/kernel/rcu/tree.c > > +++ b/kernel/rcu/tree.c > > @@ -759,6 +759,12 @@ void rcu_irq_exit(void) > >   > >   local_irq_save(flags); > >   rdtp = this_cpu_ptr(&rcu_dynticks); > > + > > + /* Page faults can happen in NMI handlers, so check... */ > > + if (READ_ONCE(rdtp->dynticks_nmi_nesting)) > > + return; > > Shouldn't there be a local_irq_restore() on this return path? Or does > this condition imply that IRQs were already disabled? It does, but there still should be a local_irq_restore(), because it will might confuse lockdep (lockdep keeps track of these), and it just looks bad. > > > + RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_exit() invoked with irqs enabled!!!"); > > I don't see why you added RCU_LOCKDEP_WARN() here. Prior to 4.5 it's > not an error to call this function with IRQs disabled. And after > calling local_irq_save(), it's redundant to assert that IRQs are > disabled. good point. > > >   oldval = rdtp->dynticks_nesting; > >   rdtp->dynticks_nesting--; > >   WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && > > @@ -887,6 +893,12 @@ void rcu_irq_enter(void) > >   > >   local_irq_save(flags); > >   rdtp = this_cpu_ptr(&rcu_dynticks); > > + > > + /* Page faults can happen in NMI handlers, so check... */ > > + if (READ_ONCE(rdtp->dynticks_nmi_nesting)) > > + return; > > + > > + RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_enter() invoked with irqs enabled!!!"); > > Same problems here. -- Steve