Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   "Joel Fernandes (Google)" <joel@joelfernandes.org>
To:     linux-kernel@vger.kernel.org
Cc:     "Joel Fernandes (Google)" <joel@joelfernandes.org>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Jonathan Corbet <corbet@lwn.net>,
        Josh Triplett <josh@joshtriplett.org>,
        Lai Jiangshan <jiangshanlai@gmail.com>,
        linux-doc@vger.kernel.org,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Mauro Carvalho Chehab <mchehab+samsung@kernel.org>,
        neeraju@codeaurora.org, "Paul E. McKenney" <paulmck@kernel.org>,
        peterz@infradead.org, Randy Dunlap <rdunlap@infradead.org>,
        rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>,
        tglx@linutronix.de, vineethrp@gmail.com
Subject: [PATCH v4 3/5] rcu/tree: Make FQS complaining about offline CPU more aggressive
Date:   Fri,  7 Aug 2020 13:07:20 -0400
Message-Id: <20200807170722.2897328-4-joel@joelfernandes.org>
In-Reply-To: <20200807170722.2897328-1-joel@joelfernandes.org>
References: <20200807170722.2897328-1-joel@joelfernandes.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

The FQS loop detecting that an offline CPU has not yet reported a
quiescent state, is a serious problem. The current interaction between
RCU quiescent-state reporting and CPU-hotplug operations means that the
FQS loop should never find that an offline CPU (by RCU's understanding
of what is offline or not) has not yet reported a quiescent state.

First, the outgoing CPU explicitly reports a quiescent state if needed
in rcu_report_dead().  Second, the race where the CPU is leaving just as
RCU is initializing a new grace period is handled by an explicit check
for this condition in rcu_gp_init().  Third, the CPU's leaf rcu_node
structure's ->lock serializes these modifications and checks.

At the moment, the FQS loop detections this anomaly but only reports it
after a second has elapsed.

This commit therefore makes the warning more aggressive such that we fail
immediately when the FQS loop scan happens and bring the problem to
everyone's attention.

Light testing with TREE03 and hotplug shows no warnings. Converted the
warning as well to WARN_ON_ONCE() to reduce log spam.

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 kernel/rcu/tree.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 2fb66cdbfa25..850628aba813 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1208,13 +1208,27 @@ static int rcu_implicit_dynticks_qs(struct rcu_data *rdp)
 		return 1;
 	}
 
-	/* If waiting too long on an offline CPU, complain. */
-	if (!(rdp->grpmask & rcu_rnp_online_cpus(rnp)) &&
-	    time_after(jiffies, rcu_state.gp_start + HZ)) {
+	/*
+	 * Complain if an offline CPU by RCU's hotplug bookkeeping has not
+	 * reported a quiescent state yet.
+	 *
+	 * Either a quiescent state for the offline CPU should have been
+	 * reported during CPU offlining, or during rcu_gp_init() if it
+	 * detected a race with either CPU offlining or task unblocking on a
+	 * node with all of its CPUs previously offlined. If neither of these
+	 * happened, we have a problem.
+	 *
+	 * The node lock is held here, ensuring exclusion from the
+	 * modification/checking of RCU's hotplug books by RCU's hotplug
+	 * notifier hooks and grace period initialization code.
+	 *
+	 * Refer to RCU's Requirements documentation about hotplug requirements
+	 * as well.
+	 */
+	if (WARN_ON_ONCE(!(rdp->grpmask & rcu_rnp_online_cpus(rnp)))) {
 		bool onl;
 		struct rcu_node *rnp1;
 
-		WARN_ON(1);  /* Offline CPUs are supposed to report QS! */
 		pr_info("%s: grp: %d-%d level: %d ->gp_seq %ld ->completedqs %ld\n",
 			__func__, rnp->grplo, rnp->grphi, rnp->level,
 			(long)rnp->gp_seq, (long)rnp->completedqs);
-- 
2.28.0.236.gb10cc79966-goog