From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752833AbdKIR5D (ORCPT ); Thu, 9 Nov 2017 12:57:03 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:46914 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752529AbdKIR5B (ORCPT ); Thu, 9 Nov 2017 12:57:01 -0500 Date: Thu, 9 Nov 2017 09:56:54 -0800 From: "Paul E. McKenney" To: Ben Hutchings Cc: Steven Rostedt , stable@vger.kernel.org, "gregkh@linuxfoundation.org" , LKML Subject: Re: [4.4,06/28] rcu: Allow for page faults in NMI handlers Reply-To: paulmck@linux.vnet.ibm.com References: <20171016160923.860937466@linuxfoundation.org> <1510240648.2465.46.camel@codethink.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1510240648.2465.46.camel@codethink.co.uk> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17110917-2213-0000-0000-0000023903D0 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00008039; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000239; SDB=6.00943481; UDB=6.00476026; IPR=6.00723764; BA=6.00005682; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017938; XFM=3.00000015; UTC=2017-11-09 17:56:58 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17110917-2214-0000-0000-0000581C142A Message-Id: <20171109175654.GK3624@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-11-09_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1711090240 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Nov 09, 2017 at 03:17:28PM +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? > > > + 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. > > >   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. Indeed, it looks like I need to rework this for 4.5 and earlier. Sorry for the noise! Thanx, Paul From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:53238 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751065AbdKIR5B (ORCPT ); Thu, 9 Nov 2017 12:57:01 -0500 Received: from pps.filterd (m0098421.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id vA9Ht6Pm017628 for ; Thu, 9 Nov 2017 12:57:00 -0500 Received: from e11.ny.us.ibm.com (e11.ny.us.ibm.com [129.33.205.201]) by mx0a-001b2d01.pphosted.com with ESMTP id 2e4u3wa9bk-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 09 Nov 2017 12:57:00 -0500 Received: from localhost by e11.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 9 Nov 2017 12:56:59 -0500 Date: Thu, 9 Nov 2017 09:56:54 -0800 From: "Paul E. McKenney" To: Ben Hutchings Cc: Steven Rostedt , stable@vger.kernel.org, "gregkh@linuxfoundation.org" , LKML Subject: Re: [4.4,06/28] rcu: Allow for page faults in NMI handlers Reply-To: paulmck@linux.vnet.ibm.com References: <20171016160923.860937466@linuxfoundation.org> <1510240648.2465.46.camel@codethink.co.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1510240648.2465.46.camel@codethink.co.uk> Message-Id: <20171109175654.GK3624@linux.vnet.ibm.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Nov 09, 2017 at 03:17:28PM +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? > > > + 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. > > > � 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. Indeed, it looks like I need to rework this for 4.5 and earlier. Sorry for the noise! Thanx, Paul