Re: [PATCH 1/2] xen/spinlocks: spin_trylock with interrupts off is always fine

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.10.2020 15:24, Juergen Gross wrote:
> Even if a spinlock was taken with interrupts on before calling
> spin_trylock() with interrupts off is fine, as it can't block.
> 
> Add a bool parameter "try" to check_lock() for handling this case.
> 
> Remove the call of check_lock() from _spin_is_locked(), as it really
> serves no purpose and it can even lead to false crashes, e.g. when
> a lock was taken correctly with interrupts enabled and the call of
> _spin_is_locked() happened with interrupts off. In case the lock is
> taken with wrong interrupt flags this will be catched when taking
> the lock.
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
albeit I guess ...

> @@ -42,7 +42,13 @@ static void check_lock(union lock_debug *debug)
>       * 
>       * To guard against this subtle bug we latch the IRQ safety of every
>       * spinlock in the system, on first use.
> +     *
> +     * A spin_trylock() or spin_is_locked() with interrupts off is always
> +     * fine, as those can't block and above deadlock scenario doesn't apply.
>       */
> +    if ( try && irq_safe )
> +        return;

... the reference to spin_is_locked() here wants dropping,
since ...

> @@ -220,8 +226,6 @@ void _spin_unlock_irqrestore(spinlock_t *lock, unsigned 
> long flags)
>  
>  int _spin_is_locked(spinlock_t *lock)
>  {
> -    check_lock(&lock->debug);

... you drop the call here?

Jan