Linux RCU subsystem development
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: rcu@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, mingo@kernel.org,
	jiangshanlai@gmail.com, dipankar@in.ibm.com,
	akpm@linux-foundation.org, mathieu.desnoyers@efficios.com,
	josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org,
	rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com,
	fweisbec@gmail.com, oleg@redhat.com, joel@joelfernandes.org,
	byungchul.park@lge.com, kernel-team@android.com,
	"Paul E . McKenney" <paulmck@linux.ibm.com>
Subject: [PATCH tip/core/rcu 10/12] rcu: Add checks for dynticks counters in rcu_is_cpu_rrupt_from_idle()
Date: Thu, 30 May 2019 07:52:27 -0700	[thread overview]
Message-ID: <20190530145229.29565-10-paulmck@linux.ibm.com> (raw)
In-Reply-To: <20190530145204.GA28526@linux.ibm.com>

From: "Joel Fernandes (Google)" <joel@joelfernandes.org>

It would be good to combine the dynticks and dynticks_nesting counters
in order to simplify the code.  Unfortunately, there are concerns
about usermode upcalls appearing to RCU as half of an interrupt, as
Byungchul learned [1].  The "half" in "half interrupt" is due to an
unpaired rcu_irq_enter(): Normally, each rcu_irq_enter() has a later
call to rcu_irq_exit().

Out of an abundance of caution, Paul added warnings [2] in the RCU
code which if not fired by 2021 will be interpreted as meaning that
this half-interrupt scenario cannot happen any more, thus permitting
simplification of this code.

In the meantime, this commit makes the following changes:

(1) Combining these two counters requires that rcu_rrupt_from_idle()
    is invoked only from hard-interrupt contexts as discussed here [3].
    This commit therefore adds the required lockdep_assert_in_irq()
    to check this constraint.

(2) Furthermore, rcu_rrupt_from_idle() is not explicit about how it
    is using the counters which can lead to weird future bugs. This
    commit therefore adds comments indicating the meaning and use of
    each counter.

(3) Lastly, this commit checks for counter underflows as another check
    that half interrupts don't occur.  (Previously, the function would
    simply return true upon underflow.)

All these checks checks are NOOPs if PROVE_LOCKING (and thus PROVE_RCU)
are disabled.

[1] https://lore.kernel.org/patchwork/patch/952349/
[2] Commit e11ec65cc8d6 ("rcu: Add warning to detect half-interrupts")
[3] https://lore.kernel.org/lkml/20190312150514.GB249405@google.com/

Cc: byungchul.park@lge.com
Cc: kernel-team@android.com
Cc: rcu@vger.kernel.org
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
---
 kernel/rcu/tree.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 7822a2e1370d..b9629cf08f94 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -376,16 +376,29 @@ static void __maybe_unused rcu_momentary_dyntick_idle(void)
 }
 
 /**
- * rcu_is_cpu_rrupt_from_idle - see if idle or immediately interrupted from idle
+ * rcu_is_cpu_rrupt_from_idle - see if interrupted from idle
  *
- * If the current CPU is idle or running at a first-level (not nested)
+ * If the current CPU is idle and running at a first-level (not nested)
  * interrupt from idle, return true.  The caller must have at least
  * disabled preemption.
  */
 static int rcu_is_cpu_rrupt_from_idle(void)
 {
-	return __this_cpu_read(rcu_data.dynticks_nesting) <= 0 &&
-	       __this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 1;
+	/* Called only from within the scheduling-clock interrupt */
+	lockdep_assert_in_irq();
+
+	/* Check for counter underflows */
+	RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nesting) < 0,
+			 "RCU dynticks_nesting counter underflow!");
+	RCU_LOCKDEP_WARN(__this_cpu_read(rcu_data.dynticks_nmi_nesting) <= 0,
+			 "RCU dynticks_nmi_nesting counter underflow/zero!");
+
+	/* Are we at first interrupt nesting level? */
+	if (__this_cpu_read(rcu_data.dynticks_nmi_nesting) != 1)
+		return false;
+
+	/* Does CPU appear to be idle from an RCU standpoint? */
+	return __this_cpu_read(rcu_data.dynticks_nesting) == 0;
 }
 
 #define DEFAULT_RCU_BLIMIT 10     /* Maximum callbacks per rcu_do_batch. */
-- 
2.17.1


  parent reply	other threads:[~2019-05-30 14:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-30 14:52 [PATCH tip/core/rcu 0/12] Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 01/12] rcu: Enable elimination of Tree-RCU softirq processing Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 02/12] rcu: Check for wakeup-safe conditions in rcu_read_unlock_special() Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 03/12] rcu: Only do rcu_read_unlock_special() wakeups if expedited Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 04/12] rcu: Allow rcu_read_unlock_special() to raise_softirq() if in_irq() Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 05/12] rcu: Use irq_work to get scheduler's attention in clean context Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 06/12] rcu: Inline invoke_rcu_callbacks() into its sole remaining caller Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 07/12] rcu: Avoid self-IPI in sync_rcu_exp_select_node_cpus() Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 08/12] rcu: Avoid self-IPI in sync_sched_exp_online_cleanup() Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 09/12] lockdep: Add assertion to check if in an interrupt Paul E. McKenney
2019-05-30 14:52 ` Paul E. McKenney [this message]
2019-05-30 14:52 ` [PATCH tip/core/rcu 11/12] rcu: Rename rcu_data's ->deferred_qs to ->exp_deferred_qs Paul E. McKenney
2019-05-30 14:52 ` [PATCH tip/core/rcu 12/12] rcu: Remove unused rdp local from synchronize_rcu_expedited() Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190530145229.29565-10-paulmck@linux.ibm.com \
    --to=paulmck@linux.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=byungchul.park@lge.com \
    --cc=dhowells@redhat.com \
    --cc=dipankar@in.ibm.com \
    --cc=edumazet@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox