From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932215AbdIYEll convert rfc822-to-8bit (ORCPT ); Mon, 25 Sep 2017 00:41:41 -0400 Received: from smtprelay0025.hostedemail.com ([216.40.44.25]:35231 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753213AbdIYElk (ORCPT ); Mon, 25 Sep 2017 00:41:40 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,::::::::::::,RULES_HIT:2:41:69:152:355:379:599:800:960:968:973:988:989:1260:1261:1277:1311:1313:1314:1345:1359:1437:1513:1515:1516:1518:1521:1535:1593:1594:1605:1606:1730:1747:1777:1792:2194:2198:2199:2200:2393:2553:2559:2562:2693:2731:2840:2895:2901:2904:3138:3139:3140:3141:3142:3865:3866:3867:3868:3870:3871:3872:3873:3874:4117:4250:4321:4362:5007:6117:6119:6691:7652:7875:7903:8957:9040:10004:10848:11026:11232:11473:11658:11914:12043:12296:12438:12555:12663:12683:12740:12895:12986:14093:14096:14097:21080:21365:21433:21434:21451:21611:21627:30012:30026:30034:30045:30054:30056:30060:30070:30090:30091,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:2,LUA_SUMMARY:none X-HE-Tag: cup36_4844f49ee911e X-Filterd-Recvd-Size: 6541 Date: Mon, 25 Sep 2017 06:41:30 +0200 User-Agent: K-9 Mail for Android In-Reply-To: <20170925003456.GA13412@linux.vnet.ibm.com> References: <20170923205621.811805556@goodmis.org> <20170923205804.098759069@goodmis.org> <20170925000303.GJ3521@linux.vnet.ibm.com> <20170925002653.GL3521@linux.vnet.ibm.com> <20170925003456.GA13412@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Subject: Re: [PATCH 1/4] rcu: Allow for page faults in NMI handlers To: paulmck@linux.vnet.ibm.com, "Paul E. McKenney" , Linus Torvalds CC: Linux Kernel Mailing List , Ingo Molnar , Andrew Morton , stable From: Steven Rostedt Message-ID: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Sorry for the top post, currently on a train to Paris. This series already went through all my testing, and I would hate to rebase it for this reason. Can you just add a patch to remove the READ_ONCE()s? Thanks, -- Steve On September 25, 2017 2:34:56 AM GMT+02:00, "Paul E. McKenney" wrote: >On Sun, Sep 24, 2017 at 05:26:53PM -0700, Paul E. McKenney wrote: >> On Sun, Sep 24, 2017 at 05:12:13PM -0700, Linus Torvalds wrote: >> > On Sun, Sep 24, 2017 at 5:03 PM, Paul E. McKenney >> > wrote: >> > > >> > > Mostly just paranoia on my part. I would be happy to remove it >if >> > > you prefer. Or you or Steve can do so if that is more >convenient. >> > >> > I really don't think it's warranted. The values are *stable*. >There's >> > no subtle lack of locking, or some optimistic access to a value >that >> > can change. >> > >> > The compiler can generate code to read the value fifteen billion >> > times, and it will always get the same value. >> > >> > Yes, maybe in between the different accesses, an NMI will happen, >and >> > the value will be incremented, but then as the NMI exits, it will >> > decrement again, so the code that got interrupted will not actually >> > see the change. >> > >> > So the READ_ONCE() isn't "paranoia". It's just confusing. >> > >> > > And yes, consistency would dictate that the uses in >rcu_nmi_enter() >> > > and rcu_nmi_exit() should be _ONCE(), particularly the stores to >> > > ->dynticks_nmi_nesting. >> > >> > NO. >> > >> > That would be just more of that confusion. >> > >> > That value is STABLE. It's stable even within an NMI handler. The >NMI >> > code can read it, modify it, write it back, do a little dance, all >> > without having to care. There's no "_ONCE()" about it - not for the >> > readers, not for the writers, not for _anybody_. >> > >> > So adding even more READ/WRITE_ONCE() accesses wouldn't be >> > "consistent", it would just be insanity. >> > >> > Now, if an NMI happens and the value would be different on entry >than >> > it is on exit, that would be something else. Then it really >wouldn't >> > be stable wrt random users. But that would also be a major bug in >the >> > NMI handler, as far as I can tell. >> > >> > So the reason I'm objecting to that READ_ONCE() is that it isn't >> > "paranoia", it's "voodoo programming". And we don't do voodoo >> > programming. >> >> I already agreed that the READ_ONCE() can be removed. > >And for whatever it is worth, here is the updated patch. > > Thanx, Paul > >------------------------------------------------------------------------ > >commit 3e2baa988b9c13095995c46c51e0e32c0b6a7d43 >Author: Paul E. McKenney >Date: Fri Sep 22 13:14:42 2017 -0700 > > rcu: Allow for page faults in NMI handlers > > 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. > > Reported-by: Steven Rostedt > Signed-off-by: Paul E. McKenney > [ paulmck: Remove READ_ONCE() per Linux Torvalds feedback. ] > >diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c >index db5eb8c3f7af..e4fe06d42385 100644 >--- a/kernel/rcu/tree.c >+++ b/kernel/rcu/tree.c >@@ -891,6 +891,11 @@ void rcu_irq_exit(void) > > RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_exit() invoked with irqs >enabled!!!"); > rdtp = this_cpu_ptr(&rcu_dynticks); >+ >+ /* Page faults can happen in NMI handlers, so check... */ >+ if (rdtp->dynticks_nmi_nesting) >+ return; >+ > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && > rdtp->dynticks_nesting < 1); > if (rdtp->dynticks_nesting <= 1) { >@@ -1036,6 +1041,11 @@ void rcu_irq_enter(void) > > RCU_LOCKDEP_WARN(!irqs_disabled(), "rcu_irq_enter() invoked with irqs >enabled!!!"); > rdtp = this_cpu_ptr(&rcu_dynticks); >+ >+ /* Page faults can happen in NMI handlers, so check... */ >+ if (rdtp->dynticks_nmi_nesting) >+ return; >+ > oldval = rdtp->dynticks_nesting; > rdtp->dynticks_nesting++; > WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && -- Sent from my Android device with K-9 Mail. Please excuse my brevity.