| Message ID | 20240314072029.16937-12-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/spinlock: make recursive spinlocks a dedicated type | expand |
On 14.03.2024 08:20, Juergen Gross wrote: > --- a/xen/include/xen/spinlock.h > +++ b/xen/include/xen/spinlock.h > @@ -8,16 +8,16 @@ > #include <asm/system.h> > #include <asm/spinlock.h> > > -#define SPINLOCK_CPU_BITS 12 > +#define SPINLOCK_CPU_BITS 16 > > #ifdef CONFIG_DEBUG_LOCKS > union lock_debug { > - uint16_t val; > -#define LOCK_DEBUG_INITVAL 0xffff > + uint32_t val; > +#define LOCK_DEBUG_INITVAL 0xffffffff With this #define I can see the desire for using a fixed width type for "val". However, ... > struct { > - uint16_t cpu:SPINLOCK_CPU_BITS; > -#define LOCK_DEBUG_PAD_BITS (14 - SPINLOCK_CPU_BITS) > - uint16_t :LOCK_DEBUG_PAD_BITS; > + uint32_t cpu:SPINLOCK_CPU_BITS; > +#define LOCK_DEBUG_PAD_BITS (30 - SPINLOCK_CPU_BITS) > + uint32_t :LOCK_DEBUG_PAD_BITS; .. "unsigned int" ought to be fine for both of these. Jan
On 18.03.24 16:08, Jan Beulich wrote: > On 14.03.2024 08:20, Juergen Gross wrote: >> --- a/xen/include/xen/spinlock.h >> +++ b/xen/include/xen/spinlock.h >> @@ -8,16 +8,16 @@ >> #include <asm/system.h> >> #include <asm/spinlock.h> >> >> -#define SPINLOCK_CPU_BITS 12 >> +#define SPINLOCK_CPU_BITS 16 >> >> #ifdef CONFIG_DEBUG_LOCKS >> union lock_debug { >> - uint16_t val; >> -#define LOCK_DEBUG_INITVAL 0xffff >> + uint32_t val; >> +#define LOCK_DEBUG_INITVAL 0xffffffff > > With this #define I can see the desire for using a fixed width type for "val". > However, ... > >> struct { >> - uint16_t cpu:SPINLOCK_CPU_BITS; >> -#define LOCK_DEBUG_PAD_BITS (14 - SPINLOCK_CPU_BITS) >> - uint16_t :LOCK_DEBUG_PAD_BITS; >> + uint32_t cpu:SPINLOCK_CPU_BITS; >> +#define LOCK_DEBUG_PAD_BITS (30 - SPINLOCK_CPU_BITS) >> + uint32_t :LOCK_DEBUG_PAD_BITS; > > .. "unsigned int" ought to be fine for both of these. Fine with me. Juergen
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c index b28239f74d..5be48be082 100644 --- a/xen/common/spinlock.c +++ b/xen/common/spinlock.c @@ -481,7 +481,9 @@ bool _rspin_trylock(rspinlock_t *lock) /* Don't allow overflow of recurse_cpu field. */ BUILD_BUG_ON(NR_CPUS > SPINLOCK_NO_CPU); + BUILD_BUG_ON(SPINLOCK_CPU_BITS > sizeof(lock->recurse_cpu) * 8); BUILD_BUG_ON(SPINLOCK_RECURSE_BITS < 3); + BUILD_BUG_ON(SPINLOCK_MAX_RECURSE > ((1u << SPINLOCK_RECURSE_BITS) - 1)); check_lock(&lock->debug, true); diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h index 1b50a7e6a0..984da6d4c9 100644 --- a/xen/include/xen/spinlock.h +++ b/xen/include/xen/spinlock.h @@ -8,16 +8,16 @@ #include <asm/system.h> #include <asm/spinlock.h> -#define SPINLOCK_CPU_BITS 12 +#define SPINLOCK_CPU_BITS 16 #ifdef CONFIG_DEBUG_LOCKS union lock_debug { - uint16_t val; -#define LOCK_DEBUG_INITVAL 0xffff + uint32_t val; +#define LOCK_DEBUG_INITVAL 0xffffffff struct { - uint16_t cpu:SPINLOCK_CPU_BITS; -#define LOCK_DEBUG_PAD_BITS (14 - SPINLOCK_CPU_BITS) - uint16_t :LOCK_DEBUG_PAD_BITS; + uint32_t cpu:SPINLOCK_CPU_BITS; +#define LOCK_DEBUG_PAD_BITS (30 - SPINLOCK_CPU_BITS) + uint32_t :LOCK_DEBUG_PAD_BITS; bool irq_safe:1; bool unseen:1; }; @@ -211,11 +211,11 @@ typedef struct spinlock { typedef struct rspinlock { spinlock_tickets_t tickets; - uint16_t recurse_cpu:SPINLOCK_CPU_BITS; + uint16_t recurse_cpu; #define SPINLOCK_NO_CPU ((1u << SPINLOCK_CPU_BITS) - 1) -#define SPINLOCK_RECURSE_BITS (16 - SPINLOCK_CPU_BITS) - uint16_t recurse_cnt:SPINLOCK_RECURSE_BITS; -#define SPINLOCK_MAX_RECURSE ((1u << SPINLOCK_RECURSE_BITS) - 1) +#define SPINLOCK_RECURSE_BITS 8 + uint8_t recurse_cnt; +#define SPINLOCK_MAX_RECURSE 15 union lock_debug debug; #ifdef CONFIG_DEBUG_LOCK_PROFILE struct lock_profile *profile;
Allow 16 bits per cpu number, which is the limit imposed by spinlock_tickets_t. This will allow up to 65535 cpus, while increasing only the size of recursive spinlocks in debug builds from 8 to 12 bytes. The current Xen limit of 4095 cpus is imposed by SPINLOCK_CPU_BITS being 12. There are machines available with more cpus than the current Xen limit, so it makes sense to have the possibility to use more cpus. Signed-off-by: Juergen Gross <jgross@suse.com> --- V5: - keep previous recursion limit (Julien Grall) --- xen/common/spinlock.c | 2 ++ xen/include/xen/spinlock.h | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 10 deletions(-)