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

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

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