Re: [PATCH v4 07/12] xen/spinlock: add explicit non-recursive locking functions

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 12/12/2023 09:47, Juergen Gross wrote:
> -#define spin_lock_init_prof(s, l) __spin_lock_init_prof(s, l, spinlock_t)
> -#define rspin_lock_init_prof(s, l) __spin_lock_init_prof(s, l, rspinlock_t)
> +#define spin_lock_init_prof(s, l)                                             \
> +    __spin_lock_init_prof(s, l, lock, spinlock_t, 0)
> +#define rspin_lock_init_prof(s, l)                                            \
> +    __spin_lock_init_prof(s, l, rlock, rspinlock_t, 1)
>   
>   void _lock_profile_register_struct(
>       int32_t type, struct lock_profile_qhead *qhead, int32_t idx);
> @@ -174,6 +179,7 @@ struct lock_profile_qhead { };
>   
>   #endif
>   
> +
Spurious change?

>   typedef union {
>       uint32_t head_tail;
>       struct {
> @@ -261,4 +267,12 @@ void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned 
> long flags);
>   /* Ensure a lock is quiescent between two critical operations. */
>   #define spin_barrier(l)               _spin_barrier(l)
>   
> +#define nrspin_trylock(l)    spin_trylock(l)
> +#define nrspin_lock(l)       spin_lock(l)
> +#define nrspin_unlock(l)     spin_unlock(l)
> +#define nrspin_lock_irq(l)   spin_lock_irq(l)
> +#define nrspin_unlock_irq(l) spin_unlock_irq(l)
> +#define nrspin_lock_irqsave(l, f)      spin_lock_irqsave(l, f)
> +#define nrspin_unlock_irqrestore(l, f) spin_unlock_irqrestore(l, f)
There is a comment describing [r]spinlock but not this new variant. Can 
you add one?
That said, I know this is existing code, but I have to admit this is a 
bit unclear why we are allowing an recursive spinlock to be 
non-recursive. To me it sounds like we are making the typesafe not very 
safe because it doesn't guarantee that we are not mixing the call 
nrspin_* with rspin_*.
Cheers,

--
Julien Grall