Date: Sun, 24 Sep 2017 17:03:03 -0700
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        stable <stable@vger.kernel.org>
Subject: Re: [PATCH 1/4] rcu: Allow for page faults in NMI handlers
Reply-To: paulmck@linux.vnet.ibm.com
References: <20170923205621.811805556@goodmis.org>
 <20170923205804.098759069@goodmis.org>
 <CA+55aFyHXW5smbkWdpz-v3n6=9u1+m68YH4rB8Kb99AWaz50Kw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CA+55aFyHXW5smbkWdpz-v3n6=9u1+m68YH4rB8Kb99AWaz50Kw@mail.gmail.com>
Message-Id: <20170925000303.GJ3521@linux.vnet.ibm.com>
Sender: stable-owner@vger.kernel.org

On Sun, Sep 24, 2017 at 12:42:32PM -0700, Linus Torvalds wrote:
> On Sat, Sep 23, 2017 at 1:56 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > +
> > +       /* Page faults can happen in NMI handlers, so check... */
> > +       if (READ_ONCE(rdtp->dynticks_nmi_nesting))
> > +               return;
> > +
> 
> What is the reason for the READ_ONCE() here (and in the other case)?
> 
> It doesn't seem to have any actual reason.  It's a "stable" per-cpu
> value in that even if an NMI were to happen, it gets incremented and
> then decremented, so there is nothing really volatile about it
> anywhere that I can see.
> 
> So the READ_ONCE() seems to be just pure confusion.
> 
> What am I missing?

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.

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.  But I am not too worried about that right now
because I suspect that I should be able to combine rcu_irq_{enter,exit}()
and rcu_nmi_{enter,exit}(), which would be a good simplification.

							Thanx, aul