Mailing List Archive

[PATCH v5 11/13] xen/spinlock: support higher number of cpus
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(-)

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;
--
2.35.3
Re: [PATCH v5 11/13] xen/spinlock: support higher number of cpus [ In reply to ]
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
Re: [PATCH v5 11/13] xen/spinlock: support higher number of cpus [ In reply to ]
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