Skip to content

Commit e9e0cdf

Browse files
kkdwvdKernel Patches Daemon
authored andcommitted
rqspinlock: Disable queue destruction for deadlocks
Disable propagation and unwinding of the waiter queue in case the head waiter detects a deadlock condition, but keep it enabled in case of the timeout fallback. Currently, when the head waiter experiences an AA deadlock, it will signal all its successors in the queue to exit with an error. This is not ideal for cases where the same lock is held in contexts which can cause errors in an unrestricted fashion (e.g., BPF programs, or kernel paths invoked through BPF programs), and core kernel logic which is written in a correct fashion and does not expect deadlocks. The same reasoning can be extended to ABBA situations. Depending on the actual runtime schedule, one or both of the head waiters involved in an ABBA situation can detect and exit directly without terminating their waiter queue. If the ABBA situation manifests again, the waiters will keep exiting until progress can be made, or a timeout is triggered in case of more complicated locking dependencies. We still preserve the queue destruction in case of timeouts, as either the locking dependencies are too complex to be captured by AA and ABBA heuristics, or the owner is perpetually stuck. As such, it would be unwise to continue to apply the timeout for each new head waiter without terminating the queue, since we may end up waiting for more than 250 ms in aggregate with all participants in the locking transaction. The patch itself is fairly simple; we can simply signal our successor to become the next head waiter, and leave the queue without attempting to acquire the lock. With this change, the behavior for waiters in case of deadlocks experienced by a predecessor changes. It is guaranteed that call sites will no longer receive errors if the predecessors encounter deadlocks and the successors do not participate in one. This should lower the failure rate for waiters that are not doing improper locking opreations, just because they were unlucky to queue behind a misbehaving waiter. However, timeouts are still a possibility, hence they must be accounted for, so users cannot rely upon errors not occuring at all. Suggested-by: Amery Hung <[email protected]> Suggested-by: Alexei Starovoitov <[email protected]> Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
1 parent b8a9697 commit e9e0cdf

File tree

1 file changed

+8
-0
lines changed

1 file changed

+8
-0
lines changed

kernel/bpf/rqspinlock.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -572,6 +572,14 @@ int __lockfunc resilient_queued_spin_lock_slowpath(rqspinlock_t *lock, u32 val)
572572
val = res_atomic_cond_read_acquire(&lock->val, !(VAL & _Q_LOCKED_PENDING_MASK) ||
573573
RES_CHECK_TIMEOUT(ts, ret, _Q_LOCKED_PENDING_MASK));
574574

575+
/* Disable queue destruction when we detect deadlocks. */
576+
if (ret == -EDEADLK) {
577+
if (!next)
578+
next = smp_cond_load_relaxed(&node->next, (VAL));
579+
arch_mcs_spin_unlock_contended(&next->locked);
580+
goto err_release_node;
581+
}
582+
575583
waitq_timeout:
576584
if (ret) {
577585
/*

0 commit comments

Comments
 (0)