rqspinlock: Enclose lock/unlock within lock entry acquisitions

There is a tiny window between the fast-path cmpxchg and the grabbing of
the lock entry where an NMI could land, attempt the same lock that was
just acquired, and end up timing out. This is not ideal. Instead, move
the lock entry acquisition from the fast path to before the cmpxchg, and
remove the grabbing of the lock entry in the slow path, assuming it was
already taken by the fast path.

Do something similar for the unlock case, where smp_store_release is
moved to before the WRITE_ONCE on the lock entry. We could have the case
before where if an NMI landed write after the WRITE_ONCE, but before the
unlock, it would miss the AA case.

 [ kkd: The movement of the store release is not trivial, see
	comments/revisit later. ]

Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>

Author: kkdwvd

include/asm-generic/rqspinlock.h

@@ -175,8 +175,15 @@ static __always_inline int res_spin_lock(rqspinlock_t *lock)
 {
 	int val = 0;
 
+	/*
+	 * Grab the deadlock detection entry before doing the cmpxchg, so that
+	 * reentrancy due to NMIs between the succeeding cmpxchg and creation of
+	 * held lock entry can correctly detect an acquisition attempt in the
+	 * interrupted context.
+	 */
+	grab_held_lock_entry(lock);
+
 	if (likely(atomic_try_cmpxchg_acquire(&lock->val, &val, _Q_LOCKED_VAL))) {
-		grab_held_lock_entry(lock);
 		return 0;
 	}
 	return resilient_queued_spin_lock_slowpath(lock, val);
@@ -192,6 +199,7 @@ static __always_inline void res_spin_unlock(rqspinlock_t *lock)
 {
 	struct rqspinlock_held *rqh = this_cpu_ptr(&rqspinlock_held_locks);
 
+	smp_store_release(&lock->locked, 0);
 	if (unlikely(rqh->cnt > RES_NR_HELD))
 		goto unlock;
 	WRITE_ONCE(rqh->locks[rqh->cnt - 1], NULL);
@@ -213,7 +221,6 @@ static __always_inline void res_spin_unlock(rqspinlock_t *lock)
 	 * the store below is done, which would ensure the entry is overwritten
 	 * to NULL, etc.
 	 */
-	smp_store_release(&lock->locked, 0);
 	this_cpu_dec(rqspinlock_held_locks.cnt);
 }
 

kernel/bpf/rqspinlock.c

@@ -275,7 +275,7 @@ int __lockfunc resilient_tas_spin_lock(rqspinlock_t *lock)
 	int val, ret = 0;
 
 	RES_INIT_TIMEOUT(ts);
-	grab_held_lock_entry(lock);
+	/* Deadlock detection entry already held after failing slow path. */
 
 	/*
 	 * Since the waiting loop's time is dependent on the amount of
@@ -397,10 +397,7 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
 		goto queue;
 	}
 
-	/*
-	 * Grab an entry in the held locks array, to enable deadlock detection.
-	 */
-	grab_held_lock_entry(lock);
+	/* Deadlock detection entry already held after failing slow path. */
 
 	/*
 	 * We're pending, wait for the owner to go away.
@@ -448,11 +445,7 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
 	 */
 queue:
 	lockevent_inc(lock_slowpath);
-	/*
-	 * Grab deadlock detection entry for the queue path.
-	 */
-	grab_held_lock_entry(lock);
-
+	/* Deadlock detection entry already held after failing slow path. */
 	node = this_cpu_ptr(&rqnodes[0].mcs);
 	idx = node->count++;
 	tail = encode_tail(smp_processor_id(), idx);