[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v5 08/13] xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
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. @@ -465,6 +458,23 @@ void _spin_barrier(spinlock_t *lock) spin_barrier_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR); }+bool _rspin_is_locked(const rspinlock_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(); +}Here otoh I wonder if both the comment and the spin_is_locked_common() part of the condition are actually correct. Oh, the latter needs retaining as long as we have nrspin_*() functions, I suppose. But the comment could surely do with improving a little - at the very least "yet we return "false"" isn't quite right; minimally there's a "may" missing. If anything I guess the comment shouldn't gain a "may", but rather say "Recursive locks may be locked by another CPU via rspin_lock() ..." Juergen
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |