On 18.03.24 16:44, Jan Beulich wrote:
On 18.03.2024 16:31, Jürgen Groß wrote:
On 18.03.24 15:57, Jan Beulich wrote:
On 14.03.2024 08:20, Juergen Gross wrote:
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -395,14 +395,7 @@ static bool always_inline spin_is_locked_common(const
spinlock_tickets_t *t)
int _spin_is_locked(const spinlock_t *lock)
{
- /*
- * Recursive locks may be locked by another CPU, yet we return
- * "false" here, making this function suitable only for use in
- * ASSERT()s and alike.
- */
- return lock->recurse_cpu == SPINLOCK_NO_CPU
- ? spin_is_locked_common(&lock->tickets)
- : lock->recurse_cpu == smp_processor_id();
+ return spin_is_locked_common(&lock->tickets);
}
The "only suitable for ASSERT()s and alike" part of the comment wants
to survive here, I think.
Why?
I could understand you asking for putting such a comment to spinlock.h
mentioning that any *_is_locked() variant isn't safe, but with
_spin_is_locked() no longer covering recursive locks the comment's reasoning
is no longer true.
Hmm. I guess there is a difference in expectations. To me, these
functions in principle ought to report whether the lock is "owned",
not just "locked by some CPU". They don't, hence why they may not be
used for other than ASSERT()s.
Okay, thanks for clarification. I'll change the comment accordingly.
Juergen
