[PATCH v2 0/7] srcu: NMI-safe atomic SRCU readers, and their irqsave forms

[PATCH v2 0/7] srcu: NMI-safe atomic SRCU readers, and their irqsave forms

[Mathieu Desnoyers]

Hi Paul,

This series adapts atomic SRCU to the needs of RCU pseudo-transactions
[1], as we discussed [2].  Their readers resolve a proxy within a short
section, in any context, NMI included.  A writer waits for these
sections without sleeping, from task, softirq or hardirq context.
Atomic SRCU fits, with one fix, one rule made explicit and two
additions:

- Patch 1 fixes Tiny synchronize_srcu_atomic().  With
  CONFIG_PREEMPTION=y, it called synchronize_rcu(), which ends an RCU
  grace period on Tiny RCU.  That is wrong when the caller is in an
  interrupt handler or within rcu_read_lock().
- Patch 2 is about calling synchronize_srcu_atomic() from an interrupt
  handler.  All calls on that srcu_struct, task context included, must
  then disable interrupts around it.  Otherwise, the interrupt handler
  may spin forever on the CPU of a waiter it interrupted.  Patch 2
  documents this rule, and warns when it is broken.
- Patch 3 makes the atomic readers NMI-safe.  They use the counter
  increments of __srcu_read_lock_nmisafe(), which keep the full barriers
  that synchronize_srcu_atomic() pairs with.
- Patch 4 adds srcu_read_lock_atomic_irqsave() and
  srcu_read_unlock_atomic_irqrestore().  They disable interrupts instead
  of preemption, for a domain whose writers wait with interrupts
  disabled.
- Patch 5 makes rcutorture run readers from NMI, for the NMI-safe reader
  flavors.
- Patch 6 makes rcutorture alternate the atomic readers of each CPU with
  irqsave readers, with a new atomic_irqsave parameter.  It also adds an
  SRCU-P run of this mode to torture.sh.
- Patch 7 makes rcutorture check what patch 1 fixes.

Patch 2 does not make synchronize_srcu_atomic() disable interrupts
itself.  A reader that disables only preemption may wait for an IPI to
the waiter's CPU, and that IPI would never run.  Only the caller knows
that its readers disable interrupts.

On architectures without NMI-safe this_cpu_inc(), every atomic reader
now pays an atomic increment at lock and at unlock.  The update side is
unchanged.  On architectures with NMI-safe this_cpu_inc(), such as x86,
the readers use the same counter increments as before.  David agreed to
the change, as long as synchronize_srcu_atomic() stays within the 4-16us
it was designed for [3].

Changes since v1 [4]
--------------------

- Rebased on v7.4-rc2.  Patch 5 (v1 patch 3) no longer applied,
  because of the kzalloc_objs() conversion in rcutorture.
- New patches 1 and 7, the Tiny SRCU fix above and its test.  Paul,
  please check that nothing needs an atomic SRCU grace period to imply
  an RCU grace period.  Tree SRCU never provided that.
- New patch 2, the rule above.  A prototype KUnit test of RCU
  pseudo-transactions hit the hang: it waits from a task, and from an
  IPI on the same CPU.  RCU pseudo-transactions now disable interrupts
  around their wait.
- Patch 5 (v1 patch 3), after your review: the readers in NMI count
  their pipe and batch values in atomic arrays of their own.  The
  statistics print them as "NMI Reader Pipe:" and "NMI Reader Batch:".
  A too-short grace period seen in NMI now gets "!!!" and a warning.
- Patch 6 (v1 patch 4), after your review:
  - the atomic_irqsave description says that the readers alternate per
    CPU, which also holds for a single reader;
  - SRCU_TORTURE_IRQSAVE is 0x80, defined next to RCUTORTURE_RDR_MASK_1
    and _2;
  - the irqsave readers run only when ->extendables is zero, instead of
    clearing extendables.

Testing
-------

Testing was on x86_64 under QEMU/KVM, with
CONFIG_FORCE_NEED_SRCU_NMI_SAFE=y:

- SRCU-P with atomic readers alternating with irqsave readers, for 5
  minutes: no error.  It ran 40876 grace periods and 94824 readers in
  NMI.  The same without irqsave readers, for 2 minutes: no error.
- SRCU-P with the NMI-safe reader flavor, then with the fast one, for 2
  minutes each: no error.  Both ran readers in NMI.
- SRCU-T (Tiny SRCU) with atomic and irqsave readers, for 2 minutes: no
  error.
- The srcu_lockdep.sh cases of synchronize_srcu_atomic() give their
  expected results.  A wait within rcu_read_lock() (61 to 63) gets no
  lockdep warning.  A reader that ran with interrupts enabled, then a
  wait from an IPI (1071 to 1073), still gets
  "inconsistent {HARDIRQ-ON-R}".
- A throwaway KUnit test waits from an IPI on the CPU of a waiter in
  task context.  The new check warns, then the wait hangs, as the rule
  says it must.  With every caller disabling interrupts, the same test
  passes.

Each new check fails when what it checks is broken:

- A reader's counter update is split in two, with an NMI reader in
  between: the writer stalls in its grace period.
- The flavor check rejects atomic readers in NMI: it warns.
- An irqsave unlock restores interrupts while another irqsave reader of
  its CPU is held: the new check in rcutorture warns.
- Every reader in NMI sees a pipe count of 2: rcutorture prints "!!!"
  and warns of a too-short grace period in NMI.
- Patch 1 is reverted, on SRCU-T: the check of patch 7 warns.  lockdep
  also reports "Illegal synchronize_rcu() in RCU read-side critical
  section".

The readers in NMI need a PMU in the guest.  kvm.sh starts x86 guests
with -cpu kvm64, which has none.  So these runs added --qemu-args
"-cpu host,pmu=on".  Without it, rcutorture reports that NMIs went
untested.  This also holds for the existing test of ->call() from NMI.

This series was developed with assistance from an LLM.  I reviewed the
code and the tests.

The series is based on v7.4-rc2.

[1] https://lore.kernel.org/all/20261016133319.433617-1-mathieu.desnoyers@efficios.com/
[2] https://lore.kernel.org/all/853d8469-94a4-4b33-a0d5-d6d2138fe3ef@paulmck-laptop/
[3] https://lore.kernel.org/all/84fdca577207092bd16fc8f3949c9b7a776750ba.camel@infradead.org/
[4] https://lore.kernel.org/all/20261110192844.44713-1-mathieu.desnoyers@efficios.com/

Thanks,

Mathieu

Mathieu Desnoyers (7):
  srcutiny: Do not call synchronize_rcu() from synchronize_srcu_atomic()
  srcu: Rule for using synchronize_srcu_atomic() from interrupt handlers
  srcu: Make atomic SRCU readers NMI-safe
  srcu: Add irqsave variants of the atomic SRCU readers
  rcutorture: Run NMI-safe SRCU readers from NMI
  rcutorture: Test the irqsave forms of atomic SRCU readers
  rcutorture: Check an atomic SRCU wait within an RCU reader

 include/linux/srcu.h                          | 108 +++++++---
 kernel/rcu/rcutorture.c                       | 197 +++++++++++++++++-
 kernel/rcu/srcutiny.c                         |   7 +-
 kernel/rcu/srcutree.c                         |  18 +-
 .../selftests/rcutorture/bin/torture.sh       |   6 +
 5 files changed, 296 insertions(+), 40 deletions(-)


base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.47.3

[PATCH v2 1/7] srcutiny: Do not call synchronize_rcu() from synchronize_srcu_atomic()

[Mathieu Desnoyers]

synchronize_srcu_atomic() never blocks, so that it can be called from
atomic context.  This includes interrupt handlers, and code within
rcu_read_lock().

With CONFIG_PREEMPTION=y, the Tiny SRCU version first calls
synchronize_rcu().  Tiny synchronize_rcu() does not wait: it only
advances rcu_ctrlblk.gp_seq.  This is valid where the caller is in a
quiescent state, in task context and outside of any RCU reader.

The callers of synchronize_srcu_atomic() need not be in a quiescent
state.  An interrupt handler may have interrupted an RCU reader.  A
caller within rcu_read_lock() is itself an RCU reader.  In both cases,
synchronize_rcu() ends an RCU grace period while an RCU reader is still
in its critical section.  poll_state_synchronize_rcu() would then
report a grace period that did not happen.  The caller within
rcu_read_lock() also gets "Illegal synchronize_rcu() in RCU read-side
critical section" from lockdep.

srcu_drive_gp() has the same call.  There, it makes an RCU Tasks
Trace grace period imply an RCU grace period.  RCU Tasks Trace uses
call_srcu() and synchronize_srcu(), which are both forbidden on a
srcu_struct that uses synchronize_srcu_atomic().  And the Tree SRCU
version of synchronize_srcu_atomic() does not wait for an RCU grace
period either.

Fix this by removing the call.

This was found by reading the code.  An rcutorture test added later in
this series fails without this change.

Fixes: a48669f0c49c ("srcutiny: Add an atomic Tiny SRCU")
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/rcu/srcutiny.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c
index 63a3f0a8bc53..d58f4160f925 100644
--- a/kernel/rcu/srcutiny.c
+++ b/kernel/rcu/srcutiny.c
@@ -367,9 +367,10 @@ void synchronize_srcu_atomic(struct srcu_struct *ssp)
 	if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE)
 		return;
 
-	if (IS_ENABLED(CONFIG_PREEMPTION))
-		synchronize_rcu(); // Needed for RCU Tasks Trace to imply RCU grace period.
-				   // And in Tiny RCU, it is near zero cost and doesn't block.
+	// No synchronize_rcu() here, unlike in srcu_drive_gp().  The caller
+	// may be in an interrupt handler or within rcu_read_lock().  Tiny
+	// synchronize_rcu() would then end an RCU grace period that an RCU
+	// reader still holds up.
 
 	// Usually, there will be no readers.
 	preempt_disable();  // Guard against lazy preemption and some other grace period.
-- 
2.47.3

[PATCH v2 2/7] srcu: Rule for using synchronize_srcu_atomic() from interrupt handlers

[Mathieu Desnoyers]

This is about the update side.  A user that calls
synchronize_srcu_atomic() from an interrupt handler must disable
interrupts around all its calls on that srcu_struct, task context
included.  Document this rule, and warn when it is broken.  The rules
for readers are unchanged.

synchronize_srcu_atomic() spins rather than sleeps, so it may be called
from atomic context, interrupt handlers included.  Its callers on one
srcu_struct exclude each other with ->srcu_atomic_gp_flag.  One of them
drives the grace period.  The others spin until a grace period ends.

The flag owner disables only preemption.  So an interrupt can arrive on
its CPU while it holds the flag.  If that interrupt handler calls
synchronize_srcu_atomic() on the same srcu_struct, it spins forever:

  CPU 0, task                        CPU 0, hardirq
  synchronize_srcu_atomic()
    takes ->srcu_atomic_gp_flag
                                     synchronize_srcu_atomic()
                                       spins until a grace period ends
                                       it never ends: its driver is the
                                       task this interrupt interrupted

Nothing reports it.  lockdep cannot, because lock_sync() records no
interrupt state.

synchronize_srcu_atomic() cannot disable interrupts itself.  A reader
that disables only preemption may wait for an IPI to the waiter's CPU.
That IPI never runs while the waiter spins with interrupts disabled.
The kernel-doc already lets the caller choose the waiter's context.
It then asks the readers to run in that context, or a stricter one.

So the caller disables interrupts.  The existing reader rule then
requires the readers of that srcu_struct to disable interrupts too.

Also store the owner's CPU number plus one in the flag, instead of 1.
A waiter that finds its own CPU as the owner warns once, rather than
hanging silently.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/rcu/srcutree.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 2b0e2b2d2c18..8075da4b1bb5 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -2108,6 +2108,13 @@ static void srcu_advance_state(struct srcu_struct *ssp, bool is_atomic)
  * and srcu_read_unlock_atomic() must be invoked from that same context,
  * or one that is even more strict.
  *
+ * If synchronize_srcu_atomic() is invoked from an interrupt handler for
+ * a given srcu_struct structure, then for that structure, all calls to
+ * synchronize_srcu_atomic() must be invoked with interrupts disabled,
+ * including those from task context.  Its callers take turns to drive a
+ * grace period, and the others spin until it ends.  An interrupt handler
+ * that calls it on the CPU of an interrupted caller would spin forever.
+ *
  * If synchronize_srcu_atomic() is invoked on a given srcu_struct
  * structure, then none of call_srcu(), synchronize_srcu(),
  * synchronize_srcu_expedited(), start_poll_synchronize_srcu(),
@@ -2144,7 +2151,13 @@ void synchronize_srcu_atomic(struct srcu_struct *ssp)
 	srcu_state = get_state_synchronize_srcu(ssp);
 	preempt_disable();
 	while (atomic_read(&sup->srcu_atomic_gp_flag) ||
-	       atomic_xchg(&sup->srcu_atomic_gp_flag, 1)) {
+	       atomic_cmpxchg(&sup->srcu_atomic_gp_flag, 0,
+			      smp_processor_id() + 1)) {
+		// The flag holds its owner's CPU number plus one.  An owner on
+		// this CPU is a caller that this one interrupted.  It cannot
+		// release the flag, so this caller would spin forever.
+		WARN_ON_ONCE(atomic_read(&sup->srcu_atomic_gp_flag) ==
+			     smp_processor_id() + 1);
 		preempt_enable();
 		if (poll_state_synchronize_srcu(ssp, srcu_state))
 			return;
-- 
2.47.3

[PATCH v2 3/7] srcu: Make atomic SRCU readers NMI-safe

[Mathieu Desnoyers]

Atomic SRCU readers count themselves with __srcu_read_lock(), whose
this_cpu_inc() is not NMI-safe on every architecture. With
CONFIG_PROVE_RCU, which lockdep enables, __srcu_check_read_flavor() also
warns when an atomic reader runs in NMI. So an NMI handler cannot use
atomic SRCU, while a tracer or a BPF program may need to read a
structure that atomic SRCU protects.

Count atomic readers with __srcu_read_lock_nmisafe() and
__srcu_read_unlock_nmisafe() instead. They use NMI-safe increments on
the architectures that need them, and keep the full barriers that
synchronize_srcu_atomic() pairs with. The __srcu_read_lock_fast()
helpers have no such barriers: they rely on a grace period that calls
synchronize_rcu(), which synchronize_srcu_atomic() cannot do while it
spins.

Also accept the atomic flavor in NMI in __srcu_check_read_flavor().
Annotate an atomic reader in NMI as a trylock for lockdep, as
srcu_read_lock_nmisafe() does. An NMI can interrupt lockdep itself, and
lockdep then reports any other acquisition of a lock that a grace
period has used.

On the architectures that need NMI-safe counters, every atomic reader
now pays an atomic increment at lock and at unlock.

Suggested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 include/linux/srcu.h  | 18 ++++++++++++++----
 kernel/rcu/srcutree.c |  3 ++-
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index d4dcaeb83e2b..e3b541ceb8dd 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -93,7 +93,7 @@ int init_srcu_struct_atomic(struct srcu_struct *ssp);
 //					0x04		// SRCU-lite is no longer with us.
 #define SRCU_READ_FLAVOR_FAST		0x04		// srcu_read_lock_fast(), also NMI-safe.
 #define SRCU_READ_FLAVOR_FAST_UPDOWN	0x08		// srcu_read_lock_fast_updown().
-#define SRCU_READ_FLAVOR_ATOMIC		0x10		// srcu_read_lock_atomic().
+#define SRCU_READ_FLAVOR_ATOMIC		0x10		// srcu_read_lock_atomic(), also NMI-safe.
 #define SRCU_READ_FLAVOR_ALL		(SRCU_READ_FLAVOR_NORMAL | SRCU_READ_FLAVOR_NMI | \
 					 SRCU_READ_FLAVOR_FAST | SRCU_READ_FLAVOR_FAST_UPDOWN | \
 					 SRCU_READ_FLAVOR_ATOMIC)
@@ -372,6 +372,9 @@ static inline int srcu_read_lock(struct srcu_struct *ssp)
  * The lock and matching srcu_read_unlock_atomic() must be invoked on
  * the same CPU, from the same context; passing the return value to
  * another task is not permitted for this flavor.
+ *
+ * This function is NMI-safe, in a manner similar to
+ * srcu_read_lock_nmisafe().
  */
 static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
 	__acquires_shared(ssp)
@@ -391,8 +394,15 @@ static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
 	if (in_task())
 		non_block_start();
 	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_ATOMIC);
-	retval = __srcu_read_lock(ssp);
-	srcu_lock_acquire(&ssp->dep_map);
+	retval = __srcu_read_lock_nmisafe(ssp);
+	/*
+	 * An NMI may interrupt lockdep itself.  Annotate it as a trylock
+	 * there, as srcu_read_lock_nmisafe() does.
+	 */
+	if (in_nmi())
+		rcu_try_lock_acquire(&ssp->dep_map);
+	else
+		srcu_lock_acquire(&ssp->dep_map);
 	return retval;
 }
 
@@ -599,7 +609,7 @@ static inline void srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
 	WARN_ON_ONCE(idx & ~0x1);
 	srcu_check_read_flavor(ssp, SRCU_READ_FLAVOR_ATOMIC);
 	srcu_lock_release(&ssp->dep_map);
-	__srcu_read_unlock(ssp, idx);
+	__srcu_read_unlock_nmisafe(ssp, idx);
 	if (in_task())
 		non_block_end();
 	preempt_enable();
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 8075da4b1bb5..8fa366861dcc 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -828,7 +828,8 @@ void __srcu_check_read_flavor(struct srcu_struct *ssp, int read_flavor)
 
 	/* NMI-unsafe use in NMI is a bad sign, as is multi-bit read_flavor values. */
 	WARN_ON_ONCE(read_flavor != SRCU_READ_FLAVOR_NMI &&
-		     read_flavor != SRCU_READ_FLAVOR_FAST && in_nmi());
+		     read_flavor != SRCU_READ_FLAVOR_FAST &&
+		     read_flavor != SRCU_READ_FLAVOR_ATOMIC && in_nmi());
 	WARN_ON_ONCE(read_flavor & (read_flavor - 1));
 
 	sdp = raw_cpu_ptr(ssp->sda);
-- 
2.47.3

[PATCH v2 4/7] srcu: Add irqsave variants of the atomic SRCU readers

[Mathieu Desnoyers]

synchronize_srcu_atomic() may be invoked with interrupts disabled, for
example under a raw spinlock. Its kerneldoc then requires every reader
of that srcu_struct to disable interrupts too. srcu_read_lock_atomic()
only disables preemption, so such a reader has to disable interrupts
itself around it.

Add srcu_read_lock_atomic_irqsave() and
srcu_read_unlock_atomic_irqrestore(). They disable interrupts instead
of preemption, and keep the interrupt state in a flags argument, so
they are macros. Both forms share the rest of the reader through
__srcu_read_lock_atomic() and __srcu_read_unlock_atomic().

Suggested-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 include/linux/srcu.h | 100 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 25 deletions(-)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index e3b541ceb8dd..11e9ad914a71 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -357,31 +357,15 @@ static inline int srcu_read_lock(struct srcu_struct *ssp)
 	return retval;
 }
 
-/**
- * srcu_read_lock_atomic - register a new reader promising an atomic section
- * @ssp: srcu_struct in which to register the new reader.
- *
- * As srcu_read_lock(), but the caller promises that the read-side
- * critical section never sleeps and never blocks on anything which
- * may itself depend on memory allocation to make progress. Preemption
- * is disabled for the duration, which both enforces that promise (any
- * sleepable call in the section will splat on every configuration)
- * and bounds the section so that the update side may spin rather
- * than sleep when waiting for readers: see synchronize_srcu_atomic().
- *
- * The lock and matching srcu_read_unlock_atomic() must be invoked on
- * the same CPU, from the same context; passing the return value to
- * another task is not permitted for this flavor.
- *
- * This function is NMI-safe, in a manner similar to
- * srcu_read_lock_nmisafe().
+/*
+ * Register an atomic-section reader.  The caller has disabled preemption,
+ * or interrupts.
  */
-static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
+static inline int __srcu_read_lock_atomic(struct srcu_struct *ssp)
 	__acquires_shared(ssp)
 {
 	int retval;
 
-	preempt_disable();
 	/*
 	 * Arm might_sleep() to catch even a *potentially* sleeping call
 	 * in the section, not just an actual schedule: the atomic-domain
@@ -406,6 +390,50 @@ static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
 	return retval;
 }
 
+/**
+ * srcu_read_lock_atomic - register a new reader promising an atomic section
+ * @ssp: srcu_struct in which to register the new reader.
+ *
+ * As srcu_read_lock(), but the caller promises that the read-side
+ * critical section never sleeps and never blocks on anything which
+ * may itself depend on memory allocation to make progress. Preemption
+ * is disabled for the duration, which both enforces that promise (any
+ * sleepable call in the section will splat on every configuration)
+ * and bounds the section so that the update side may spin rather
+ * than sleep when waiting for readers: see synchronize_srcu_atomic().
+ *
+ * The lock and matching srcu_read_unlock_atomic() must be invoked on
+ * the same CPU, from the same context; passing the return value to
+ * another task is not permitted for this flavor.
+ *
+ * This function is NMI-safe, in a manner similar to
+ * srcu_read_lock_nmisafe().
+ */
+static inline int srcu_read_lock_atomic(struct srcu_struct *ssp)
+	__acquires_shared(ssp)
+{
+	preempt_disable();
+	return __srcu_read_lock_atomic(ssp);
+}
+
+/**
+ * srcu_read_lock_atomic_irqsave - register an atomic reader, interrupts off
+ * @ssp: srcu_struct in which to register the new reader.
+ * @flags: interrupt state to restore at srcu_read_unlock_atomic_irqrestore().
+ *
+ * As srcu_read_lock_atomic(), but with interrupts disabled for the
+ * duration instead of preemption.  Use it when synchronize_srcu_atomic()
+ * may be invoked with interrupts disabled for this srcu_struct: its
+ * readers must then disable interrupts too.
+ *
+ * Returns the index to pass to srcu_read_unlock_atomic_irqrestore().
+ */
+#define srcu_read_lock_atomic_irqsave(ssp, flags)			\
+({									\
+	local_irq_save(flags);						\
+	__srcu_read_lock_atomic((ssp));					\
+})
+
 /**
  * srcu_read_lock_fast - register a new reader for an SRCU-protected structure.
  * @ssp: srcu_struct in which to register the new reader.
@@ -598,12 +626,11 @@ static inline void srcu_read_unlock(struct srcu_struct *ssp, int idx)
 	__srcu_read_unlock(ssp, idx);
 }
 
-/**
- * srcu_read_unlock_atomic - unregister an atomic-section reader
- * @ssp: srcu_struct from which to unregister the old reader.
- * @idx: return value from corresponding srcu_read_lock_atomic().
+/*
+ * Unregister an atomic-section reader.  The caller then enables preemption,
+ * or interrupts, again.
  */
-static inline void srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
+static inline void __srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
 	__releases_shared(ssp)
 {
 	WARN_ON_ONCE(idx & ~0x1);
@@ -612,9 +639,32 @@ static inline void srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
 	__srcu_read_unlock_nmisafe(ssp, idx);
 	if (in_task())
 		non_block_end();
+}
+
+/**
+ * srcu_read_unlock_atomic - unregister an atomic-section reader
+ * @ssp: srcu_struct from which to unregister the old reader.
+ * @idx: return value from corresponding srcu_read_lock_atomic().
+ */
+static inline void srcu_read_unlock_atomic(struct srcu_struct *ssp, int idx)
+	__releases_shared(ssp)
+{
+	__srcu_read_unlock_atomic(ssp, idx);
 	preempt_enable();
 }
 
+/**
+ * srcu_read_unlock_atomic_irqrestore - unregister an irqsave atomic reader
+ * @ssp: srcu_struct from which to unregister the old reader.
+ * @idx: return value from corresponding srcu_read_lock_atomic_irqsave().
+ * @flags: interrupt state saved by srcu_read_lock_atomic_irqsave().
+ */
+#define srcu_read_unlock_atomic_irqrestore(ssp, idx, flags)		\
+do {									\
+	__srcu_read_unlock_atomic((ssp), (idx));			\
+	local_irq_restore(flags);					\
+} while (0)
+
 /**
  * srcu_read_unlock_fast - unregister a old reader from an SRCU-protected structure.
  * @ssp: srcu_struct in which to unregister the old reader.
-- 
2.47.3

[PATCH v2 5/7] rcutorture: Run NMI-safe SRCU readers from NMI

[Mathieu Desnoyers]

rcutorture exercises ->call() from NMI, but never runs a reader there.
So nothing tests the NMI-safe reader flavors where it matters most: in
an NMI that interrupts another reader during its counter update.

Run a reader from the same per-CPU perf-counter NMI, when every reader
flavor in use is NMI-safe. These flavors are srcu_read_lock_nmisafe(),
srcu_read_lock_fast() and srcu_read_lock_atomic(). A new ops hook,
->nmi_reader_capable(), reports whether the reader flavors in use are
all NMI-safe. A new nmi_readers module parameter, on by default, enables
these NMI readers. The statistics count these readers as nmi-reads.

The reader loads rcu_torture_current and checks its element as the
other readers do. The element must not be freed before the reader ends.
It must not go through two grace periods either.

The other readers count their pipe and batch values with this_cpu_inc().
On some architectures, an NMI may interrupt it. So the NMI reader counts
its values in arrays of atomic counters of its own. The statistics print
them as "NMI Reader Pipe:" and "NMI Reader Batch:". A too-short grace
period seen in NMI gets the same "!!!" and the same warning as for the
other readers.

The NMI may interrupt lockdep, which then does not record the reader.
So the reader loads the pointer with rcu_dereference_raw().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/rcu/rcutorture.c | 96 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 90 insertions(+), 6 deletions(-)

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 93ca7f06371e..9889293c968b 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -117,6 +117,7 @@ torture_param(int, n_barrier_cbs, 0, "# of callbacks/kthreads for barrier testin
 torture_param(int, n_up_down, 32, "# of concurrent up/down hrtimer-based RCU readers");
 torture_param(int, nfakewriters, 4, "Number of RCU fake writer threads");
 torture_param(bool, nmi_calls, true, "Exercise ->call() from NMI on nmi_capable flavors");
+torture_param(bool, nmi_readers, true, "Run readers from NMI with NMI-safe reader flavors");
 torture_param(int, nreaders, -1, "Number of RCU reader threads");
 torture_param(bool, nwriters, 1, "Number of RCU writer threads (0 or 1)");
 torture_param(int, object_debug, 0, "Enable debug-object double call_rcu() testing");
@@ -220,6 +221,11 @@ static atomic_long_t n_rcu_torture_timers;
 static atomic_long_t n_rcu_torture_irqs;
 static atomic_long_t n_rcu_torture_nmi_call;
 static atomic_long_t n_rcu_torture_nmi_cb;
+static atomic_long_t n_rcu_torture_nmi_read;
+// Pipe and batch counts of the readers in NMI, as rcu_torture_count and
+// rcu_torture_batch for the others.  An NMI may interrupt this_cpu_inc().
+static atomic_long_t rcu_torture_nmi_pipe[RCU_TORTURE_PIPE_LEN + 1];
+static atomic_long_t rcu_torture_nmi_batch[RCU_TORTURE_PIPE_LEN + 1];
 static long n_barrier_attempts;
 static long n_barrier_successes; /* did rcu_barrier test succeed? */
 static unsigned long n_read_exits;
@@ -438,6 +444,7 @@ struct rcu_torture_ops {
 	long cbflood_max;
 	int irq_capable;
 	int nmi_capable;
+	bool (*nmi_reader_capable)(void); // Are ->readlock() and ->readunlock() NMI-safe?
 	int can_boost;
 	int extendables;
 	int slow_gps;
@@ -790,6 +797,14 @@ static int srcu_torture_read_lock(void)
 	return ret;
 }
 
+// Only these reader flavors may be used from NMI.
+static bool srcu_torture_nmi_reader_capable(void)
+{
+	return reader_flavor &&
+	       !(reader_flavor & ~(SRCU_READ_FLAVOR_NMI | SRCU_READ_FLAVOR_FAST |
+				   SRCU_READ_FLAVOR_ATOMIC));
+}
+
 static void
 srcu_read_delay(struct torture_random_state *rrsp, struct rt_read_seg *rtrsp)
 {
@@ -981,6 +996,7 @@ static struct rcu_torture_ops srcu_ops = {
 	.have_up_down	= IS_ENABLED(CONFIG_TINY_SRCU)
 				? 0 : SRCU_READ_FLAVOR_NORMAL | SRCU_READ_FLAVOR_FAST_UPDOWN,
 	.nmi_capable	= 1,
+	.nmi_reader_capable = srcu_torture_nmi_reader_capable,
 	.name		= "srcu"
 };
 
@@ -1049,6 +1065,7 @@ static struct rcu_torture_ops srcud_ops = {
 	.have_up_down	= IS_ENABLED(CONFIG_TINY_SRCU)
 				? 0 : SRCU_READ_FLAVOR_NORMAL | SRCU_READ_FLAVOR_FAST_UPDOWN,
 	.nmi_capable	= 1,
+	.nmi_reader_capable = srcu_torture_nmi_reader_capable,
 	.name		= "srcud"
 };
 
@@ -2711,6 +2728,10 @@ static DEFINE_TORTURE_RANDOM_PERCPU(rcu_torture_timer_rand);
  * per-CPU hardware perf counter overflows into an NMI, and its handler submits
  * a preallocated callback via ->call().  One callback per CPU is in flight at a
  * time (guarded by an atomic) to avoid allocating in NMI.
+ *
+ * The same handler also runs a reader, for flavors whose ->nmi_reader_capable()
+ * says that their readers are NMI-safe.  The reader may interrupt another
+ * reader on its CPU, in the middle of its counter update.
  */
 #ifdef CONFIG_PERF_EVENTS
 static struct perf_event_attr rcu_torture_nmi_attr = {
@@ -2735,6 +2756,8 @@ struct rcu_torture_nmi_cb {
 
 static struct perf_event **rcu_torture_nmi_events;
 static int rcu_torture_nmi_hp_state;
+static bool rcu_torture_nmi_do_calls;
+static bool rcu_torture_nmi_do_reads;
 static DEFINE_PER_CPU(struct rcu_torture_nmi_cb, rcu_torture_nmi_cb);
 
 static void rcu_torture_nmi_invoked(struct rcu_head *rhp)
@@ -2745,6 +2768,34 @@ static void rcu_torture_nmi_invoked(struct rcu_head *rhp)
 	atomic_set(&rtncp->inuse, 0);
 }
 
+/*
+ * A reader in NMI.  The NMI may interrupt lockdep, which then does not record
+ * the reader, so the pointer is loaded without a lockdep check.  The element
+ * must not go through two grace periods before the reader ends.
+ * rcu_torture_stats_print() checks this in the pipe counts.
+ */
+static void rcu_torture_nmi_read(void)
+{
+	unsigned long started, completed;
+	struct rcu_torture *p;
+	int idx, pipe_count;
+
+	idx = cur_ops->readlock();
+	started = cur_ops->get_gp_seq();
+	p = rcu_dereference_raw(rcu_torture_current);
+	if (p) {
+		if (!READ_ONCE(p->rtort_mbtest))
+			atomic_inc(&n_rcu_torture_mberror);
+		pipe_count = min(READ_ONCE(p->rtort_pipe_count), RCU_TORTURE_PIPE_LEN);
+		completed = rcutorture_seq_diff(cur_ops->get_gp_seq(), started);
+		completed = min_t(unsigned long, completed, RCU_TORTURE_PIPE_LEN);
+		atomic_long_inc(&rcu_torture_nmi_pipe[pipe_count]);
+		atomic_long_inc(&rcu_torture_nmi_batch[completed]);
+	}
+	cur_ops->readunlock(idx);
+	atomic_long_inc(&n_rcu_torture_nmi_read);
+}
+
 static void rcu_torture_nmi_overflow(struct perf_event *event,
 				     struct perf_sample_data *data,
 				     struct pt_regs *regs)
@@ -2753,10 +2804,12 @@ static void rcu_torture_nmi_overflow(struct perf_event *event,
 
 	if (!in_nmi())
 		return;
-	if (cur_ops->call && !atomic_xchg(&rtncp->inuse, 1)) {
+	if (rcu_torture_nmi_do_calls && !atomic_xchg(&rtncp->inuse, 1)) {
 		atomic_long_inc(&n_rcu_torture_nmi_call);
 		cur_ops->call(&rtncp->rh, rcu_torture_nmi_invoked);
 	}
+	if (rcu_torture_nmi_do_reads)
+		rcu_torture_nmi_read();
 }
 
 static int rcu_torture_nmi_online(unsigned int cpu)
@@ -2789,7 +2842,10 @@ static void rcu_torture_nmi_init(void)
 {
 	int ret;
 
-	if (!nmi_calls || !cur_ops->nmi_capable || !cur_ops->call)
+	rcu_torture_nmi_do_calls = nmi_calls && cur_ops->nmi_capable && cur_ops->call;
+	rcu_torture_nmi_do_reads = nmi_readers && cur_ops->nmi_reader_capable &&
+				   cur_ops->nmi_reader_capable();
+	if (!rcu_torture_nmi_do_calls && !rcu_torture_nmi_do_reads)
 		return;
 	rcu_torture_nmi_events = kzalloc_objs(*rcu_torture_nmi_events,
 					      nr_cpu_ids);
@@ -2815,9 +2871,12 @@ static void rcu_torture_nmi_cleanup(void)
 	}
 	kfree(rcu_torture_nmi_events);
 	rcu_torture_nmi_events = NULL;
-	if (!atomic_long_read(&n_rcu_torture_nmi_call))
+	if (rcu_torture_nmi_do_calls && !atomic_long_read(&n_rcu_torture_nmi_call))
 		pr_alert("%s: nmi_calls set but no ->call() ever issued from NMI, so NMI ->call() went untested (no PMU, or NMIs unavailable here).\n",
 			 __func__);
+	if (rcu_torture_nmi_do_reads && !atomic_long_read(&n_rcu_torture_nmi_read))
+		pr_alert("%s: nmi_readers set but no reader ever ran in NMI, so NMI readers went untested (no PMU, or NMIs unavailable here).\n",
+			 __func__);
 }
 #else /* #ifdef CONFIG_PERF_EVENTS */
 static void rcu_torture_nmi_init(void) { }
@@ -3152,6 +3211,7 @@ rcu_torture_stats_print(void)
 	int cpu;
 	int i;
 	long pipesummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
+	int j;
 	long batchsummary[RCU_TORTURE_PIPE_LEN + 1] = { 0 };
 	long n_gpwraps = 0;
 	unsigned long ndowns = 0;
@@ -3183,6 +3243,10 @@ rcu_torture_stats_print(void)
 		if (pipesummary[i] != 0)
 			break;
 	} // The value of variable "i" is used later, so don't clobber it!
+	for (j = RCU_TORTURE_PIPE_LEN; j >= 0; j--) {
+		if (atomic_long_read(&rcu_torture_nmi_pipe[j]))
+			break;
+	} // Same for "j", for the readers in NMI.
 
 	pr_alert("%s%s ", torture_type, TORTURE_FLAG);
 	rtcp = rcu_access_pointer(rcu_torture_current);
@@ -3212,9 +3276,10 @@ rcu_torture_stats_print(void)
 		data_race(n_barrier_attempts),
 		data_race(n_rcu_torture_barrier_error));
 	pr_cont("read-exits: %ld ", data_race(n_read_exits)); // Statistic.
-	pr_cont("nmi-calls: %ld nmi-cbs: %ld ",
+	pr_cont("nmi-calls: %ld nmi-cbs: %ld nmi-reads: %ld ",
 		atomic_long_read(&n_rcu_torture_nmi_call),
-		atomic_long_read(&n_rcu_torture_nmi_cb));
+		atomic_long_read(&n_rcu_torture_nmi_cb),
+		atomic_long_read(&n_rcu_torture_nmi_read));
 	pr_cont("nocb-toggles: %ld:%ld ",
 		atomic_long_read(&n_nocb_offload), atomic_long_read(&n_nocb_deoffload));
 	pr_cont("gpwraps: %ld\n", n_gpwraps);
@@ -3223,7 +3288,7 @@ rcu_torture_stats_print(void)
 	if (atomic_read(&n_rcu_torture_mberror) ||
 	    atomic_read(&n_rcu_torture_mbchk_fail) ||
 	    n_rcu_torture_barrier_error || n_rcu_torture_boost_ktrerror ||
-	    n_rcu_torture_boost_failure || i > 1) {
+	    n_rcu_torture_boost_failure || i > 1 || j > 1) {
 		pr_cont("%s", "!!! ");
 		atomic_inc(&n_rcu_torture_error);
 		WARN_ON_ONCE(atomic_read(&n_rcu_torture_mberror));
@@ -3232,6 +3297,7 @@ rcu_torture_stats_print(void)
 		WARN_ON_ONCE(n_rcu_torture_boost_ktrerror); // no boost kthread
 		WARN_ON_ONCE(n_rcu_torture_boost_failure); // boost failed (TIMER_SOFTIRQ RT prio?)
 		WARN_ON_ONCE(i > 1); // Too-short grace period
+		WARN_ON_ONCE(j > 1); // Too-short grace period, reader in NMI
 	}
 	pr_cont("Reader Pipe: ");
 	for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++)
@@ -3244,6 +3310,20 @@ rcu_torture_stats_print(void)
 		pr_cont(" %ld", batchsummary[i]);
 	pr_cont("\n");
 
+	if (atomic_long_read(&n_rcu_torture_nmi_read)) {
+		pr_alert("%s%s ", torture_type, TORTURE_FLAG);
+		pr_cont("NMI Reader Pipe: ");
+		for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++)
+			pr_cont(" %ld", atomic_long_read(&rcu_torture_nmi_pipe[i]));
+		pr_cont("\n");
+
+		pr_alert("%s%s ", torture_type, TORTURE_FLAG);
+		pr_cont("NMI Reader Batch: ");
+		for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++)
+			pr_cont(" %ld", atomic_long_read(&rcu_torture_nmi_batch[i]));
+		pr_cont("\n");
+	}
+
 	pr_alert("%s%s ", torture_type, TORTURE_FLAG);
 	pr_cont("Free-Block Circulation: ");
 	for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++) {
@@ -5049,6 +5129,10 @@ rcu_torture_init(void)
 			per_cpu(rcu_torture_batch, cpu)[i] = 0;
 		}
 	}
+	for (i = 0; i < RCU_TORTURE_PIPE_LEN + 1; i++) {
+		atomic_long_set(&rcu_torture_nmi_pipe[i], 0);
+		atomic_long_set(&rcu_torture_nmi_batch[i], 0);
+	}
 	err_segs_recorded = 0;
 	rt_read_nsegs = 0;
 
-- 
2.47.3

[PATCH v2 6/7] rcutorture: Test the irqsave forms of atomic SRCU readers

[Mathieu Desnoyers]

Nothing tests srcu_read_lock_atomic_irqsave() and
srcu_read_unlock_atomic_irqrestore(). Testing them in the normal readers
has two obstacles. rcutorture_one_extend() ends the readers of a CPU in
any order, but each irqrestore restores the state its own irqsave saw.
A reader extension may also enable interrupts or bottom halves within
a reader, which an irqsave reader forbids.

Add an atomic_irqsave module parameter. With it, the atomic readers of
each CPU alternate between the plain and the irqsave forms. A per-CPU
count tracks the irqsave readers: the first one saves the interrupt
state, and only the last one restores it. The others restore the
current state, with interrupts disabled. The irqsave readers run only
for a torture type without reader extensions: every srcu type but
busted_srcud. The parameter also stops readers from ending under
->pi_lock, which would enable interrupts under a raw spinlock.

An irqsave reader checks that interrupts are still disabled when it
ends.

Add an atomic-srcu-irqsave run of SRCU-P to torture.sh.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/rcu/rcutorture.c                       | 80 +++++++++++++++++--
 .../selftests/rcutorture/bin/torture.sh       |  6 ++
 2 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 9889293c968b..2ec461414263 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -61,6 +61,8 @@ MODULE_AUTHOR("Paul E. McKenney <paulmck@linux.ibm.com> and Josh Triplett <josh@
 #define RCUTORTURE_RDR_MASK_1	(0xff << RCUTORTURE_RDR_SHIFT_1)
 #define RCUTORTURE_RDR_SHIFT_2	16	// Put SRCU index in upper bits.
 #define RCUTORTURE_RDR_MASK_2	(0xff << RCUTORTURE_RDR_SHIFT_2)
+#define SRCU_TORTURE_IRQSAVE	0x80	// In an SRCU reader index (MASK_1 or _2):
+					//  the atomic reader disabled interrupts.
 #define RCUTORTURE_RDR_BH	0x01	// Extend readers by disabling bh.
 #define RCUTORTURE_RDR_IRQ	0x02	//  ... disabling interrupts.
 #define RCUTORTURE_RDR_PREEMPT	0x04	//  ... disabling preemption.
@@ -134,6 +136,8 @@ torture_param(int, preempt_interval, MSEC_PER_SEC, "Interval between preemptions
 torture_param(int, read_exit_delay, 13, "Delay between read-then-exit episodes (s)");
 torture_param(int, read_exit_burst, 16, "# of read-then-exit bursts per episode, zero to disable");
 torture_param(int, reader_flavor, SRCU_READ_FLAVOR_NORMAL, "Reader flavors to use, one per bit.");
+torture_param(bool, atomic_irqsave, false,
+	      "Atomic SRCU readers alternate per CPU with irqsave readers");
 torture_param(int, shuffle_interval, 3, "Number of seconds between shuffles");
 torture_param(int, shutdown_secs, 0, "Shutdown time (s), <= zero to disable.");
 torture_param(int, stall_cpu, 0, "Stall duration (s), zero to disable.");
@@ -732,6 +736,69 @@ static void srcu_torture_init_forbidden_apis(void)
 	cur_ops->start_gp_poll = NULL;
 }
 
+/*
+ * With atomic_irqsave, every other atomic SRCU reader of a CPU disables
+ * interrupts instead of preemption.  rcutorture_one_extend() may end the
+ * readers of a CPU in any order, so the first one saves the interrupt state
+ * and the last one restores it.  No extension may enable interrupts or bottom
+ * halves within such a reader.  So the irqsave readers run only for a torture
+ * type without extendables: every srcu type but busted_srcud.  No reader may
+ * end under ->pi_lock.
+ */
+static bool srcu_torture_irqsave_on;
+
+struct srcu_torture_irqsave {
+	unsigned int count;	// Picks the form of the next reader.
+	int depth;		// Readers of this CPU that disabled interrupts.
+	unsigned long flags;	// Interrupt state before the first one.
+};
+
+static DEFINE_PER_CPU(struct srcu_torture_irqsave, srcu_torture_irqsave);
+
+static void srcu_torture_init_irqsave(void)
+{
+	if (!atomic_irqsave || !(reader_flavor & SRCU_READ_FLAVOR_ATOMIC) ||
+	    cur_ops->extendables)
+		return;
+	srcu_torture_irqsave_on = true;
+	cur_ops->no_pi_lock = 1;
+	VERBOSE_TOROUT_STRING("srcu_torture_init atomic SRCU, irqsave readers");
+}
+
+static int srcu_torture_read_lock_atomic(void)
+{
+	struct srcu_torture_irqsave *stip;
+	unsigned long flags;
+	int idx;
+
+	if (!srcu_torture_irqsave_on || in_nmi() ||
+	    !(this_cpu_inc_return(srcu_torture_irqsave.count) & 0x1))
+		return srcu_read_lock_atomic(srcu_ctlp);
+	idx = srcu_read_lock_atomic_irqsave(srcu_ctlp, flags);
+	stip = this_cpu_ptr(&srcu_torture_irqsave);
+	if (!stip->depth++)
+		stip->flags = flags;
+	return idx | SRCU_TORTURE_IRQSAVE;
+}
+
+static void srcu_torture_read_unlock_atomic(int idx)
+{
+	struct srcu_torture_irqsave *stip;
+	unsigned long flags;
+
+	if (!(idx & SRCU_TORTURE_IRQSAVE)) {
+		srcu_read_unlock_atomic(srcu_ctlp, idx);
+		return;
+	}
+	idx &= ~SRCU_TORTURE_IRQSAVE;
+	WARN_ON_ONCE(!irqs_disabled());  // Its section must be interrupts-off.
+	stip = this_cpu_ptr(&srcu_torture_irqsave);
+	local_save_flags(flags);  // Disabled: keep it so for the other readers.
+	if (!WARN_ON_ONCE(stip->depth <= 0) && !--stip->depth)
+		flags = stip->flags;
+	srcu_read_unlock_atomic_irqrestore(srcu_ctlp, idx, flags);
+}
+
 static void srcu_torture_init(void)
 {
 	rcu_sync_torture_init();
@@ -751,6 +818,7 @@ static void srcu_torture_init(void)
 		srcu_ctlp = &srcu_ctla;
 		VERBOSE_TOROUT_STRING("srcu_torture_init atomic SRCU");
 		srcu_torture_init_forbidden_apis();
+		srcu_torture_init_irqsave();
 	}
 }
 
@@ -790,9 +858,9 @@ static int srcu_torture_read_lock(void)
 		ret += idx << 3;
 	}
 	if (reader_flavor & SRCU_READ_FLAVOR_ATOMIC) {
-		idx = srcu_read_lock_atomic(srcu_ctlp);
-		WARN_ON_ONCE(idx & ~0x1);
-		ret += idx << 4;
+		idx = srcu_torture_read_lock_atomic();
+		WARN_ON_ONCE(idx & ~(0x1 | SRCU_TORTURE_IRQSAVE));
+		ret += ((idx & 0x1) << 4) | (idx & SRCU_TORTURE_IRQSAVE);
 	}
 	return ret;
 }
@@ -832,9 +900,10 @@ srcu_read_delay(struct torture_random_state *rrsp, struct rt_read_seg *rtrsp)
 
 static void srcu_torture_read_unlock(int idx)
 {
-	WARN_ON_ONCE((reader_flavor && (idx & ~reader_flavor)) || (!reader_flavor && (idx & ~0x1)));
+	WARN_ON_ONCE((reader_flavor && (idx & ~(reader_flavor | SRCU_TORTURE_IRQSAVE))) ||
+		     (!reader_flavor && (idx & ~0x1)));
 	if (reader_flavor & SRCU_READ_FLAVOR_ATOMIC)
-		srcu_read_unlock_atomic(srcu_ctlp, (idx & 0x10) >> 4);
+		srcu_torture_read_unlock_atomic(((idx & 0x10) >> 4) | (idx & SRCU_TORTURE_IRQSAVE));
 	if (reader_flavor & SRCU_READ_FLAVOR_FAST_UPDOWN)
 		srcu_read_unlock_fast_updown(srcu_ctlp,
 					     __srcu_ctr_to_ptr(srcu_ctlp, (idx & 0x8) >> 3));
@@ -1019,6 +1088,7 @@ static void srcud_torture_init(void)
 		WARN_ON(init_srcu_struct_atomic(&srcu_ctld));
 		VERBOSE_TOROUT_STRING("srcud_torture_init atomic SRCU");
 		srcu_torture_init_forbidden_apis();
+		srcu_torture_init_irqsave();
 	} else {
 		WARN_ON(init_srcu_struct(&srcu_ctld));
 	}
diff --git a/tools/testing/selftests/rcutorture/bin/torture.sh b/tools/testing/selftests/rcutorture/bin/torture.sh
index e7b578ec152f..18a03fad1038 100755
--- a/tools/testing/selftests/rcutorture/bin/torture.sh
+++ b/tools/testing/selftests/rcutorture/bin/torture.sh
@@ -519,6 +519,12 @@ then
 	torture_set "atomic-srcu" tools/testing/selftests/rcutorture/bin/kvm.sh \
 		--allcpus --duration "$duration_rcutorture" \
 		--configs "$configs_atomic_srcu" --trust-make
+
+	# Half the readers then use srcu_read_lock_atomic_irqsave().
+	torture_bootargs="rcutorture.reader_flavor=0x10 rcutorture.atomic_irqsave=1"
+	torture_set "atomic-srcu-irqsave" tools/testing/selftests/rcutorture/bin/kvm.sh \
+		--allcpus --duration "$duration_rcutorture" \
+		--configs "SRCU-P" --trust-make
 fi
 
 if test "$do_locktorture" = "yes"
-- 
2.47.3

[PATCH v2 7/7] rcutorture: Check an atomic SRCU wait within an RCU reader

[Mathieu Desnoyers]

synchronize_srcu_atomic() may be called within rcu_read_lock().  It must
then not end an RCU grace period, because its caller still holds it up.
Tiny SRCU did end one, by calling synchronize_rcu().

Check this once, when a torture type starts with atomic SRCU readers.
Take a cookie from get_state_synchronize_rcu() within rcu_read_lock(),
call synchronize_srcu_atomic(), and warn if
poll_state_synchronize_rcu() then reports the cookie as completed.  The
cookie is taken within the reader, so no RCU grace period can complete
it before rcu_read_unlock().

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/rcu/rcutorture.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 2ec461414263..d9c9ff653cb6 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c
@@ -799,6 +799,24 @@ static void srcu_torture_read_unlock_atomic(int idx)
 	srcu_read_unlock_atomic_irqrestore(srcu_ctlp, idx, flags);
 }
 
+/*
+ * Wait for an atomic SRCU grace period within an RCU reader.  The wait must
+ * not end an RCU grace period: this reader still holds it up.  The cookie is
+ * taken within the reader, so no RCU grace period can complete it there.
+ */
+static void srcu_torture_check_atomic_in_rcu_reader(void)
+{
+	unsigned long cookie;
+
+	rcu_read_lock();
+	cookie = get_state_synchronize_rcu();
+	synchronize_srcu_atomic(srcu_ctlp);
+	WARN_ONCE(poll_state_synchronize_rcu(cookie),
+		  "%s: synchronize_srcu_atomic() ended an RCU grace period within an RCU reader\n",
+		  __func__);
+	rcu_read_unlock();
+}
+
 static void srcu_torture_init(void)
 {
 	rcu_sync_torture_init();
@@ -819,6 +837,7 @@ static void srcu_torture_init(void)
 		VERBOSE_TOROUT_STRING("srcu_torture_init atomic SRCU");
 		srcu_torture_init_forbidden_apis();
 		srcu_torture_init_irqsave();
+		srcu_torture_check_atomic_in_rcu_reader();
 	}
 }
 
@@ -1093,6 +1112,8 @@ static void srcud_torture_init(void)
 		WARN_ON(init_srcu_struct(&srcu_ctld));
 	}
 	srcu_ctlp = &srcu_ctld;
+	if (reader_flavor & SRCU_READ_FLAVOR_ATOMIC)
+		srcu_torture_check_atomic_in_rcu_reader();
 }
 
 static void srcu_torture_cleanup(void)
-- 
2.47.3