Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 29 Aug 2019 21:20:36 -0400
From:   Joel Fernandes <joel@joelfernandes.org>
To:     "Paul E. McKenney" <paulmck@kernel.org>
Cc:     linux-kernel@vger.kernel.org,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Jonathan Corbet <corbet@lwn.net>,
        Josh Triplett <josh@joshtriplett.org>, kernel-team@android.com,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        linux-doc@vger.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
        rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [RFC v1 2/2] rcu/tree: Remove dynticks_nmi_nesting counter
Message-ID: <20190830012036.GA184995@google.com>
References: <20190828220108.GC26530@linux.ibm.com>
 <20190828221444.GA100789@google.com>
 <20190828231247.GE26530@linux.ibm.com>
 <20190829015155.GB100789@google.com>
 <20190829034336.GD4125@linux.ibm.com>
 <20190829144355.GE63638@google.com>
 <20190829151325.GF63638@google.com>
 <20190829161301.GQ4125@linux.ibm.com>
 <20190829171454.GA115245@google.com>
 <20190830004756.GW4125@linux.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190830004756.GW4125@linux.ibm.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Aug 29, 2019 at 05:47:56PM -0700, Paul E. McKenney wrote:
[snip]
> > > > Paul, also what what happens in the following scenario:
> > > > 
> > > > CPU0                                                 CPU1
> > > > 
> > > > A syscall causes rcu_eqs_exit()
> > > > rcu_read_lock();
> > > >                                                      ---> FQS loop waiting on
> > > > 						           dyntick_snap
> > > > usermode-upcall  entry -->causes rcu_eqs_enter();
> > > > 
> > > > usermode-upcall  exit  -->causes rcu_eqs_exit();
> > > > 
> > > >                                                      ---> FQS loop sees
> > > > 						          dyntick snap
> > > > 							  increment and
> > > > 							  declares CPU0 is
> > > > 							  in a QS state
> > > > 							  before the
> > > > 							  rcu_read_unlock!
> > > > 
> > > > rcu_read_unlock();
> > > > ---
> > > > 
> > > > Does the context tracking not call rcu_user_enter() in this case, or did I
> > > > really miss something?
> > > 
> > > Holding rcu_read_lock() across usermode execution (in this case,
> > > the usermode upcall) is a bad idea.  Why is CPU 0 doing that?
> > 
> > Oh, ok. I was just hypothesizing that since usermode upcalls from
> > something as heavy as interrupts, it could also mean we had the same from
> > some path that held an rcu_read_lock() as well. It was just a theoretical
> > concern, if it is not an issue, no problem.
> 
> Are there the usual lockdep checks in the upcall code?  Holding a spinlock
> across them would seem to be at least as bad as holding an rcu_read_lock()
> across them.

Great point, I'll take a look.

> > The other question I had was, in which cases would dyntick_nesting in current
> > RCU code be > 1 (after removing the lower bit and any crowbarring) ? In the
> > scenarios I worked out on paper, I can only see this as 1 or 0. But the
> > wording of it is 'dynticks_nesting'. May be I am missing a nesting scenario?
> > We can exit RCU-idleness into process context only once (either exiting idle
> > mode or user mode). Both cases would imply a value of 1.
> 
> Interrrupt -> NMI -> certain types of tracing.  I believe that can get
> it to 5.  There might be even more elaborate sequences of events.

I am only talking about dynticks_nesting, not dynticks_nmi_nesting. In
current mainline, I see this only 0 or 1. I am running the below patch
overnight on all RCU configurations to see if it is ever any other value.

And, please feel free to ignore my emails as you mentioned you are supposed
to be out next 2 days! Thanks for the replies though!

---8<-----------------------

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 68ebf0eb64c8..8c8ddb6457d5 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -571,6 +571,9 @@ static void rcu_eqs_enter(bool user)
 	WRITE_ONCE(rdp->dynticks_nmi_nesting, 0);
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) &&
 		     rdp->dynticks_nesting == 0);
+
+	WARN_ON_ONCE(rdp->dynticks_nesting != 1);
+
 	if (rdp->dynticks_nesting != 1) {
 		rdp->dynticks_nesting--;
 		return;
@@ -736,6 +739,9 @@ static void rcu_eqs_exit(bool user)
 	lockdep_assert_irqs_disabled();
 	rdp = this_cpu_ptr(&rcu_data);
 	oldval = rdp->dynticks_nesting;
+
+	WARN_ON_ONCE(rdp->dynticks_nesting != 0);
+
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && oldval < 0);
 	if (oldval) {
 		rdp->dynticks_nesting++;
-- 
2.23.0.187.g17f5b7556c-goog