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)
V6:
- use unsigned int instead of uint32_t (Jan Beulich)
---
xen/common/spinlock.c | 2 ++
xen/include/xen/spinlock.h | 20 ++++++++++----------
2 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 7ccb725171..5aa9ba6188 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -485,7 +485,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 3a4092626c..db00a24646 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;
+ unsigned int cpu:SPINLOCK_CPU_BITS;
+#define LOCK_DEBUG_PAD_BITS (30 - SPINLOCK_CPU_BITS)
+ unsigned int :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;
--
2.35.3On 27.03.2024 16:22, Juergen Gross wrote: > 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> Reviewed-by: Jan Beulich <jbeulich@suse.com> albeit I have to say that I'm not entirely convinced of ... > --- a/xen/common/spinlock.c > +++ b/xen/common/spinlock.c > @@ -485,7 +485,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); ... the two additions here: The two checks we had verify independent properties, whereas the new ones basically check that struct rspinlock and its associated #define-s were got right. We don't check such elsewhere, I don't think. Jan
On 02.04.24 16:42, Jan Beulich wrote: > On 27.03.2024 16:22, Juergen Gross wrote: >> 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> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > albeit I have to say that I'm not entirely convinced of ... > >> --- a/xen/common/spinlock.c >> +++ b/xen/common/spinlock.c >> @@ -485,7 +485,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); > > ... the two additions here: The two checks we had verify independent > properties, whereas the new ones basically check that struct rspinlock > and its associated #define-s were got right. We don't check such > elsewhere, I don't think. I think we do. What about: BUILD_BUG_ON(sizeof(hwp_req) != sizeof(hwp_req.raw)) checking that two union elements are of the same size (and both elements don't contain any other structs). Additionally it is not obvious at a first glance that SPINLOCK_CPU_BITS defined in line 11 is relevant for the definition of recurse_cpu in line 217. Regarding the second added BUILD_BUG_ON() there was a comment by Julien related to the definition of SPINLOCK_MAX_RECURSE in V4 of this patch. We settled to use the current form including the added BUILD_BUG_ON(). Juergen
© 2016 - 2026 Red Hat, Inc.