On 12.12.2023 10:47, Juergen Gross wrote:
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -541,6 +541,55 @@ void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned
> long flags)
> local_irq_restore(flags);
> }
>
> +int nrspin_trylock(rspinlock_t *lock)
> +{
> + check_lock(&lock->debug, true);
> +
> + if ( unlikely(lock->recurse_cpu != SPINLOCK_NO_CPU) )
> + return 0;
> +
> + return spin_trylock_common(&lock->tickets, &lock->debug,
> LOCK_PROFILE_PAR);
> +}
I wonder if we shouldn't take the opportunity and stop having trylock
functions have "int" return type. Perhaps already spin_trylock_common()
when introduced could use "bool" instead, then here following suit.
> +void nrspin_lock(rspinlock_t *lock)
> +{
> + spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, NULL,
> + NULL);
> +}
> +
> +void nrspin_unlock(rspinlock_t *lock)
> +{
> + spin_unlock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
> +}
> +
> +void nrspin_lock_irq(rspinlock_t *lock)
> +{
> + ASSERT(local_irq_is_enabled());
> + local_irq_disable();
> + nrspin_lock(lock);
> +}
> +
> +void nrspin_unlock_irq(rspinlock_t *lock)
> +{
> + nrspin_unlock(lock);
> + local_irq_enable();
> +}
> +
> +unsigned long __nrspin_lock_irqsave(rspinlock_t *lock)
> +{
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + nrspin_lock(lock);
> + return flags;
Nit: Strictly speaking we want a blank line ahead of this "return".
> @@ -166,11 +172,15 @@ struct lock_profile_qhead { };
> struct lock_profile { };
>
> #define SPIN_LOCK_UNLOCKED {
> \
> + .debug =_LOCK_DEBUG,
> \
> +}
> +#define RSPIN_LOCK_UNLOCKED {
> \
> + .debug =_LOCK_DEBUG,
> \
> .recurse_cpu = SPINLOCK_NO_CPU,
> \
> .debug =_LOCK_DEBUG,
> \
> }
Initializing .debug twice?
> @@ -180,7 +190,6 @@ struct lock_profile { };
>
> #endif
>
> -
> typedef union {
> uint32_t head_tail;
> struct {
Looks like this might be undoing what the earlier patch isn't going to
do anymore?
> @@ -242,6 +257,19 @@ void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned
> long flags);
> int rspin_is_locked(const rspinlock_t *lock);
> void rspin_barrier(rspinlock_t *lock);
>
> +int nrspin_trylock(rspinlock_t *lock);
> +void nrspin_lock(rspinlock_t *lock);
> +void nrspin_unlock(rspinlock_t *lock);
> +void nrspin_lock_irq(rspinlock_t *lock);
> +void nrspin_unlock_irq(rspinlock_t *lock);
> +#define nrspin_lock_irqsave(l, f) \
> + ({ \
> + BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long)); \
> + ((f) = __nrspin_lock_irqsave(l)); \
I don't think the outer pair of parentheses is needed here. As to the
leading double underscores, see comments elsewhere.
Jan
