Re: [PATCH v3 04/13] xen/spinlock: introduce new type for recursive spinlocks

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 20/11/2023 11:38, Juergen Gross wrote:
> Introduce a new type "rspinlock_t" to be used for recursive spinlocks.
>
> For now it is only an alias of spinlock_t, so both types can still be
> used for recursive spinlocks. This will be changed later, though.
>
> Switch all recursive spinlocks to the new type.
>
> Define the initializer helpers and use them where appropriate.
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2:
> - carved out from V1 patch
> ---
>   xen/arch/x86/include/asm/mm.h |  2 +-
>   xen/arch/x86/mm/mm-locks.h    |  2 +-
>   xen/common/domain.c           |  4 ++--
>   xen/common/ioreq.c            |  2 +-
>   xen/drivers/char/console.c    |  4 ++--
>   xen/drivers/passthrough/pci.c |  2 +-
>   xen/include/xen/sched.h       |  6 +++---
>   xen/include/xen/spinlock.h    | 19 +++++++++++++++----
>   8 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/mm.h b/xen/arch/x86/include/asm/mm.h
> index 05dfe35502..8a6e0c283f 100644
> --- a/xen/arch/x86/include/asm/mm.h
> +++ b/xen/arch/x86/include/asm/mm.h
> @@ -596,7 +596,7 @@ unsigned long domain_get_maximum_gpfn(struct domain *d);
>   
>   /* Definition of an mm lock: spinlock with extra fields for debugging */
>   typedef struct mm_lock {
> -    spinlock_t         lock;
> +    rspinlock_t        lock;
Considering it's rspinlock_t rather than spinlock_recursive, do we want
to change also mm_lock_recursive() and paging_lock_recursive() into
mm_rlock() and paging_rlock() ?


> diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
> index bbe1472571..19561d5e61 100644
> --- a/xen/include/xen/spinlock.h
> +++ b/xen/include/xen/spinlock.h
> [snip]
> @@ -182,8 +191,10 @@ typedef struct spinlock {
>   #endif
>   } spinlock_t;
>   
> +typedef spinlock_t rspinlock_t;
>   
>   #define spin_lock_init(l) (*(l) = (spinlock_t)SPIN_LOCK_UNLOCKED)
> +#define rspin_lock_init(l) (*(l) = (rspinlock_t)SPIN_LOCK_UNLOCKED)
nit: Both variants of [r]spin_lock_init(l) could be inline functions
rather than macros.

Cheers,
Alejandro