* [PATCH v2 01/13] xen/spinlock: fix coding style issues
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-18 15:47 ` Jan Beulich
2023-10-13 9:42 ` [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs Juergen Gross
` (12 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
Fix some coding style issues in spinlock coding:
- Instead of u64 etc. use the preferred uint64_t etc. types
- Remove some trailing spaces
- Add missing spaces in "if" statements
- Add some missing blank lines
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/common/spinlock.c | 20 +++++++++++---------
xen/include/xen/spinlock.h | 16 ++++++++--------
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 7f453234a9..202c707540 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -100,7 +100,7 @@ void check_lock(union lock_debug *debug, bool try)
* every lock must be consistently observed else we can deadlock in
* IRQ-context rendezvous functions (a rendezvous which gets every CPU
* into IRQ context before any CPU is released from the rendezvous).
- *
+ *
* If we can mix IRQ-disabled and IRQ-enabled callers, the following can
* happen:
* * Lock is held by CPU A, with IRQs enabled
@@ -108,7 +108,7 @@ void check_lock(union lock_debug *debug, bool try)
* * Rendezvous starts -- CPU A takes interrupt and enters rendezbous spin
* * DEADLOCK -- CPU B will never enter rendezvous, CPU A will never exit
* the rendezvous, and will hence never release the lock.
- *
+ *
* To guard against this subtle bug we latch the IRQ safety of every
* spinlock in the system, on first use.
*
@@ -151,12 +151,12 @@ static void check_barrier(union lock_debug *debug)
/*
* For a barrier, we have a relaxed IRQ-safety-consistency check.
- *
+ *
* It is always safe to spin at the barrier with IRQs enabled -- that does
* not prevent us from entering an IRQ-context rendezvous, and nor are
* we preventing anyone else from doing so (since we do not actually
* acquire the lock during a barrier operation).
- *
+ *
* However, if we spin on an IRQ-unsafe lock with IRQs disabled then that
* is clearly wrong, for the same reason outlined in check_lock() above.
*/
@@ -262,7 +262,7 @@ void spin_debug_disable(void)
#ifdef CONFIG_DEBUG_LOCK_PROFILE
#define LOCK_PROFILE_REL \
- if (lock->profile) \
+ if ( lock->profile ) \
{ \
lock->profile->time_hold += NOW() - lock->profile->time_locked; \
lock->profile->lock_cnt++; \
@@ -270,10 +270,10 @@ void spin_debug_disable(void)
#define LOCK_PROFILE_VAR s_time_t block = 0
#define LOCK_PROFILE_BLOCK block = block ? : NOW();
#define LOCK_PROFILE_GOT \
- if (lock->profile) \
+ if ( lock->profile ) \
{ \
lock->profile->time_locked = NOW(); \
- if (block) \
+ if ( block ) \
{ \
lock->profile->time_block += lock->profile->time_locked - block; \
lock->profile->block_cnt++; \
@@ -298,7 +298,7 @@ static always_inline spinlock_tickets_t observe_lock(spinlock_tickets_t *t)
return v;
}
-static always_inline u16 observe_head(spinlock_tickets_t *t)
+static always_inline uint16_t observe_head(spinlock_tickets_t *t)
{
smp_rmb();
return read_atomic(&t->head);
@@ -412,7 +412,7 @@ int _spin_trylock(spinlock_t *lock)
*/
got_lock(&lock->debug);
#ifdef CONFIG_DEBUG_LOCK_PROFILE
- if (lock->profile)
+ if ( lock->profile )
lock->profile->time_locked = NOW();
#endif
return 1;
@@ -616,6 +616,7 @@ int spinlock_profile_control(struct xen_sysctl_lockprof_op *pc)
case XEN_SYSCTL_LOCKPROF_reset:
spinlock_profile_reset('\0');
break;
+
case XEN_SYSCTL_LOCKPROF_query:
pc->nr_elem = 0;
par.rc = 0;
@@ -624,6 +625,7 @@ int spinlock_profile_control(struct xen_sysctl_lockprof_op *pc)
pc->time = NOW() - lock_profile_start;
rc = par.rc;
break;
+
default:
rc = -EINVAL;
break;
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index e7a1c1aa89..4b82739083 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -81,8 +81,8 @@ struct lock_profile {
struct lock_profile *next; /* forward link */
const char *name; /* lock name */
struct spinlock *lock; /* the lock itself */
- u64 lock_cnt; /* # of complete locking ops */
- u64 block_cnt; /* # of complete wait for lock */
+ uint64_t lock_cnt; /* # of complete locking ops */
+ uint64_t block_cnt; /* # of complete wait for lock */
s_time_t time_hold; /* cumulated lock time */
s_time_t time_block; /* cumulated wait time */
s_time_t time_locked; /* system time of last locking */
@@ -122,7 +122,7 @@ struct lock_profile_qhead {
prof->lock = &(s)->l; \
prof->next = (s)->profile_head.elem_q; \
(s)->profile_head.elem_q = prof; \
- } while(0)
+ } while( 0 )
void _lock_profile_register_struct(
int32_t type, struct lock_profile_qhead *qhead, int32_t idx);
@@ -153,10 +153,10 @@ struct lock_profile_qhead { };
#endif
typedef union {
- u32 head_tail;
+ uint32_t head_tail;
struct {
- u16 head;
- u16 tail;
+ uint16_t head;
+ uint16_t tail;
};
} spinlock_tickets_t;
@@ -164,10 +164,10 @@ typedef union {
typedef struct spinlock {
spinlock_tickets_t tickets;
- u16 recurse_cpu:SPINLOCK_CPU_BITS;
+ uint16_t recurse_cpu:SPINLOCK_CPU_BITS;
#define SPINLOCK_NO_CPU ((1u << SPINLOCK_CPU_BITS) - 1)
#define SPINLOCK_RECURSE_BITS (16 - SPINLOCK_CPU_BITS)
- u16 recurse_cnt:SPINLOCK_RECURSE_BITS;
+ uint16_t recurse_cnt:SPINLOCK_RECURSE_BITS;
#define SPINLOCK_MAX_RECURSE ((1u << SPINLOCK_RECURSE_BITS) - 1)
union lock_debug debug;
#ifdef CONFIG_DEBUG_LOCK_PROFILE
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 01/13] xen/spinlock: fix coding style issues
2023-10-13 9:42 ` [PATCH v2 01/13] xen/spinlock: fix coding style issues Juergen Gross
@ 2023-10-18 15:47 ` Jan Beulich
2023-10-18 15:52 ` Juergen Gross
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2023-10-18 15:47 UTC (permalink / raw)
To: Juergen Gross
Cc: javi.merino, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
On 13.10.2023 11:42, Juergen Gross wrote:
> Fix some coding style issues in spinlock coding:
>
> - Instead of u64 etc. use the preferred uint64_t etc. types
> - Remove some trailing spaces
> - Add missing spaces in "if" statements
> - Add some missing blank lines
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
with one possible further request:
> @@ -298,7 +298,7 @@ static always_inline spinlock_tickets_t observe_lock(spinlock_tickets_t *t)
> return v;
> }
>
> -static always_inline u16 observe_head(spinlock_tickets_t *t)
> +static always_inline uint16_t observe_head(spinlock_tickets_t *t)
While there, could you also add const to the pointed-to parameter type?
t, after all, ...
> {
> smp_rmb();
> return read_atomic(&t->head);
... is only read through. (I'd also be happy for this to be adjusted on
commit, so long as you agree.)
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 01/13] xen/spinlock: fix coding style issues
2023-10-18 15:47 ` Jan Beulich
@ 2023-10-18 15:52 ` Juergen Gross
0 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-18 15:52 UTC (permalink / raw)
To: Jan Beulich
Cc: javi.merino, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 1047 bytes --]
On 18.10.23 17:47, Jan Beulich wrote:
> On 13.10.2023 11:42, Juergen Gross wrote:
>> Fix some coding style issues in spinlock coding:
>>
>> - Instead of u64 etc. use the preferred uint64_t etc. types
>> - Remove some trailing spaces
>> - Add missing spaces in "if" statements
>> - Add some missing blank lines
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> with one possible further request:
>
>> @@ -298,7 +298,7 @@ static always_inline spinlock_tickets_t observe_lock(spinlock_tickets_t *t)
>> return v;
>> }
>>
>> -static always_inline u16 observe_head(spinlock_tickets_t *t)
>> +static always_inline uint16_t observe_head(spinlock_tickets_t *t)
>
> While there, could you also add const to the pointed-to parameter type?
> t, after all, ...
>
>> {
>> smp_rmb();
>> return read_atomic(&t->head);
>
> ... is only read through. (I'd also be happy for this to be adjusted on
> commit, so long as you agree.)
I agree.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
2023-10-13 9:42 ` [PATCH v2 01/13] xen/spinlock: fix coding style issues Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-18 15:57 ` Jan Beulich
2023-10-13 9:42 ` [PATCH v2 03/13] xen/spinlock: make spinlock initializers more readable Juergen Gross
` (11 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs
can be dropped from spinlock.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/common/spinlock.c | 45 ++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 26 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 202c707540..4878a01302 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -267,25 +267,28 @@ void spin_debug_disable(void)
lock->profile->time_hold += NOW() - lock->profile->time_locked; \
lock->profile->lock_cnt++; \
}
-#define LOCK_PROFILE_VAR s_time_t block = 0
-#define LOCK_PROFILE_BLOCK block = block ? : NOW();
-#define LOCK_PROFILE_GOT \
+#define LOCK_PROFILE_VAR(val) s_time_t block = (val)
+#define LOCK_PROFILE_BLOCK block = block ? : NOW()
+#define LOCK_PROFILE_BLKACC(tst, val) \
+ if ( tst ) \
+ { \
+ lock->profile->time_block += lock->profile->time_locked - (val); \
+ lock->profile->block_cnt++; \
+ }
+#define LOCK_PROFILE_GOT(val) \
if ( lock->profile ) \
{ \
lock->profile->time_locked = NOW(); \
- if ( block ) \
- { \
- lock->profile->time_block += lock->profile->time_locked - block; \
- lock->profile->block_cnt++; \
- } \
+ LOCK_PROFILE_BLKACC(val, val); \
}
#else
#define LOCK_PROFILE_REL
-#define LOCK_PROFILE_VAR
+#define LOCK_PROFILE_VAR(val)
#define LOCK_PROFILE_BLOCK
-#define LOCK_PROFILE_GOT
+#define LOCK_PROFILE_BLKACC(tst, val)
+#define LOCK_PROFILE_GOT(val)
#endif
@@ -308,7 +311,7 @@ static void always_inline spin_lock_common(spinlock_t *lock,
void (*cb)(void *), void *data)
{
spinlock_tickets_t tickets = SPINLOCK_TICKET_INC;
- LOCK_PROFILE_VAR;
+ LOCK_PROFILE_VAR(0);
check_lock(&lock->debug, false);
preempt_disable();
@@ -323,7 +326,7 @@ static void always_inline spin_lock_common(spinlock_t *lock,
}
arch_lock_acquire_barrier();
got_lock(&lock->debug);
- LOCK_PROFILE_GOT;
+ LOCK_PROFILE_GOT(block);
}
void _spin_lock(spinlock_t *lock)
@@ -411,19 +414,15 @@ int _spin_trylock(spinlock_t *lock)
* arch_lock_acquire_barrier().
*/
got_lock(&lock->debug);
-#ifdef CONFIG_DEBUG_LOCK_PROFILE
- if ( lock->profile )
- lock->profile->time_locked = NOW();
-#endif
+ LOCK_PROFILE_GOT(0);
+
return 1;
}
void _spin_barrier(spinlock_t *lock)
{
spinlock_tickets_t sample;
-#ifdef CONFIG_DEBUG_LOCK_PROFILE
- s_time_t block = NOW();
-#endif
+ LOCK_PROFILE_VAR(NOW());
check_barrier(&lock->debug);
smp_mb();
@@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock)
{
while ( observe_head(&lock->tickets) == sample.head )
arch_lock_relax();
-#ifdef CONFIG_DEBUG_LOCK_PROFILE
- if ( lock->profile )
- {
- lock->profile->time_block += NOW() - block;
- lock->profile->block_cnt++;
- }
-#endif
+ LOCK_PROFILE_BLKACC(lock->profile, block);
}
smp_mb();
}
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs
2023-10-13 9:42 ` [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs Juergen Gross
@ 2023-10-18 15:57 ` Jan Beulich
2023-10-19 9:05 ` Juergen Gross
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2023-10-18 15:57 UTC (permalink / raw)
To: Juergen Gross
Cc: javi.merino, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
On 13.10.2023 11:42, Juergen Gross wrote:
> With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs
> can be dropped from spinlock.c.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V2:
> - new patch
> ---
> xen/common/spinlock.c | 45 ++++++++++++++++++-------------------------
> 1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
> index 202c707540..4878a01302 100644
> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -267,25 +267,28 @@ void spin_debug_disable(void)
> lock->profile->time_hold += NOW() - lock->profile->time_locked; \
> lock->profile->lock_cnt++; \
> }
> -#define LOCK_PROFILE_VAR s_time_t block = 0
> -#define LOCK_PROFILE_BLOCK block = block ? : NOW();
> -#define LOCK_PROFILE_GOT \
> +#define LOCK_PROFILE_VAR(val) s_time_t block = (val)
This macro, and then at least for consistency also ...
> +#define LOCK_PROFILE_BLOCK block = block ? : NOW()
... this one should imo take the variable name as an argument. Otherwise
situations like ...
> void _spin_barrier(spinlock_t *lock)
> {
> spinlock_tickets_t sample;
> -#ifdef CONFIG_DEBUG_LOCK_PROFILE
> - s_time_t block = NOW();
> -#endif
> + LOCK_PROFILE_VAR(NOW());
>
> check_barrier(&lock->debug);
> smp_mb();
> @@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock)
> {
> while ( observe_head(&lock->tickets) == sample.head )
> arch_lock_relax();
> -#ifdef CONFIG_DEBUG_LOCK_PROFILE
> - if ( lock->profile )
> - {
> - lock->profile->time_block += NOW() - block;
> - lock->profile->block_cnt++;
> - }
> -#endif
> + LOCK_PROFILE_BLKACC(lock->profile, block);
> }
> smp_mb();
> }
... this arise where there's no visible declaration of "block", but a
use. (Originally I was meaning to ask how this function would build,
when the declaration is dropped.)
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs
2023-10-18 15:57 ` Jan Beulich
@ 2023-10-19 9:05 ` Juergen Gross
0 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-19 9:05 UTC (permalink / raw)
To: Jan Beulich
Cc: javi.merino, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 2292 bytes --]
On 18.10.23 17:57, Jan Beulich wrote:
> On 13.10.2023 11:42, Juergen Gross wrote:
>> With some small adjustments to the LOCK_PROFILE_* macros some #ifdefs
>> can be dropped from spinlock.c.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V2:
>> - new patch
>> ---
>> xen/common/spinlock.c | 45 ++++++++++++++++++-------------------------
>> 1 file changed, 19 insertions(+), 26 deletions(-)
>>
>> diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
>> index 202c707540..4878a01302 100644
>> --- a/xen/common/spinlock.c
>> +++ b/xen/common/spinlock.c
>> @@ -267,25 +267,28 @@ void spin_debug_disable(void)
>> lock->profile->time_hold += NOW() - lock->profile->time_locked; \
>> lock->profile->lock_cnt++; \
>> }
>> -#define LOCK_PROFILE_VAR s_time_t block = 0
>> -#define LOCK_PROFILE_BLOCK block = block ? : NOW();
>> -#define LOCK_PROFILE_GOT \
>> +#define LOCK_PROFILE_VAR(val) s_time_t block = (val)
>
> This macro, and then at least for consistency also ...
>
>> +#define LOCK_PROFILE_BLOCK block = block ? : NOW()
>
> ... this one should imo take the variable name as an argument. Otherwise
> situations like ...
>
>> void _spin_barrier(spinlock_t *lock)
>> {
>> spinlock_tickets_t sample;
>> -#ifdef CONFIG_DEBUG_LOCK_PROFILE
>> - s_time_t block = NOW();
>> -#endif
>> + LOCK_PROFILE_VAR(NOW());
>>
>> check_barrier(&lock->debug);
>> smp_mb();
>> @@ -432,13 +431,7 @@ void _spin_barrier(spinlock_t *lock)
>> {
>> while ( observe_head(&lock->tickets) == sample.head )
>> arch_lock_relax();
>> -#ifdef CONFIG_DEBUG_LOCK_PROFILE
>> - if ( lock->profile )
>> - {
>> - lock->profile->time_block += NOW() - block;
>> - lock->profile->block_cnt++;
>> - }
>> -#endif
>> + LOCK_PROFILE_BLKACC(lock->profile, block);
>> }
>> smp_mb();
>> }
>
> ... this arise where there's no visible declaration of "block", but a
> use. (Originally I was meaning to ask how this function would build,
> when the declaration is dropped.)
Okay.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v2 03/13] xen/spinlock: make spinlock initializers more readable
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
2023-10-13 9:42 ` [PATCH v2 01/13] xen/spinlock: fix coding style issues Juergen Gross
2023-10-13 9:42 ` [PATCH v2 02/13] xen/spinlock: reduce lock profile ifdefs Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-18 16:00 ` Jan Beulich
2023-10-13 9:42 ` [PATCH v2 04/13] xen/spinlock: introduce new type for recursive spinlocks Juergen Gross
` (10 subsequent siblings)
13 siblings, 1 reply; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
Use named member initializers instead of positional ones for the macros
used to initialize structures.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/include/xen/spinlock.h | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 4b82739083..bbe1472571 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -21,7 +21,7 @@ union lock_debug {
bool unseen:1;
};
};
-#define _LOCK_DEBUG { LOCK_DEBUG_INITVAL }
+#define _LOCK_DEBUG { .val = LOCK_DEBUG_INITVAL }
void check_lock(union lock_debug *debug, bool try);
void lock_enter(const union lock_debug *debug);
void lock_exit(const union lock_debug *debug);
@@ -94,12 +94,16 @@ struct lock_profile_qhead {
int32_t idx; /* index for printout */
};
-#define _LOCK_PROFILE(name) { 0, #name, &name, 0, 0, 0, 0, 0 }
+#define _LOCK_PROFILE(lockname) { .name = #lockname, .lock = &lockname, }
#define _LOCK_PROFILE_PTR(name) \
static struct lock_profile * const __lock_profile_##name \
__used_section(".lockprofile.data") = \
&__lock_profile_data_##name
-#define _SPIN_LOCK_UNLOCKED(x) { { 0 }, SPINLOCK_NO_CPU, 0, _LOCK_DEBUG, x }
+#define _SPIN_LOCK_UNLOCKED(x) { \
+ .recurse_cpu = SPINLOCK_NO_CPU, \
+ .debug =_LOCK_DEBUG, \
+ .profile = x, \
+}
#define SPIN_LOCK_UNLOCKED _SPIN_LOCK_UNLOCKED(NULL)
#define DEFINE_SPINLOCK(l) \
spinlock_t l = _SPIN_LOCK_UNLOCKED(NULL); \
@@ -142,7 +146,10 @@ extern void cf_check spinlock_profile_reset(unsigned char key);
struct lock_profile_qhead { };
-#define SPIN_LOCK_UNLOCKED { { 0 }, SPINLOCK_NO_CPU, 0, _LOCK_DEBUG }
+#define SPIN_LOCK_UNLOCKED { \
+ .recurse_cpu = SPINLOCK_NO_CPU, \
+ .debug =_LOCK_DEBUG, \
+}
#define DEFINE_SPINLOCK(l) spinlock_t l = SPIN_LOCK_UNLOCKED
#define spin_lock_init_prof(s, l) spin_lock_init(&((s)->l))
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 04/13] xen/spinlock: introduce new type for recursive spinlocks
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (2 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 03/13] xen/spinlock: make spinlock initializers more readable Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 05/13] xen/spinlock: rename recursive lock functions Juergen Gross
` (9 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Wei Liu, George Dunlap, Julien Grall,
Stefano Stabellini, Paul Durrant
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@suse.com>
---
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;
int unlock_level;
int locker; /* processor which holds the lock */
const char *locker_function; /* func that took it */
diff --git a/xen/arch/x86/mm/mm-locks.h b/xen/arch/x86/mm/mm-locks.h
index 5a3f96fbaa..b2c0467024 100644
--- a/xen/arch/x86/mm/mm-locks.h
+++ b/xen/arch/x86/mm/mm-locks.h
@@ -20,7 +20,7 @@ DECLARE_PERCPU_RWLOCK_GLOBAL(p2m_percpu_rwlock);
static inline void mm_lock_init(mm_lock_t *l)
{
- spin_lock_init(&l->lock);
+ rspin_lock_init(&l->lock);
l->locker = -1;
l->locker_function = "nobody";
l->unlock_level = 0;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 8f9ab01c0c..604f70ff5a 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -627,8 +627,8 @@ struct domain *domain_create(domid_t domid,
atomic_set(&d->refcnt, 1);
RCU_READ_LOCK_INIT(&d->rcu_lock);
- spin_lock_init_prof(d, domain_lock);
- spin_lock_init_prof(d, page_alloc_lock);
+ rspin_lock_init_prof(d, domain_lock);
+ rspin_lock_init_prof(d, page_alloc_lock);
spin_lock_init(&d->hypercall_deadlock_mutex);
INIT_PAGE_LIST_HEAD(&d->page_list);
INIT_PAGE_LIST_HEAD(&d->extra_page_list);
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 62b907f4c4..652c18a9b5 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -1331,7 +1331,7 @@ unsigned int ioreq_broadcast(ioreq_t *p, bool buffered)
void ioreq_domain_init(struct domain *d)
{
- spin_lock_init(&d->ioreq_server.lock);
+ rspin_lock_init(&d->ioreq_server.lock);
arch_ioreq_domain_init(d);
}
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index f81b8b6b47..826f45b922 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -120,7 +120,7 @@ static int __read_mostly sercon_handle = -1;
int8_t __read_mostly opt_console_xen; /* console=xen */
#endif
-static DEFINE_SPINLOCK(console_lock);
+static DEFINE_RSPINLOCK(console_lock);
/*
* To control the amount of printing, thresholds are added.
@@ -1169,7 +1169,7 @@ void console_force_unlock(void)
{
watchdog_disable();
spin_debug_disable();
- spin_lock_init(&console_lock);
+ rspin_lock_init(&console_lock);
serial_force_unlock(sercon_handle);
console_locks_busted = 1;
console_start_sync();
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 04d00c7c37..61be34e75f 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -50,7 +50,7 @@ struct pci_seg {
} bus2bridge[MAX_BUSES];
};
-static spinlock_t _pcidevs_lock = SPIN_LOCK_UNLOCKED;
+static DEFINE_RSPINLOCK(_pcidevs_lock);
void pcidevs_lock(void)
{
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 3609ef88c4..c6604aef78 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -376,9 +376,9 @@ struct domain
rcu_read_lock_t rcu_lock;
- spinlock_t domain_lock;
+ rspinlock_t domain_lock;
- spinlock_t page_alloc_lock; /* protects all the following fields */
+ rspinlock_t page_alloc_lock; /* protects all the following fields */
struct page_list_head page_list; /* linked list */
struct page_list_head extra_page_list; /* linked list (size extra_pages) */
struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
@@ -597,7 +597,7 @@ struct domain
#ifdef CONFIG_IOREQ_SERVER
/* Lock protects all other values in the sub-struct */
struct {
- spinlock_t lock;
+ rspinlock_t lock;
struct ioreq_server *server[MAX_NR_IOREQ_SERVERS];
} ioreq_server;
#endif
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
@@ -45,7 +45,7 @@ union lock_debug { };
lock profiling on:
Global locks which should be subject to profiling must be declared via
- DEFINE_SPINLOCK.
+ DEFINE_[R]SPINLOCK.
For locks in structures further measures are necessary:
- the structure definition must include a profile_head with exactly this
@@ -56,7 +56,7 @@ union lock_debug { };
- the single locks which are subject to profiling have to be initialized
via
- spin_lock_init_prof(ptr, lock);
+ [r]spin_lock_init_prof(ptr, lock);
with ptr being the main structure pointer and lock the spinlock field
@@ -109,12 +109,16 @@ struct lock_profile_qhead {
spinlock_t l = _SPIN_LOCK_UNLOCKED(NULL); \
static struct lock_profile __lock_profile_data_##l = _LOCK_PROFILE(l); \
_LOCK_PROFILE_PTR(l)
+#define DEFINE_RSPINLOCK(l) \
+ rspinlock_t l = _SPIN_LOCK_UNLOCKED(NULL); \
+ static struct lock_profile __lock_profile_data_##l = _LOCK_PROFILE(l); \
+ _LOCK_PROFILE_PTR(l)
-#define spin_lock_init_prof(s, l) \
+#define __spin_lock_init_prof(s, l, locktype) \
do { \
struct lock_profile *prof; \
prof = xzalloc(struct lock_profile); \
- (s)->l = (spinlock_t)_SPIN_LOCK_UNLOCKED(prof); \
+ (s)->l = (locktype)_SPIN_LOCK_UNLOCKED(prof); \
if ( !prof ) \
{ \
printk(XENLOG_WARNING \
@@ -128,6 +132,9 @@ struct lock_profile_qhead {
(s)->profile_head.elem_q = prof; \
} while( 0 )
+#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)
+
void _lock_profile_register_struct(
int32_t type, struct lock_profile_qhead *qhead, int32_t idx);
void _lock_profile_deregister_struct(int32_t type,
@@ -151,8 +158,10 @@ struct lock_profile_qhead { };
.debug =_LOCK_DEBUG, \
}
#define DEFINE_SPINLOCK(l) spinlock_t l = SPIN_LOCK_UNLOCKED
+#define DEFINE_RSPINLOCK(l) rspinlock_t l = SPIN_LOCK_UNLOCKED
#define spin_lock_init_prof(s, l) spin_lock_init(&((s)->l))
+#define rspin_lock_init_prof(s, l) rspin_lock_init(&((s)->l))
#define lock_profile_register_struct(type, ptr, idx)
#define lock_profile_deregister_struct(type, ptr)
#define spinlock_profile_printall(key)
@@ -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)
void _spin_lock(spinlock_t *lock);
void _spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data);
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 05/13] xen/spinlock: rename recursive lock functions
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (3 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 04/13] xen/spinlock: introduce new type for recursive spinlocks Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 06/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]() Juergen Gross
` (8 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper, George Dunlap,
Jan Beulich, Wei Liu, Roger Pau Monné, Tamas K Lengyel,
Paul Durrant
Rename the recursive spin_lock() functions by replacing the trailing
"_recursive" with a leading "r".
Switch the parameter to be a pointer to rspinlock_t.
Remove the indirection through a macro, as it is adding only complexity
without any gain.
Suggested-by: Jan Beulich <jbeulich.suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/arch/arm/domain.c | 4 +--
xen/arch/x86/domain.c | 8 +++---
xen/arch/x86/mm/mem_sharing.c | 8 +++---
xen/arch/x86/mm/mm-locks.h | 4 +--
xen/common/ioreq.c | 52 +++++++++++++++++------------------
xen/common/page_alloc.c | 12 ++++----
xen/common/spinlock.c | 6 ++--
xen/drivers/char/console.c | 12 ++++----
xen/drivers/passthrough/pci.c | 4 +--
xen/include/xen/sched.h | 4 +--
xen/include/xen/spinlock.h | 24 +++++++---------
11 files changed, 67 insertions(+), 71 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 28e3aaa5e4..603a5f7c81 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -996,7 +996,7 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list)
int ret = 0;
/* Use a recursive lock, as we may enter 'free_domheap_page'. */
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
page_list_for_each_safe( page, tmp, list )
{
@@ -1023,7 +1023,7 @@ static int relinquish_memory(struct domain *d, struct page_list_head *list)
}
out:
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
return ret;
}
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index d05ee0da55..57dd5deeb5 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1314,7 +1314,7 @@ int arch_set_info_guest(
{
bool done = false;
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
for ( i = 0; ; )
{
@@ -1335,7 +1335,7 @@ int arch_set_info_guest(
break;
}
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
if ( !done )
return -ERESTART;
@@ -2174,7 +2174,7 @@ static int relinquish_memory(
int ret = 0;
/* Use a recursive lock, as we may enter 'free_domheap_page'. */
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
while ( (page = page_list_remove_head(list)) )
{
@@ -2315,7 +2315,7 @@ static int relinquish_memory(
page_list_move(list, &d->arch.relmem_list);
out:
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
return ret;
}
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 94b6b782ef..146f61f354 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -688,7 +688,7 @@ static int page_make_sharable(struct domain *d,
int rc = 0;
bool drop_dom_ref = false;
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
if ( d->is_dying )
{
@@ -731,7 +731,7 @@ static int page_make_sharable(struct domain *d,
}
out:
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
if ( drop_dom_ref )
put_domain(d);
@@ -1940,7 +1940,7 @@ int mem_sharing_fork_reset(struct domain *d, bool reset_state,
goto state;
/* need recursive lock because we will free pages */
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
page_list_for_each_safe(page, tmp, &d->page_list)
{
shr_handle_t sh;
@@ -1969,7 +1969,7 @@ int mem_sharing_fork_reset(struct domain *d, bool reset_state,
put_page_alloc_ref(page);
put_page_and_type(page);
}
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
state:
if ( reset_state )
diff --git a/xen/arch/x86/mm/mm-locks.h b/xen/arch/x86/mm/mm-locks.h
index b2c0467024..3779c552a6 100644
--- a/xen/arch/x86/mm/mm-locks.h
+++ b/xen/arch/x86/mm/mm-locks.h
@@ -79,7 +79,7 @@ static inline void _mm_lock(const struct domain *d, mm_lock_t *l,
{
if ( !((mm_locked_by_me(l)) && rec) )
_check_lock_level(d, level);
- spin_lock_recursive(&l->lock);
+ rspin_lock(&l->lock);
if ( l->lock.recurse_cnt == 1 )
{
l->locker_function = func;
@@ -200,7 +200,7 @@ static inline void mm_unlock(mm_lock_t *l)
l->locker_function = "nobody";
_set_lock_level(l->unlock_level);
}
- spin_unlock_recursive(&l->lock);
+ rspin_unlock(&l->lock);
}
static inline void mm_enforce_order_unlock(int unlock_level,
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 652c18a9b5..1257a3d972 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -329,7 +329,7 @@ bool is_ioreq_server_page(struct domain *d, const struct page_info *page)
unsigned int id;
bool found = false;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
FOR_EACH_IOREQ_SERVER(d, id, s)
{
@@ -340,7 +340,7 @@ bool is_ioreq_server_page(struct domain *d, const struct page_info *page)
}
}
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return found;
}
@@ -658,7 +658,7 @@ static int ioreq_server_create(struct domain *d, int bufioreq_handling,
return -ENOMEM;
domain_pause(d);
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
for ( i = 0; i < MAX_NR_IOREQ_SERVERS; i++ )
{
@@ -686,13 +686,13 @@ static int ioreq_server_create(struct domain *d, int bufioreq_handling,
if ( id )
*id = i;
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
domain_unpause(d);
return 0;
fail:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
domain_unpause(d);
xfree(s);
@@ -704,7 +704,7 @@ static int ioreq_server_destroy(struct domain *d, ioservid_t id)
struct ioreq_server *s;
int rc;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -736,7 +736,7 @@ static int ioreq_server_destroy(struct domain *d, ioservid_t id)
rc = 0;
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -749,7 +749,7 @@ static int ioreq_server_get_info(struct domain *d, ioservid_t id,
struct ioreq_server *s;
int rc;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -783,7 +783,7 @@ static int ioreq_server_get_info(struct domain *d, ioservid_t id,
rc = 0;
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -796,7 +796,7 @@ int ioreq_server_get_frame(struct domain *d, ioservid_t id,
ASSERT(is_hvm_domain(d));
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -834,7 +834,7 @@ int ioreq_server_get_frame(struct domain *d, ioservid_t id,
}
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -850,7 +850,7 @@ static int ioreq_server_map_io_range(struct domain *d, ioservid_t id,
if ( start > end )
return -EINVAL;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -886,7 +886,7 @@ static int ioreq_server_map_io_range(struct domain *d, ioservid_t id,
rc = rangeset_add_range(r, start, end);
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -902,7 +902,7 @@ static int ioreq_server_unmap_io_range(struct domain *d, ioservid_t id,
if ( start > end )
return -EINVAL;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -938,7 +938,7 @@ static int ioreq_server_unmap_io_range(struct domain *d, ioservid_t id,
rc = rangeset_remove_range(r, start, end);
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -963,7 +963,7 @@ int ioreq_server_map_mem_type(struct domain *d, ioservid_t id,
if ( flags & ~XEN_DMOP_IOREQ_MEM_ACCESS_WRITE )
return -EINVAL;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -978,7 +978,7 @@ int ioreq_server_map_mem_type(struct domain *d, ioservid_t id,
rc = arch_ioreq_server_map_mem_type(d, s, flags);
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
if ( rc == 0 )
arch_ioreq_server_map_mem_type_completed(d, s, flags);
@@ -992,7 +992,7 @@ static int ioreq_server_set_state(struct domain *d, ioservid_t id,
struct ioreq_server *s;
int rc;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
s = get_ioreq_server(d, id);
@@ -1016,7 +1016,7 @@ static int ioreq_server_set_state(struct domain *d, ioservid_t id,
rc = 0;
out:
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -1026,7 +1026,7 @@ int ioreq_server_add_vcpu_all(struct domain *d, struct vcpu *v)
unsigned int id;
int rc;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
FOR_EACH_IOREQ_SERVER(d, id, s)
{
@@ -1035,7 +1035,7 @@ int ioreq_server_add_vcpu_all(struct domain *d, struct vcpu *v)
goto fail;
}
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return 0;
@@ -1050,7 +1050,7 @@ int ioreq_server_add_vcpu_all(struct domain *d, struct vcpu *v)
ioreq_server_remove_vcpu(s, v);
}
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
return rc;
}
@@ -1060,12 +1060,12 @@ void ioreq_server_remove_vcpu_all(struct domain *d, struct vcpu *v)
struct ioreq_server *s;
unsigned int id;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
FOR_EACH_IOREQ_SERVER(d, id, s)
ioreq_server_remove_vcpu(s, v);
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
}
void ioreq_server_destroy_all(struct domain *d)
@@ -1076,7 +1076,7 @@ void ioreq_server_destroy_all(struct domain *d)
if ( !arch_ioreq_server_destroy_all(d) )
return;
- spin_lock_recursive(&d->ioreq_server.lock);
+ rspin_lock(&d->ioreq_server.lock);
/* No need to domain_pause() as the domain is being torn down */
@@ -1094,7 +1094,7 @@ void ioreq_server_destroy_all(struct domain *d)
xfree(s);
}
- spin_unlock_recursive(&d->ioreq_server.lock);
+ rspin_unlock(&d->ioreq_server.lock);
}
struct ioreq_server *ioreq_server_select(struct domain *d,
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 9b5df74fdd..8c6a3d9274 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2497,7 +2497,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
if ( unlikely(is_xen_heap_page(pg)) )
{
/* NB. May recursively lock from relinquish_memory(). */
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
for ( i = 0; i < (1 << order); i++ )
arch_free_heap_page(d, &pg[i]);
@@ -2505,7 +2505,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
d->xenheap_pages -= 1 << order;
drop_dom_ref = (d->xenheap_pages == 0);
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
}
else
{
@@ -2514,7 +2514,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
if ( likely(d) && likely(d != dom_cow) )
{
/* NB. May recursively lock from relinquish_memory(). */
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
for ( i = 0; i < (1 << order); i++ )
{
@@ -2537,7 +2537,7 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
drop_dom_ref = !domain_adjust_tot_pages(d, -(1 << order));
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
/*
* Normally we expect a domain to clear pages before freeing them,
@@ -2753,7 +2753,7 @@ void free_domstatic_page(struct page_info *page)
ASSERT_ALLOC_CONTEXT();
/* NB. May recursively lock from relinquish_memory(). */
- spin_lock_recursive(&d->page_alloc_lock);
+ rspin_lock(&d->page_alloc_lock);
arch_free_heap_page(d, page);
@@ -2764,7 +2764,7 @@ void free_domstatic_page(struct page_info *page)
/* Add page on the resv_page_list *after* it has been freed. */
page_list_add_tail(page, &d->resv_page_list);
- spin_unlock_recursive(&d->page_alloc_lock);
+ rspin_unlock(&d->page_alloc_lock);
if ( drop_dom_ref )
put_domain(d);
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 4878a01302..58d19ea12c 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -436,7 +436,7 @@ void _spin_barrier(spinlock_t *lock)
smp_mb();
}
-int _spin_trylock_recursive(spinlock_t *lock)
+int rspin_trylock(rspinlock_t *lock)
{
unsigned int cpu = smp_processor_id();
@@ -460,7 +460,7 @@ int _spin_trylock_recursive(spinlock_t *lock)
return 1;
}
-void _spin_lock_recursive(spinlock_t *lock)
+void rspin_lock(rspinlock_t *lock)
{
unsigned int cpu = smp_processor_id();
@@ -475,7 +475,7 @@ void _spin_lock_recursive(spinlock_t *lock)
lock->recurse_cnt++;
}
-void _spin_unlock_recursive(spinlock_t *lock)
+void rspin_unlock(rspinlock_t *lock)
{
if ( likely(--lock->recurse_cnt == 0) )
{
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 826f45b922..394c01d097 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -911,7 +911,7 @@ static void vprintk_common(const char *prefix, const char *fmt, va_list args)
/* console_lock can be acquired recursively from __printk_ratelimit(). */
local_irq_save(flags);
- spin_lock_recursive(&console_lock);
+ rspin_lock(&console_lock);
state = &this_cpu(state);
(void)vsnprintf(buf, sizeof(buf), fmt, args);
@@ -947,7 +947,7 @@ static void vprintk_common(const char *prefix, const char *fmt, va_list args)
state->continued = 1;
}
- spin_unlock_recursive(&console_lock);
+ rspin_unlock(&console_lock);
local_irq_restore(flags);
}
@@ -1154,14 +1154,14 @@ unsigned long console_lock_recursive_irqsave(void)
unsigned long flags;
local_irq_save(flags);
- spin_lock_recursive(&console_lock);
+ rspin_lock(&console_lock);
return flags;
}
void console_unlock_recursive_irqrestore(unsigned long flags)
{
- spin_unlock_recursive(&console_lock);
+ rspin_unlock(&console_lock);
local_irq_restore(flags);
}
@@ -1222,12 +1222,12 @@ int __printk_ratelimit(int ratelimit_ms, int ratelimit_burst)
char lost_str[8];
snprintf(lost_str, sizeof(lost_str), "%d", lost);
/* console_lock may already be acquired by printk(). */
- spin_lock_recursive(&console_lock);
+ rspin_lock(&console_lock);
printk_start_of_line("(XEN) ");
__putstr("printk: ");
__putstr(lost_str);
__putstr(" messages suppressed.\n");
- spin_unlock_recursive(&console_lock);
+ rspin_unlock(&console_lock);
}
local_irq_restore(flags);
return 1;
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 61be34e75f..22342f07ac 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -54,12 +54,12 @@ static DEFINE_RSPINLOCK(_pcidevs_lock);
void pcidevs_lock(void)
{
- spin_lock_recursive(&_pcidevs_lock);
+ rspin_lock(&_pcidevs_lock);
}
void pcidevs_unlock(void)
{
- spin_unlock_recursive(&_pcidevs_lock);
+ rspin_unlock(&_pcidevs_lock);
}
bool pcidevs_locked(void)
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index c6604aef78..8cf751ad0c 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -358,8 +358,8 @@ struct sched_unit {
(v) = (v)->next_in_list )
/* Per-domain lock can be recursively acquired in fault handlers. */
-#define domain_lock(d) spin_lock_recursive(&(d)->domain_lock)
-#define domain_unlock(d) spin_unlock_recursive(&(d)->domain_lock)
+#define domain_lock(d) rspin_lock(&(d)->domain_lock)
+#define domain_unlock(d) rspin_unlock(&(d)->domain_lock)
struct evtchn_port_ops;
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 19561d5e61..c99ee52458 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -209,9 +209,16 @@ int _spin_is_locked(const spinlock_t *lock);
int _spin_trylock(spinlock_t *lock);
void _spin_barrier(spinlock_t *lock);
-int _spin_trylock_recursive(spinlock_t *lock);
-void _spin_lock_recursive(spinlock_t *lock);
-void _spin_unlock_recursive(spinlock_t *lock);
+/*
+ * rspin_[un]lock(): Use these forms when the lock can (safely!) be
+ * reentered recursively on the same CPU. All critical regions that may form
+ * part of a recursively-nested set must be protected by these forms. If there
+ * are any critical regions that cannot form part of such a set, they can use
+ * standard spin_[un]lock().
+ */
+int rspin_trylock(rspinlock_t *lock);
+void rspin_lock(rspinlock_t *lock);
+void rspin_unlock(rspinlock_t *lock);
#define spin_lock(l) _spin_lock(l)
#define spin_lock_cb(l, c, d) _spin_lock_cb(l, c, d)
@@ -241,15 +248,4 @@ void _spin_unlock_recursive(spinlock_t *lock);
/* Ensure a lock is quiescent between two critical operations. */
#define spin_barrier(l) _spin_barrier(l)
-/*
- * spin_[un]lock_recursive(): Use these forms when the lock can (safely!) be
- * reentered recursively on the same CPU. All critical regions that may form
- * part of a recursively-nested set must be protected by these forms. If there
- * are any critical regions that cannot form part of such a set, they can use
- * standard spin_[un]lock().
- */
-#define spin_trylock_recursive(l) _spin_trylock_recursive(l)
-#define spin_lock_recursive(l) _spin_lock_recursive(l)
-#define spin_unlock_recursive(l) _spin_unlock_recursive(l)
-
#endif /* __SPINLOCK_H__ */
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 06/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]()
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (4 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 05/13] xen/spinlock: rename recursive lock functions Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 07/13] xen/spinlock: make struct lock_profile rspinlock_t aware Juergen Gross
` (7 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Wei Liu, George Dunlap, Julien Grall,
Stefano Stabellini
Instead of special casing rspin_lock_irqsave() and
rspin_unlock_irqrestore() for the console lock, add those functions
to spinlock handling and use them where needed.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/arch/x86/traps.c | 14 ++++++++------
xen/common/spinlock.c | 16 ++++++++++++++++
xen/drivers/char/console.c | 18 +-----------------
xen/include/xen/console.h | 5 +++--
xen/include/xen/spinlock.h | 7 +++++++
5 files changed, 35 insertions(+), 25 deletions(-)
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e1356f696a..f72769e79b 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -647,13 +647,15 @@ void show_stack_overflow(unsigned int cpu, const struct cpu_user_regs *regs)
void show_execution_state(const struct cpu_user_regs *regs)
{
/* Prevent interleaving of output. */
- unsigned long flags = console_lock_recursive_irqsave();
+ unsigned long flags;
+
+ rspin_lock_irqsave(&console_lock, flags);
show_registers(regs);
show_code(regs);
show_stack(regs);
- console_unlock_recursive_irqrestore(flags);
+ rspin_unlock_irqrestore(&console_lock, flags);
}
void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs)
@@ -663,7 +665,7 @@ void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs)
void vcpu_show_execution_state(struct vcpu *v)
{
- unsigned long flags = 0;
+ unsigned long flags;
if ( test_bit(_VPF_down, &v->pause_flags) )
{
@@ -698,7 +700,7 @@ void vcpu_show_execution_state(struct vcpu *v)
#endif
/* Prevent interleaving of output. */
- flags = console_lock_recursive_irqsave();
+ rspin_lock_irqsave(&console_lock, flags);
vcpu_show_registers(v);
@@ -708,7 +710,7 @@ void vcpu_show_execution_state(struct vcpu *v)
* Stop interleaving prevention: The necessary P2M lookups involve
* locking, which has to occur with IRQs enabled.
*/
- console_unlock_recursive_irqrestore(flags);
+ rspin_unlock_irqrestore(&console_lock, flags);
show_hvm_stack(v, &v->arch.user_regs);
}
@@ -717,7 +719,7 @@ void vcpu_show_execution_state(struct vcpu *v)
if ( guest_kernel_mode(v, &v->arch.user_regs) )
show_guest_stack(v, &v->arch.user_regs);
- console_unlock_recursive_irqrestore(flags);
+ rspin_unlock_irqrestore(&console_lock, flags);
}
#ifdef CONFIG_HVM
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 58d19ea12c..3b1d1fd8d5 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -475,6 +475,16 @@ void rspin_lock(rspinlock_t *lock)
lock->recurse_cnt++;
}
+unsigned long __rspin_lock_irqsave(rspinlock_t *lock)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ rspin_lock(lock);
+
+ return flags;
+}
+
void rspin_unlock(rspinlock_t *lock)
{
if ( likely(--lock->recurse_cnt == 0) )
@@ -484,6 +494,12 @@ void rspin_unlock(rspinlock_t *lock)
}
}
+void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags)
+{
+ rspin_unlock(lock);
+ local_irq_restore(flags);
+}
+
#ifdef CONFIG_DEBUG_LOCK_PROFILE
struct lock_profile_anc {
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 394c01d097..360c2afbf3 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -120,7 +120,7 @@ static int __read_mostly sercon_handle = -1;
int8_t __read_mostly opt_console_xen; /* console=xen */
#endif
-static DEFINE_RSPINLOCK(console_lock);
+DEFINE_RSPINLOCK(console_lock);
/*
* To control the amount of printing, thresholds are added.
@@ -1149,22 +1149,6 @@ void console_end_log_everything(void)
atomic_dec(&print_everything);
}
-unsigned long console_lock_recursive_irqsave(void)
-{
- unsigned long flags;
-
- local_irq_save(flags);
- rspin_lock(&console_lock);
-
- return flags;
-}
-
-void console_unlock_recursive_irqrestore(unsigned long flags)
-{
- rspin_unlock(&console_lock);
- local_irq_restore(flags);
-}
-
void console_force_unlock(void)
{
watchdog_disable();
diff --git a/xen/include/xen/console.h b/xen/include/xen/console.h
index ab5c30c0da..dff0096b27 100644
--- a/xen/include/xen/console.h
+++ b/xen/include/xen/console.h
@@ -8,8 +8,11 @@
#define __CONSOLE_H__
#include <xen/inttypes.h>
+#include <xen/spinlock.h>
#include <public/xen.h>
+extern rspinlock_t console_lock;
+
struct xen_sysctl_readconsole;
long read_console_ring(struct xen_sysctl_readconsole *op);
@@ -20,8 +23,6 @@ void console_init_postirq(void);
void console_endboot(void);
int console_has(const char *device);
-unsigned long console_lock_recursive_irqsave(void);
-void console_unlock_recursive_irqrestore(unsigned long flags);
void console_force_unlock(void);
void console_start_sync(void);
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index c99ee52458..53f0f72ac4 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -218,7 +218,14 @@ void _spin_barrier(spinlock_t *lock);
*/
int rspin_trylock(rspinlock_t *lock);
void rspin_lock(rspinlock_t *lock);
+#define rspin_lock_irqsave(l, f) \
+ ({ \
+ BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long)); \
+ ((f) = __rspin_lock_irqsave(l)); \
+ })
+unsigned long __rspin_lock_irqsave(rspinlock_t *lock);
void rspin_unlock(rspinlock_t *lock);
+void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags);
#define spin_lock(l) _spin_lock(l)
#define spin_lock_cb(l, c, d) _spin_lock_cb(l, c, d)
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 07/13] xen/spinlock: make struct lock_profile rspinlock_t aware
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (5 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 06/13] xen/spinlock: add rspin_[un]lock_irq[save|restore]() Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 08/13] xen/spinlock: add explicit non-recursive locking functions Juergen Gross
` (6 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
Struct lock_profile contains a pointer to the spinlock it is associated
with. Prepare support of differing spinlock_t and rspinlock_t types by
adding a type indicator of the pointer. Use the highest bit of the
block_cnt member for this indicator in order to not grow the struct
while hurting only the slow path with slightly less performant code.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/common/spinlock.c | 26 +++++++++++++++++++-------
xen/include/xen/spinlock.h | 10 ++++++++--
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 3b1d1fd8d5..7b8e842eba 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -538,19 +538,31 @@ static void spinlock_profile_iterate(lock_profile_subfunc *sub, void *par)
static void cf_check spinlock_profile_print_elem(struct lock_profile *data,
int32_t type, int32_t idx, void *par)
{
- struct spinlock *lock = data->lock;
+ unsigned int cpu;
+ uint32_t lockval;
+
+ if ( data->is_rlock )
+ {
+ cpu = data->rlock->debug.cpu;
+ lockval = data->rlock->tickets.head_tail;
+ }
+ else
+ {
+ cpu = data->lock->debug.cpu;
+ lockval = data->lock->tickets.head_tail;
+ }
printk("%s ", lock_profile_ancs[type].name);
if ( type != LOCKPROF_TYPE_GLOBAL )
printk("%d ", idx);
- printk("%s: addr=%p, lockval=%08x, ", data->name, lock,
- lock->tickets.head_tail);
- if ( lock->debug.cpu == SPINLOCK_NO_CPU )
+ printk("%s: addr=%p, lockval=%08x, ", data->name, data->lock, lockval);
+ if ( cpu == SPINLOCK_NO_CPU )
printk("not locked\n");
else
- printk("cpu=%d\n", lock->debug.cpu);
- printk(" lock:%" PRId64 "(%" PRI_stime "), block:%" PRId64 "(%" PRI_stime ")\n",
- data->lock_cnt, data->time_hold, data->block_cnt, data->time_block);
+ printk("cpu=%u\n", cpu);
+ printk(" lock:%" PRIu64 "(%" PRI_stime "), block:%" PRIu64 "(%" PRI_stime ")\n",
+ data->lock_cnt, data->time_hold, (uint64_t)data->block_cnt,
+ data->time_block);
}
void cf_check spinlock_profile_printall(unsigned char key)
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 53f0f72ac4..5ada9dce3d 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -76,13 +76,19 @@ union lock_debug { };
*/
struct spinlock;
+/* Temporary hack until a dedicated struct rspinlock is existing. */
+#define rspinlock spinlock
struct lock_profile {
struct lock_profile *next; /* forward link */
const char *name; /* lock name */
- struct spinlock *lock; /* the lock itself */
+ union {
+ struct spinlock *lock; /* the lock itself */
+ struct rspinlock *rlock; /* the recursive lock itself */
+ };
uint64_t lock_cnt; /* # of complete locking ops */
- uint64_t block_cnt; /* # of complete wait for lock */
+ uint64_t block_cnt:63; /* # of complete wait for lock */
+ uint64_t is_rlock:1; /* use rlock pointer */
s_time_t time_hold; /* cumulated lock time */
s_time_t time_block; /* cumulated wait time */
s_time_t time_locked; /* system time of last locking */
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 08/13] xen/spinlock: add explicit non-recursive locking functions
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (6 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 07/13] xen/spinlock: make struct lock_profile rspinlock_t aware Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 09/13] xen/spinlock: add another function level Juergen Gross
` (5 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Stefano Stabellini, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk, Andrew Cooper, George Dunlap,
Jan Beulich, Wei Liu, Roger Pau Monné, Tamas K Lengyel,
Lukasz Hawrylko, Daniel P. Smith, Mateusz Mówka
In order to prepare a type-safe recursive spinlock structure, add
explicitly non-recursive locking functions to be used for non-recursive
locking of spinlocks, which are used recursively, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- rename functions (Jan Beulich)
- get rid of !! in pcidevs_locked() (Jan Beulich)
---
xen/arch/arm/mm.c | 4 ++--
xen/arch/x86/domain.c | 12 ++++++------
xen/arch/x86/mm.c | 12 ++++++------
xen/arch/x86/mm/mem_sharing.c | 8 ++++----
xen/arch/x86/mm/p2m-pod.c | 4 ++--
xen/arch/x86/mm/p2m.c | 4 ++--
xen/arch/x86/tboot.c | 4 ++--
xen/common/domctl.c | 4 ++--
xen/common/grant_table.c | 10 +++++-----
xen/common/memory.c | 4 ++--
xen/common/numa.c | 4 ++--
xen/common/page_alloc.c | 16 ++++++++--------
xen/drivers/char/console.c | 16 ++++++++--------
xen/include/xen/spinlock.h | 24 +++++++++++++++++++-----
14 files changed, 70 insertions(+), 56 deletions(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index c34cc94c90..c3a822b834 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1317,7 +1317,7 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d,
if ( page_get_owner(page) == d )
return;
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
/*
* The incremented type count pins as writable or read-only.
@@ -1348,7 +1348,7 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d,
page_list_add_tail(page, &d->xenpage_list);
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
}
int xenmem_add_to_physmap_one(
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 57dd5deeb5..feb0678967 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -212,7 +212,7 @@ void dump_pageframe_info(struct domain *d)
{
unsigned long total[MASK_EXTR(PGT_type_mask, PGT_type_mask) + 1] = {};
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_for_each ( page, &d->page_list )
{
unsigned int index = MASK_EXTR(page->u.inuse.type_info,
@@ -231,13 +231,13 @@ void dump_pageframe_info(struct domain *d)
_p(mfn_x(page_to_mfn(page))),
page->count_info, page->u.inuse.type_info);
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
}
if ( is_hvm_domain(d) )
p2m_pod_dump_data(d);
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_for_each ( page, &d->xenpage_list )
{
@@ -253,7 +253,7 @@ void dump_pageframe_info(struct domain *d)
page->count_info, page->u.inuse.type_info);
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
}
void update_guest_memory_policy(struct vcpu *v,
@@ -2439,10 +2439,10 @@ int domain_relinquish_resources(struct domain *d)
d->arch.auto_unmask = 0;
}
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_splice(&d->arch.relmem_list, &d->page_list);
INIT_PAGE_LIST_HEAD(&d->arch.relmem_list);
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
PROGRESS(xen):
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 39544bd9f9..34682c1795 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -482,7 +482,7 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d,
set_gpfn_from_mfn(mfn_x(page_to_mfn(page)), INVALID_M2P_ENTRY);
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
/* The incremented type count pins as writable or read-only. */
page->u.inuse.type_info =
@@ -502,7 +502,7 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d,
page_list_add_tail(page, &d->xenpage_list);
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
}
void make_cr3(struct vcpu *v, mfn_t mfn)
@@ -3584,11 +3584,11 @@ long do_mmuext_op(
{
bool drop_ref;
- spin_lock(&pg_owner->page_alloc_lock);
+ nrspin_lock(&pg_owner->page_alloc_lock);
drop_ref = (pg_owner->is_dying &&
test_and_clear_bit(_PGT_pinned,
&page->u.inuse.type_info));
- spin_unlock(&pg_owner->page_alloc_lock);
+ nrspin_unlock(&pg_owner->page_alloc_lock);
if ( drop_ref )
{
pin_drop:
@@ -4411,7 +4411,7 @@ int steal_page(
* that it might be upon return from alloc_domheap_pages with
* MEMF_no_owner set.
*/
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
BUG_ON(page->u.inuse.type_info & (PGT_count_mask | PGT_locked |
PGT_pinned));
@@ -4423,7 +4423,7 @@ int steal_page(
if ( !(memflags & MEMF_no_refcount) && !domain_adjust_tot_pages(d, -1) )
drop_dom_ref = true;
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
if ( unlikely(drop_dom_ref) )
put_domain(d);
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 146f61f354..73a82c4f78 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -746,11 +746,11 @@ static int page_make_private(struct domain *d, struct page_info *page)
if ( !get_page(page, dom_cow) )
return -EINVAL;
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
if ( d->is_dying )
{
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
put_page(page);
return -EBUSY;
}
@@ -758,7 +758,7 @@ static int page_make_private(struct domain *d, struct page_info *page)
expected_type = (PGT_shared_page | PGT_validated | PGT_locked | 2);
if ( page->u.inuse.type_info != expected_type )
{
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
put_page(page);
return -EEXIST;
}
@@ -775,7 +775,7 @@ static int page_make_private(struct domain *d, struct page_info *page)
if ( domain_adjust_tot_pages(d, 1) == 1 )
get_knownalive_domain(d);
page_list_add_tail(page, &d->page_list);
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
put_page(page);
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 9969eb45fa..5361c2c5b1 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -27,7 +27,7 @@
static inline void lock_page_alloc(struct p2m_domain *p2m)
{
page_alloc_mm_pre_lock(p2m->domain);
- spin_lock(&(p2m->domain->page_alloc_lock));
+ nrspin_lock(&(p2m->domain->page_alloc_lock));
page_alloc_mm_post_lock(p2m->domain,
p2m->domain->arch.page_alloc_unlock_level);
}
@@ -35,7 +35,7 @@ static inline void lock_page_alloc(struct p2m_domain *p2m)
static inline void unlock_page_alloc(struct p2m_domain *p2m)
{
page_alloc_mm_unlock(p2m->domain->arch.page_alloc_unlock_level);
- spin_unlock(&(p2m->domain->page_alloc_lock));
+ nrspin_unlock(&(p2m->domain->page_alloc_lock));
}
/*
diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index 0983bd71d9..f78c46bda2 100644
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2225,7 +2225,7 @@ void audit_p2m(struct domain *d,
/* Audit part two: walk the domain's page allocation list, checking
* the m2p entries. */
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_for_each ( page, &d->page_list )
{
mfn = mfn_x(page_to_mfn(page));
@@ -2277,7 +2277,7 @@ void audit_p2m(struct domain *d,
P2M_PRINTK("OK: mfn=%#lx, gfn=%#lx, p2mfn=%#lx\n",
mfn, gfn, mfn_x(p2mfn));
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
pod_unlock(p2m);
p2m_unlock(p2m);
diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
index 86c4c22cac..5b33a1bf9d 100644
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -205,14 +205,14 @@ static void tboot_gen_domain_integrity(const uint8_t key[TB_KEY_SIZE],
continue;
printk("MACing Domain %u\n", d->domain_id);
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_for_each(page, &d->page_list)
{
void *pg = __map_domain_page(page);
vmac_update(pg, PAGE_SIZE, &ctx);
unmap_domain_page(pg);
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
if ( is_iommu_enabled(d) && is_vtd )
{
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 505e29c0dc..3733f60f6a 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -621,14 +621,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
{
uint64_t new_max = op->u.max_mem.max_memkb >> (PAGE_SHIFT - 10);
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
/*
* NB. We removed a check that new_max >= current tot_pages; this means
* that the domain will now be allowed to "ratchet" down to new_max. In
* the meantime, while tot > max, all new allocations are disallowed.
*/
d->max_pages = min(new_max, (uint64_t)(typeof(d->max_pages))-1);
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
break;
}
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 89b7811c51..8145974d3a 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2376,7 +2376,7 @@ gnttab_transfer(
mfn = page_to_mfn(page);
}
- spin_lock(&e->page_alloc_lock);
+ nrspin_lock(&e->page_alloc_lock);
/*
* Check that 'e' will accept the page and has reservation
@@ -2387,7 +2387,7 @@ gnttab_transfer(
unlikely(domain_tot_pages(e) >= e->max_pages) ||
unlikely(!(e->tot_pages + 1)) )
{
- spin_unlock(&e->page_alloc_lock);
+ nrspin_unlock(&e->page_alloc_lock);
if ( e->is_dying )
gdprintk(XENLOG_INFO, "Transferee d%d is dying\n",
@@ -2411,7 +2411,7 @@ gnttab_transfer(
* safely drop the lock and re-aquire it later to add page to the
* pagelist.
*/
- spin_unlock(&e->page_alloc_lock);
+ nrspin_unlock(&e->page_alloc_lock);
okay = gnttab_prepare_for_transfer(e, d, gop.ref);
/*
@@ -2427,9 +2427,9 @@ gnttab_transfer(
* Need to grab this again to safely free our "reserved"
* page in the page total
*/
- spin_lock(&e->page_alloc_lock);
+ nrspin_lock(&e->page_alloc_lock);
drop_dom_ref = !domain_adjust_tot_pages(e, -1);
- spin_unlock(&e->page_alloc_lock);
+ nrspin_unlock(&e->page_alloc_lock);
if ( okay /* i.e. e->is_dying due to the surrounding if() */ )
gdprintk(XENLOG_INFO, "Transferee d%d is now dying\n",
diff --git a/xen/common/memory.c b/xen/common/memory.c
index fa165ebc14..f6a4dc17ee 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -770,10 +770,10 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
(1UL << in_chunk_order)) -
(j * (1UL << exch.out.extent_order)));
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
drop_dom_ref = (dec_count &&
!domain_adjust_tot_pages(d, -dec_count));
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
if ( drop_dom_ref )
put_domain(d);
diff --git a/xen/common/numa.c b/xen/common/numa.c
index f454c4d894..47b1d0b5a8 100644
--- a/xen/common/numa.c
+++ b/xen/common/numa.c
@@ -718,13 +718,13 @@ static void cf_check dump_numa(unsigned char key)
memset(page_num_node, 0, sizeof(page_num_node));
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_for_each ( page, &d->page_list )
{
i = page_to_nid(page);
page_num_node[i]++;
}
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
for_each_online_node ( i )
printk(" Node %u: %u\n", i, page_num_node[i]);
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 8c6a3d9274..a25c00a7d4 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -515,7 +515,7 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
* must always take the global heap_lock rather than only in the much
* rarer case that d->outstanding_pages is non-zero
*/
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
spin_lock(&heap_lock);
/* pages==0 means "unset" the claim. */
@@ -561,7 +561,7 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
out:
spin_unlock(&heap_lock);
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
return ret;
}
@@ -2343,7 +2343,7 @@ int assign_pages(
int rc = 0;
unsigned int i;
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
if ( unlikely(d->is_dying) )
{
@@ -2425,7 +2425,7 @@ int assign_pages(
}
out:
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
return rc;
}
@@ -2906,9 +2906,9 @@ mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags)
ASSERT_ALLOC_CONTEXT();
/* Acquire a page from reserved page list(resv_page_list). */
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page = page_list_remove_head(&d->resv_page_list);
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
if ( unlikely(!page) )
return INVALID_MFN;
@@ -2927,9 +2927,9 @@ mfn_t acquire_reserved_page(struct domain *d, unsigned int memflags)
*/
unprepare_staticmem_pages(page, 1, false);
fail:
- spin_lock(&d->page_alloc_lock);
+ nrspin_lock(&d->page_alloc_lock);
page_list_add_tail(page, &d->resv_page_list);
- spin_unlock(&d->page_alloc_lock);
+ nrspin_unlock(&d->page_alloc_lock);
return INVALID_MFN;
}
#endif
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 360c2afbf3..d5d8b6cd77 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -369,9 +369,9 @@ long read_console_ring(struct xen_sysctl_readconsole *op)
if ( op->clear )
{
- spin_lock_irq(&console_lock);
+ nrspin_lock_irq(&console_lock);
conringc = p - c > conring_size ? p - conring_size : c;
- spin_unlock_irq(&console_lock);
+ nrspin_unlock_irq(&console_lock);
}
op->count = sofar;
@@ -630,7 +630,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
if ( is_hardware_domain(cd) )
{
/* Use direct console output as it could be interactive */
- spin_lock_irq(&console_lock);
+ nrspin_lock_irq(&console_lock);
console_serial_puts(kbuf, kcount);
video_puts(kbuf, kcount);
@@ -651,7 +651,7 @@ static long guest_console_write(XEN_GUEST_HANDLE_PARAM(char) buffer,
tasklet_schedule(¬ify_dom0_con_ring_tasklet);
}
- spin_unlock_irq(&console_lock);
+ nrspin_unlock_irq(&console_lock);
}
else
{
@@ -1018,9 +1018,9 @@ void __init console_init_preirq(void)
pv_console_set_rx_handler(serial_rx);
/* HELLO WORLD --- start-of-day banner text. */
- spin_lock(&console_lock);
+ nrspin_lock(&console_lock);
__putstr(xen_banner());
- spin_unlock(&console_lock);
+ nrspin_unlock(&console_lock);
printk("Xen version %d.%d%s (%s@%s) (%s) %s %s\n",
xen_major_version(), xen_minor_version(), xen_extra_version(),
xen_compile_by(), xen_compile_domain(), xen_compiler(),
@@ -1057,13 +1057,13 @@ void __init console_init_ring(void)
}
opt_conring_size = PAGE_SIZE << order;
- spin_lock_irqsave(&console_lock, flags);
+ nrspin_lock_irqsave(&console_lock, flags);
for ( i = conringc ; i != conringp; i++ )
ring[i & (opt_conring_size - 1)] = conring[i & (conring_size - 1)];
conring = ring;
smp_wmb(); /* Allow users of console_force_unlock() to see larger buffer. */
conring_size = opt_conring_size;
- spin_unlock_irqrestore(&console_lock, flags);
+ nrspin_unlock_irqrestore(&console_lock, flags);
printk("Allocated console ring of %u KiB.\n", opt_conring_size >> 10);
}
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 5ada9dce3d..12764bcddf 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -101,6 +101,8 @@ struct lock_profile_qhead {
};
#define _LOCK_PROFILE(lockname) { .name = #lockname, .lock = &lockname, }
+#define _RLOCK_PROFILE(lockname) { .name = #lockname, .rlock = &lockname, \
+ .is_rlock = 1, }
#define _LOCK_PROFILE_PTR(name) \
static struct lock_profile * const __lock_profile_##name \
__used_section(".lockprofile.data") = \
@@ -117,10 +119,10 @@ struct lock_profile_qhead {
_LOCK_PROFILE_PTR(l)
#define DEFINE_RSPINLOCK(l) \
rspinlock_t l = _SPIN_LOCK_UNLOCKED(NULL); \
- static struct lock_profile __lock_profile_data_##l = _LOCK_PROFILE(l); \
+ static struct lock_profile __lock_profile_data_##l = _RLOCK_PROFILE(l); \
_LOCK_PROFILE_PTR(l)
-#define __spin_lock_init_prof(s, l, locktype) \
+#define __spin_lock_init_prof(s, l, lockptr, locktype, isr) \
do { \
struct lock_profile *prof; \
prof = xzalloc(struct lock_profile); \
@@ -133,13 +135,16 @@ struct lock_profile_qhead {
break; \
} \
prof->name = #l; \
- prof->lock = &(s)->l; \
+ prof->lockptr = &(s)->l; \
+ prof->is_rlock = isr; \
prof->next = (s)->profile_head.elem_q; \
(s)->profile_head.elem_q = prof; \
} while( 0 )
-#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
+
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)
+
#endif /* __SPINLOCK_H__ */
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 09/13] xen/spinlock: add another function level
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (7 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 08/13] xen/spinlock: add explicit non-recursive locking functions Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 10/13] xen/spinlock: add missing rspin_is_locked() and rspin_barrier() Juergen Gross
` (4 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
Add another function level in spinlock.c hiding the spinlock_t layout
from the low level locking code.
This is done in preparation of introducing rspinlock_t for recursive
locks without having to duplicate all of the locking code.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/common/spinlock.c | 104 +++++++++++++++++++++++--------------
xen/include/xen/spinlock.h | 1 +
2 files changed, 65 insertions(+), 40 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 7b8e842eba..c17ea42fe6 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -261,29 +261,31 @@ void spin_debug_disable(void)
#ifdef CONFIG_DEBUG_LOCK_PROFILE
+#define LOCK_PROFILE_PAR lock->profile
#define LOCK_PROFILE_REL \
- if ( lock->profile ) \
+ if ( profile ) \
{ \
- lock->profile->time_hold += NOW() - lock->profile->time_locked; \
- lock->profile->lock_cnt++; \
+ profile->time_hold += NOW() - profile->time_locked; \
+ profile->lock_cnt++; \
}
#define LOCK_PROFILE_VAR(val) s_time_t block = (val)
#define LOCK_PROFILE_BLOCK block = block ? : NOW()
#define LOCK_PROFILE_BLKACC(tst, val) \
if ( tst ) \
{ \
- lock->profile->time_block += lock->profile->time_locked - (val); \
- lock->profile->block_cnt++; \
+ profile->time_block += profile->time_locked - (val); \
+ profile->block_cnt++; \
}
#define LOCK_PROFILE_GOT(val) \
- if ( lock->profile ) \
+ if ( profile ) \
{ \
- lock->profile->time_locked = NOW(); \
+ profile->time_locked = NOW(); \
LOCK_PROFILE_BLKACC(val, val); \
}
#else
+#define LOCK_PROFILE_PAR NULL
#define LOCK_PROFILE_REL
#define LOCK_PROFILE_VAR(val)
#define LOCK_PROFILE_BLOCK
@@ -307,17 +309,18 @@ static always_inline uint16_t observe_head(spinlock_tickets_t *t)
return read_atomic(&t->head);
}
-static void always_inline spin_lock_common(spinlock_t *lock,
+static void always_inline spin_lock_common(spinlock_tickets_t *t,
+ union lock_debug *debug,
+ struct lock_profile *profile,
void (*cb)(void *), void *data)
{
spinlock_tickets_t tickets = SPINLOCK_TICKET_INC;
LOCK_PROFILE_VAR(0);
- check_lock(&lock->debug, false);
+ check_lock(debug, false);
preempt_disable();
- tickets.head_tail = arch_fetch_and_add(&lock->tickets.head_tail,
- tickets.head_tail);
- while ( tickets.tail != observe_head(&lock->tickets) )
+ tickets.head_tail = arch_fetch_and_add(&t->head_tail, tickets.head_tail);
+ while ( tickets.tail != observe_head(t) )
{
LOCK_PROFILE_BLOCK;
if ( cb )
@@ -325,18 +328,19 @@ static void always_inline spin_lock_common(spinlock_t *lock,
arch_lock_relax();
}
arch_lock_acquire_barrier();
- got_lock(&lock->debug);
+ got_lock(debug);
LOCK_PROFILE_GOT(block);
}
void _spin_lock(spinlock_t *lock)
{
- spin_lock_common(lock, NULL, NULL);
+ spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, NULL,
+ NULL);
}
void _spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data)
{
- spin_lock_common(lock, cb, data);
+ spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, cb, data);
}
void _spin_lock_irq(spinlock_t *lock)
@@ -355,16 +359,23 @@ unsigned long _spin_lock_irqsave(spinlock_t *lock)
return flags;
}
-void _spin_unlock(spinlock_t *lock)
+static void always_inline spin_unlock_common(spinlock_tickets_t *t,
+ union lock_debug *debug,
+ struct lock_profile *profile)
{
LOCK_PROFILE_REL;
- rel_lock(&lock->debug);
+ rel_lock(debug);
arch_lock_release_barrier();
- add_sized(&lock->tickets.head, 1);
+ add_sized(&t->head, 1);
arch_lock_signal();
preempt_enable();
}
+void _spin_unlock(spinlock_t *lock)
+{
+ spin_unlock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
+}
+
void _spin_unlock_irq(spinlock_t *lock)
{
_spin_unlock(lock);
@@ -377,25 +388,25 @@ void _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
local_irq_restore(flags);
}
+static int always_inline spin_is_locked_common(const spinlock_tickets_t *t)
+{
+ return t->head != t->tail;
+}
+
int _spin_is_locked(const spinlock_t *lock)
{
- /*
- * Recursive locks may be locked by another CPU, yet we return
- * "false" here, making this function suitable only for use in
- * ASSERT()s and alike.
- */
- return lock->recurse_cpu == SPINLOCK_NO_CPU
- ? lock->tickets.head != lock->tickets.tail
- : lock->recurse_cpu == smp_processor_id();
+ return spin_is_locked_common(&lock->tickets);
}
-int _spin_trylock(spinlock_t *lock)
+static int always_inline spin_trylock_common(spinlock_tickets_t *t,
+ union lock_debug *debug,
+ struct lock_profile *profile)
{
spinlock_tickets_t old, new;
preempt_disable();
- check_lock(&lock->debug, true);
- old = observe_lock(&lock->tickets);
+ check_lock(debug, true);
+ old = observe_lock(t);
if ( old.head != old.tail )
{
preempt_enable();
@@ -403,8 +414,7 @@ int _spin_trylock(spinlock_t *lock)
}
new = old;
new.tail++;
- if ( cmpxchg(&lock->tickets.head_tail,
- old.head_tail, new.head_tail) != old.head_tail )
+ if ( cmpxchg(&t->head_tail, old.head_tail, new.head_tail) != old.head_tail )
{
preempt_enable();
return 0;
@@ -413,29 +423,41 @@ int _spin_trylock(spinlock_t *lock)
* cmpxchg() is a full barrier so no need for an
* arch_lock_acquire_barrier().
*/
- got_lock(&lock->debug);
+ got_lock(debug);
LOCK_PROFILE_GOT(0);
return 1;
}
-void _spin_barrier(spinlock_t *lock)
+int _spin_trylock(spinlock_t *lock)
+{
+ return spin_trylock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
+}
+
+static void always_inline spin_barrier_common(spinlock_tickets_t *t,
+ union lock_debug *debug,
+ struct lock_profile *profile)
{
spinlock_tickets_t sample;
LOCK_PROFILE_VAR(NOW());
- check_barrier(&lock->debug);
+ check_barrier(debug);
smp_mb();
- sample = observe_lock(&lock->tickets);
+ sample = observe_lock(t);
if ( sample.head != sample.tail )
{
- while ( observe_head(&lock->tickets) == sample.head )
+ while ( observe_head(t) == sample.head )
arch_lock_relax();
- LOCK_PROFILE_BLKACC(lock->profile, block);
+ LOCK_PROFILE_BLKACC(profile, block);
}
smp_mb();
}
+void _spin_barrier(spinlock_t *lock)
+{
+ spin_barrier_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
+}
+
int rspin_trylock(rspinlock_t *lock)
{
unsigned int cpu = smp_processor_id();
@@ -448,7 +470,8 @@ int rspin_trylock(rspinlock_t *lock)
if ( likely(lock->recurse_cpu != cpu) )
{
- if ( !spin_trylock(lock) )
+ if ( !spin_trylock_common(&lock->tickets, &lock->debug,
+ LOCK_PROFILE_PAR) )
return 0;
lock->recurse_cpu = cpu;
}
@@ -466,7 +489,8 @@ void rspin_lock(rspinlock_t *lock)
if ( likely(lock->recurse_cpu != cpu) )
{
- _spin_lock(lock);
+ spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, NULL,
+ NULL);
lock->recurse_cpu = cpu;
}
@@ -490,7 +514,7 @@ void rspin_unlock(rspinlock_t *lock)
if ( likely(--lock->recurse_cnt == 0) )
{
lock->recurse_cpu = SPINLOCK_NO_CPU;
- spin_unlock(lock);
+ spin_unlock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
}
}
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index 12764bcddf..ccb1cafa5f 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -163,6 +163,7 @@ extern void cf_check spinlock_profile_reset(unsigned char key);
#else
struct lock_profile_qhead { };
+struct lock_profile { };
#define SPIN_LOCK_UNLOCKED { \
.recurse_cpu = SPINLOCK_NO_CPU, \
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 10/13] xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (8 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 09/13] xen/spinlock: add another function level Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 11/13] xen/spinlock: split recursive spinlocks from normal ones Juergen Gross
` (3 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Jan Beulich, Andrew Cooper,
George Dunlap, Roger Pau Monné, Wei Liu, Julien Grall,
Stefano Stabellini, Paul Durrant
Add rspin_is_locked() and rspin_barrier() in order to prepare differing
spinlock_t and rspinlock_t types.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- partially carved out from V1 patch, partially new
---
xen/arch/x86/mm/p2m-pod.c | 2 +-
xen/common/domain.c | 2 +-
xen/common/page_alloc.c | 2 +-
xen/common/spinlock.c | 17 +++++++++++++++++
xen/drivers/char/console.c | 4 ++--
xen/drivers/passthrough/pci.c | 2 +-
xen/include/xen/spinlock.h | 2 ++
7 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 5361c2c5b1..ad49c64ead 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -385,7 +385,7 @@ int p2m_pod_empty_cache(struct domain *d)
/* After this barrier no new PoD activities can happen. */
BUG_ON(!d->is_dying);
- spin_barrier(&p2m->pod.lock.lock);
+ rspin_barrier(&p2m->pod.lock.lock);
lock_page_alloc(p2m);
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 604f70ff5a..17da8e0746 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -974,7 +974,7 @@ int domain_kill(struct domain *d)
case DOMDYING_alive:
domain_pause(d);
d->is_dying = DOMDYING_dying;
- spin_barrier(&d->domain_lock);
+ rspin_barrier(&d->domain_lock);
argo_destroy(d);
vnuma_destroy(d->vnuma);
domain_set_outstanding_pages(d, 0);
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index a25c00a7d4..14010b6fa5 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -476,7 +476,7 @@ unsigned long domain_adjust_tot_pages(struct domain *d, long pages)
{
long dom_before, dom_after, dom_claimed, sys_before, sys_after;
- ASSERT(spin_is_locked(&d->page_alloc_lock));
+ ASSERT(rspin_is_locked(&d->page_alloc_lock));
d->tot_pages += pages;
/*
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index c17ea42fe6..9159a5a7c3 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -458,6 +458,23 @@ void _spin_barrier(spinlock_t *lock)
spin_barrier_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
}
+int rspin_is_locked(const rspinlock_t *lock)
+{
+ /*
+ * Recursive locks may be locked by another CPU, yet we return
+ * "false" here, making this function suitable only for use in
+ * ASSERT()s and alike.
+ */
+ return lock->recurse_cpu == SPINLOCK_NO_CPU
+ ? spin_is_locked_common(&lock->tickets)
+ : lock->recurse_cpu == smp_processor_id();
+}
+
+void rspin_barrier(rspinlock_t *lock)
+{
+ spin_barrier_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
+}
+
int rspin_trylock(rspinlock_t *lock)
{
unsigned int cpu = smp_processor_id();
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index d5d8b6cd77..63cad87125 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -328,7 +328,7 @@ static void cf_check do_dec_thresh(unsigned char key, struct cpu_user_regs *regs
static void conring_puts(const char *str, size_t len)
{
- ASSERT(spin_is_locked(&console_lock));
+ ASSERT(rspin_is_locked(&console_lock));
while ( len-- )
conring[CONRING_IDX_MASK(conringp++)] = *str++;
@@ -757,7 +757,7 @@ static void __putstr(const char *str)
{
size_t len = strlen(str);
- ASSERT(spin_is_locked(&console_lock));
+ ASSERT(rspin_is_locked(&console_lock));
console_serial_puts(str, len);
video_puts(str, len);
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 22342f07ac..f55c1a5cf4 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -64,7 +64,7 @@ void pcidevs_unlock(void)
bool pcidevs_locked(void)
{
- return !!spin_is_locked(&_pcidevs_lock);
+ return rspin_is_locked(&_pcidevs_lock);
}
static struct radix_tree_root pci_segments;
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index ccb1cafa5f..fb6ca1949a 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -239,6 +239,8 @@ void rspin_lock(rspinlock_t *lock);
unsigned long __rspin_lock_irqsave(rspinlock_t *lock);
void rspin_unlock(rspinlock_t *lock);
void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags);
+int rspin_is_locked(const rspinlock_t *lock);
+void rspin_barrier(rspinlock_t *lock);
#define spin_lock(l) _spin_lock(l)
#define spin_lock_cb(l, c, d) _spin_lock_cb(l, c, d)
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 11/13] xen/spinlock: split recursive spinlocks from normal ones
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (9 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 10/13] xen/spinlock: add missing rspin_is_locked() and rspin_barrier() Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 12/13] xen/spinlock: remove indirection through macros for spin_*() functions Juergen Gross
` (2 subsequent siblings)
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
Recursive and normal spinlocks are sharing the same data structure for
representation of the lock. This has two major disadvantages:
- it is not clear from the definition of a lock, whether it is intended
to be used recursive or not, while a mixture of both usage variants
needs to be
- in production builds (builds without CONFIG_DEBUG_LOCKS) the needed
data size of an ordinary spinlock is 8 bytes instead of 4, due to the
additional recursion data needed (associated with that the rwlock
data is using 12 instead of only 8 bytes)
Fix that by introducing a struct spinlock_recursive for recursive
spinlocks only, and switch recursive spinlock functions to require
pointers to this new struct.
This allows to check the correct usage at build time.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- use shorter names (Jan Beulich)
- don't embed spinlock_t in rspinlock_t (Jan Beulich)
---
xen/common/spinlock.c | 49 ++++++++++++++++++++++++++++++++
xen/include/xen/spinlock.h | 58 +++++++++++++++++++++++++-------------
2 files changed, 88 insertions(+), 19 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 9159a5a7c3..cff3f126e6 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -541,6 +541,55 @@ void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags)
local_irq_restore(flags);
}
+int nrspin_trylock(rspinlock_t *lock)
+{
+ check_lock(&lock->debug, true);
+
+ if ( unlikely(lock->recurse_cpu != SPINLOCK_NO_CPU) )
+ return 0;
+
+ return spin_trylock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
+}
+
+void nrspin_lock(rspinlock_t *lock)
+{
+ spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, NULL,
+ NULL);
+}
+
+void nrspin_unlock(rspinlock_t *lock)
+{
+ spin_unlock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
+}
+
+void nrspin_lock_irq(rspinlock_t *lock)
+{
+ ASSERT(local_irq_is_enabled());
+ local_irq_disable();
+ nrspin_lock(lock);
+}
+
+void nrspin_unlock_irq(rspinlock_t *lock)
+{
+ nrspin_unlock(lock);
+ local_irq_enable();
+}
+
+unsigned long __nrspin_lock_irqsave(rspinlock_t *lock)
+{
+ unsigned long flags;
+
+ local_irq_save(flags);
+ nrspin_lock(lock);
+ return flags;
+}
+
+void nrspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags)
+{
+ nrspin_unlock(lock);
+ local_irq_restore(flags);
+}
+
#ifdef CONFIG_DEBUG_LOCK_PROFILE
struct lock_profile_anc {
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index fb6ca1949a..a2b1bb2df6 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -76,8 +76,6 @@ union lock_debug { };
*/
struct spinlock;
-/* Temporary hack until a dedicated struct rspinlock is existing. */
-#define rspinlock spinlock
struct lock_profile {
struct lock_profile *next; /* forward link */
@@ -108,6 +106,10 @@ struct lock_profile_qhead {
__used_section(".lockprofile.data") = \
&__lock_profile_data_##name
#define _SPIN_LOCK_UNLOCKED(x) { \
+ .debug =_LOCK_DEBUG, \
+ .profile = x, \
+}
+#define _RSPIN_LOCK_UNLOCKED(x) { \
.recurse_cpu = SPINLOCK_NO_CPU, \
.debug =_LOCK_DEBUG, \
.profile = x, \
@@ -117,8 +119,9 @@ struct lock_profile_qhead {
spinlock_t l = _SPIN_LOCK_UNLOCKED(NULL); \
static struct lock_profile __lock_profile_data_##l = _LOCK_PROFILE(l); \
_LOCK_PROFILE_PTR(l)
+#define RSPIN_LOCK_UNLOCKED _RSPIN_LOCK_UNLOCKED(NULL)
#define DEFINE_RSPINLOCK(l) \
- rspinlock_t l = _SPIN_LOCK_UNLOCKED(NULL); \
+ rspinlock_t l = _RSPIN_LOCK_UNLOCKED(NULL); \
static struct lock_profile __lock_profile_data_##l = _RLOCK_PROFILE(l); \
_LOCK_PROFILE_PTR(l)
@@ -143,8 +146,11 @@ struct lock_profile_qhead {
#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)
+#define rspin_lock_init_prof(s, l) do { \
+ __spin_lock_init_prof(s, l, rlock, rspinlock_t, 1); \
+ (s)->l.recurse_cpu = SPINLOCK_NO_CPU; \
+ (s)->l.recurse_cnt = 0; \
+ } while (0)
void _lock_profile_register_struct(
int32_t type, struct lock_profile_qhead *qhead, int32_t idx);
@@ -166,11 +172,15 @@ struct lock_profile_qhead { };
struct lock_profile { };
#define SPIN_LOCK_UNLOCKED { \
+ .debug =_LOCK_DEBUG, \
+}
+#define RSPIN_LOCK_UNLOCKED { \
+ .debug =_LOCK_DEBUG, \
.recurse_cpu = SPINLOCK_NO_CPU, \
.debug =_LOCK_DEBUG, \
}
#define DEFINE_SPINLOCK(l) spinlock_t l = SPIN_LOCK_UNLOCKED
-#define DEFINE_RSPINLOCK(l) rspinlock_t l = SPIN_LOCK_UNLOCKED
+#define DEFINE_RSPINLOCK(l) rspinlock_t l = RSPIN_LOCK_UNLOCKED
#define spin_lock_init_prof(s, l) spin_lock_init(&((s)->l))
#define rspin_lock_init_prof(s, l) rspin_lock_init(&((s)->l))
@@ -180,7 +190,6 @@ struct lock_profile { };
#endif
-
typedef union {
uint32_t head_tail;
struct {
@@ -192,6 +201,14 @@ typedef union {
#define SPINLOCK_TICKET_INC { .head_tail = 0x10000, }
typedef struct spinlock {
+ spinlock_tickets_t tickets;
+ union lock_debug debug;
+#ifdef CONFIG_DEBUG_LOCK_PROFILE
+ struct lock_profile *profile;
+#endif
+} spinlock_t;
+
+typedef struct rspinlock {
spinlock_tickets_t tickets;
uint16_t recurse_cpu:SPINLOCK_CPU_BITS;
#define SPINLOCK_NO_CPU ((1u << SPINLOCK_CPU_BITS) - 1)
@@ -202,12 +219,10 @@ typedef struct spinlock {
#ifdef CONFIG_DEBUG_LOCK_PROFILE
struct lock_profile *profile;
#endif
-} spinlock_t;
-
-typedef spinlock_t rspinlock_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)
+#define rspin_lock_init(l) (*(l) = (rspinlock_t)RSPIN_LOCK_UNLOCKED)
void _spin_lock(spinlock_t *lock);
void _spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data);
@@ -242,6 +257,19 @@ void rspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags);
int rspin_is_locked(const rspinlock_t *lock);
void rspin_barrier(rspinlock_t *lock);
+int nrspin_trylock(rspinlock_t *lock);
+void nrspin_lock(rspinlock_t *lock);
+void nrspin_unlock(rspinlock_t *lock);
+void nrspin_lock_irq(rspinlock_t *lock);
+void nrspin_unlock_irq(rspinlock_t *lock);
+#define nrspin_lock_irqsave(l, f) \
+ ({ \
+ BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long)); \
+ ((f) = __nrspin_lock_irqsave(l)); \
+ })
+unsigned long __nrspin_lock_irqsave(rspinlock_t *lock);
+void nrspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags);
+
#define spin_lock(l) _spin_lock(l)
#define spin_lock_cb(l, c, d) _spin_lock_cb(l, c, d)
#define spin_lock_irq(l) _spin_lock_irq(l)
@@ -270,12 +298,4 @@ void rspin_barrier(rspinlock_t *lock);
/* 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)
-
#endif /* __SPINLOCK_H__ */
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 12/13] xen/spinlock: remove indirection through macros for spin_*() functions
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (10 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 11/13] xen/spinlock: split recursive spinlocks from normal ones Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-13 9:42 ` [PATCH v2 13/13] xen/spinlock: support higher number of cpus Juergen Gross
2023-10-19 7:48 ` [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Jan Beulich
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
In reality all spin_*() functions are macros which are defined to just
call a related real function.
Remove this macro layer, as it is adding complexity without any gain.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- new patch
---
xen/common/spinlock.c | 28 +++++++++---------
xen/include/xen/spinlock.h | 58 +++++++++++++++-----------------------
2 files changed, 36 insertions(+), 50 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index cff3f126e6..5fa5c1890a 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -332,30 +332,30 @@ static void always_inline spin_lock_common(spinlock_tickets_t *t,
LOCK_PROFILE_GOT(block);
}
-void _spin_lock(spinlock_t *lock)
+void spin_lock(spinlock_t *lock)
{
spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, NULL,
NULL);
}
-void _spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data)
+void spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data)
{
spin_lock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR, cb, data);
}
-void _spin_lock_irq(spinlock_t *lock)
+void spin_lock_irq(spinlock_t *lock)
{
ASSERT(local_irq_is_enabled());
local_irq_disable();
- _spin_lock(lock);
+ spin_lock(lock);
}
-unsigned long _spin_lock_irqsave(spinlock_t *lock)
+unsigned long __spin_lock_irqsave(spinlock_t *lock)
{
unsigned long flags;
local_irq_save(flags);
- _spin_lock(lock);
+ spin_lock(lock);
return flags;
}
@@ -371,20 +371,20 @@ static void always_inline spin_unlock_common(spinlock_tickets_t *t,
preempt_enable();
}
-void _spin_unlock(spinlock_t *lock)
+void spin_unlock(spinlock_t *lock)
{
spin_unlock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
}
-void _spin_unlock_irq(spinlock_t *lock)
+void spin_unlock_irq(spinlock_t *lock)
{
- _spin_unlock(lock);
+ spin_unlock(lock);
local_irq_enable();
}
-void _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
+void spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags)
{
- _spin_unlock(lock);
+ spin_unlock(lock);
local_irq_restore(flags);
}
@@ -393,7 +393,7 @@ static int always_inline spin_is_locked_common(const spinlock_tickets_t *t)
return t->head != t->tail;
}
-int _spin_is_locked(const spinlock_t *lock)
+int spin_is_locked(const spinlock_t *lock)
{
return spin_is_locked_common(&lock->tickets);
}
@@ -429,7 +429,7 @@ static int always_inline spin_trylock_common(spinlock_tickets_t *t,
return 1;
}
-int _spin_trylock(spinlock_t *lock)
+int spin_trylock(spinlock_t *lock)
{
return spin_trylock_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
}
@@ -453,7 +453,7 @@ static void always_inline spin_barrier_common(spinlock_tickets_t *t,
smp_mb();
}
-void _spin_barrier(spinlock_t *lock)
+void spin_barrier(spinlock_t *lock)
{
spin_barrier_common(&lock->tickets, &lock->debug, LOCK_PROFILE_PAR);
}
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index a2b1bb2df6..e4f2e36e4f 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -224,18 +224,30 @@ typedef struct rspinlock {
#define spin_lock_init(l) (*(l) = (spinlock_t)SPIN_LOCK_UNLOCKED)
#define rspin_lock_init(l) (*(l) = (rspinlock_t)RSPIN_LOCK_UNLOCKED)
-void _spin_lock(spinlock_t *lock);
-void _spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data);
-void _spin_lock_irq(spinlock_t *lock);
-unsigned long _spin_lock_irqsave(spinlock_t *lock);
+void spin_lock(spinlock_t *lock);
+void spin_lock_cb(spinlock_t *lock, void (*cb)(void *), void *data);
+void spin_lock_irq(spinlock_t *lock);
+#define spin_lock_irqsave(l, f) \
+ ({ \
+ BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long)); \
+ ((f) = __spin_lock_irqsave(l)); \
+ })
+unsigned long __spin_lock_irqsave(spinlock_t *lock);
-void _spin_unlock(spinlock_t *lock);
-void _spin_unlock_irq(spinlock_t *lock);
-void _spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags);
+void spin_unlock(spinlock_t *lock);
+void spin_unlock_irq(spinlock_t *lock);
+void spin_unlock_irqrestore(spinlock_t *lock, unsigned long flags);
-int _spin_is_locked(const spinlock_t *lock);
-int _spin_trylock(spinlock_t *lock);
-void _spin_barrier(spinlock_t *lock);
+int spin_is_locked(const spinlock_t *lock);
+int spin_trylock(spinlock_t *lock);
+#define spin_trylock_irqsave(lock, flags) \
+({ \
+ local_irq_save(flags); \
+ spin_trylock(lock) ? \
+ 1 : ({ local_irq_restore(flags); 0; }); \
+})
+/* Ensure a lock is quiescent between two critical operations. */
+void spin_barrier(spinlock_t *lock);
/*
* rspin_[un]lock(): Use these forms when the lock can (safely!) be
@@ -270,32 +282,6 @@ void nrspin_unlock_irq(rspinlock_t *lock);
unsigned long __nrspin_lock_irqsave(rspinlock_t *lock);
void nrspin_unlock_irqrestore(rspinlock_t *lock, unsigned long flags);
-#define spin_lock(l) _spin_lock(l)
-#define spin_lock_cb(l, c, d) _spin_lock_cb(l, c, d)
-#define spin_lock_irq(l) _spin_lock_irq(l)
-#define spin_lock_irqsave(l, f) \
- ({ \
- BUILD_BUG_ON(sizeof(f) != sizeof(unsigned long)); \
- ((f) = _spin_lock_irqsave(l)); \
- })
-
-#define spin_unlock(l) _spin_unlock(l)
-#define spin_unlock_irq(l) _spin_unlock_irq(l)
-#define spin_unlock_irqrestore(l, f) _spin_unlock_irqrestore(l, f)
-
-#define spin_is_locked(l) _spin_is_locked(l)
-#define spin_trylock(l) _spin_trylock(l)
-
-#define spin_trylock_irqsave(lock, flags) \
-({ \
- local_irq_save(flags); \
- spin_trylock(lock) ? \
- 1 : ({ local_irq_restore(flags); 0; }); \
-})
-
#define spin_lock_kick(l) arch_lock_signal_wmb()
-/* Ensure a lock is quiescent between two critical operations. */
-#define spin_barrier(l) _spin_barrier(l)
-
#endif /* __SPINLOCK_H__ */
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH v2 13/13] xen/spinlock: support higher number of cpus
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (11 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 12/13] xen/spinlock: remove indirection through macros for spin_*() functions Juergen Gross
@ 2023-10-13 9:42 ` Juergen Gross
2023-10-19 7:48 ` [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Jan Beulich
13 siblings, 0 replies; 23+ messages in thread
From: Juergen Gross @ 2023-10-13 9:42 UTC (permalink / raw)
To: xen-devel
Cc: javi.merino, Juergen Gross, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
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.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
xen/common/spinlock.c | 1 +
xen/include/xen/spinlock.h | 18 +++++++++---------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 5fa5c1890a..719b067b6e 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -481,6 +481,7 @@ int 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);
check_lock(&lock->debug, true);
diff --git a/xen/include/xen/spinlock.h b/xen/include/xen/spinlock.h
index e4f2e36e4f..ea88f3ad75 100644
--- a/xen/include/xen/spinlock.h
+++ b/xen/include/xen/spinlock.h
@@ -7,16 +7,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;
};
@@ -210,10 +210,10 @@ 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_RECURSE_BITS 8
+ uint8_t recurse_cnt;
#define SPINLOCK_MAX_RECURSE ((1u << SPINLOCK_RECURSE_BITS) - 1)
union lock_debug debug;
#ifdef CONFIG_DEBUG_LOCK_PROFILE
--
2.35.3
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type
2023-10-13 9:42 [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Juergen Gross
` (12 preceding siblings ...)
2023-10-13 9:42 ` [PATCH v2 13/13] xen/spinlock: support higher number of cpus Juergen Gross
@ 2023-10-19 7:48 ` Jan Beulich
2023-10-19 9:35 ` Andrew Cooper
13 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2023-10-19 7:48 UTC (permalink / raw)
To: Juergen Gross
Cc: javi.merino, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, Roger Pau Monné, Paul Durrant,
Bertrand Marquis, Volodymyr Babchuk, Tamas K Lengyel,
Lukasz Hawrylko, Daniel P. Smith, Mateusz Mówka, xen-devel
On 13.10.2023 11:42, Juergen Gross wrote:
> Instead of being able to use normal spinlocks as recursive ones, too,
> make recursive spinlocks a special lock type.
>
> This will make the spinlock structure smaller in production builds and
> add type-safety.
>
> This allows to increase the maximum number of physical cpus from 8191
> to 65535 without increasing the size of the lock structure in production
> builds (the size of recursive spinlocks in debug builds will grow to
> 12 bytes due to that change).
>
> Changes in V2:
> - addressed comments by Jan Beulich
> - lots of additional cleanups
> - reorganized complete series
>
> Juergen Gross (13):
> xen/spinlock: fix coding style issues
> xen/spinlock: reduce lock profile ifdefs
> xen/spinlock: make spinlock initializers more readable
> xen/spinlock: introduce new type for recursive spinlocks
> xen/spinlock: rename recursive lock functions
> xen/spinlock: add rspin_[un]lock_irq[save|restore]()
> xen/spinlock: make struct lock_profile rspinlock_t aware
> xen/spinlock: add explicit non-recursive locking functions
> xen/spinlock: add another function level
> xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
> xen/spinlock: split recursive spinlocks from normal ones
> xen/spinlock: remove indirection through macros for spin_*() functions
> xen/spinlock: support higher number of cpus
Before looking at patches 4 and onwards, I'd like us to settle on the future
of recursive locking in Xen, considering in particular Andrew's objections
to their use in the code base. If the plan was to eliminate them, I'd see
little point in reworking the infrastructure. I'd like to suggest that one
of us tries to remember to put this up as an agenda item for the next
Community Call. But of course the discussion can also happen right here; I
merely expect there might not be much of a reaction.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type
2023-10-19 7:48 ` [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type Jan Beulich
@ 2023-10-19 9:35 ` Andrew Cooper
2023-10-19 9:39 ` Jan Beulich
0 siblings, 1 reply; 23+ messages in thread
From: Andrew Cooper @ 2023-10-19 9:35 UTC (permalink / raw)
To: Jan Beulich, Juergen Gross
Cc: javi.merino, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, Roger Pau Monné, Paul Durrant, Bertrand Marquis,
Volodymyr Babchuk, Tamas K Lengyel, Lukasz Hawrylko,
Daniel P. Smith, Mateusz Mówka, xen-devel
On 19/10/2023 8:48 am, Jan Beulich wrote:
> On 13.10.2023 11:42, Juergen Gross wrote:
>> Instead of being able to use normal spinlocks as recursive ones, too,
>> make recursive spinlocks a special lock type.
>>
>> This will make the spinlock structure smaller in production builds and
>> add type-safety.
>>
>> This allows to increase the maximum number of physical cpus from 8191
>> to 65535 without increasing the size of the lock structure in production
>> builds (the size of recursive spinlocks in debug builds will grow to
>> 12 bytes due to that change).
>>
>> Changes in V2:
>> - addressed comments by Jan Beulich
>> - lots of additional cleanups
>> - reorganized complete series
>>
>> Juergen Gross (13):
>> xen/spinlock: fix coding style issues
>> xen/spinlock: reduce lock profile ifdefs
>> xen/spinlock: make spinlock initializers more readable
>> xen/spinlock: introduce new type for recursive spinlocks
>> xen/spinlock: rename recursive lock functions
>> xen/spinlock: add rspin_[un]lock_irq[save|restore]()
>> xen/spinlock: make struct lock_profile rspinlock_t aware
>> xen/spinlock: add explicit non-recursive locking functions
>> xen/spinlock: add another function level
>> xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
>> xen/spinlock: split recursive spinlocks from normal ones
>> xen/spinlock: remove indirection through macros for spin_*() functions
>> xen/spinlock: support higher number of cpus
> Before looking at patches 4 and onwards, I'd like us to settle on the future
> of recursive locking in Xen, considering in particular Andrew's objections
> to their use in the code base. If the plan was to eliminate them, I'd see
> little point in reworking the infrastructure. I'd like to suggest that one
> of us tries to remember to put this up as an agenda item for the next
> Community Call. But of course the discussion can also happen right here; I
> merely expect there might not be much of a reaction.
Actually, I consider this series an improvement. The CPU limit is the
most urgent problem to fix.
XenServer has just jumped to NR_CPUS=2k in order to support 2024's range
of hardware, and it's only going to be a couple of years more before
we're stuck given the current spinlocks.
I do genuinely think the code and logic would be better without
recursive locks, but making that happen is going to be very invasive and
complicated.
But in the meantime with spinlocks properly separated from recursive
locks, it becomes easier IMO to dissuade the introduction of new cases
while we unpick the existing ones.
And so what if we do end up deleting recursive locks in a few years
time? That's not an argument against doing this untangling now.
~Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type
2023-10-19 9:35 ` Andrew Cooper
@ 2023-10-19 9:39 ` Jan Beulich
2023-10-19 11:05 ` Andrew Cooper
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2023-10-19 9:39 UTC (permalink / raw)
To: Andrew Cooper
Cc: javi.merino, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, Roger Pau Monné, Paul Durrant, Bertrand Marquis,
Volodymyr Babchuk, Tamas K Lengyel, Lukasz Hawrylko,
Daniel P. Smith, Mateusz Mówka, xen-devel, Juergen Gross
On 19.10.2023 11:35, Andrew Cooper wrote:
> On 19/10/2023 8:48 am, Jan Beulich wrote:
>> On 13.10.2023 11:42, Juergen Gross wrote:
>>> Instead of being able to use normal spinlocks as recursive ones, too,
>>> make recursive spinlocks a special lock type.
>>>
>>> This will make the spinlock structure smaller in production builds and
>>> add type-safety.
>>>
>>> This allows to increase the maximum number of physical cpus from 8191
>>> to 65535 without increasing the size of the lock structure in production
>>> builds (the size of recursive spinlocks in debug builds will grow to
>>> 12 bytes due to that change).
>>>
>>> Changes in V2:
>>> - addressed comments by Jan Beulich
>>> - lots of additional cleanups
>>> - reorganized complete series
>>>
>>> Juergen Gross (13):
>>> xen/spinlock: fix coding style issues
>>> xen/spinlock: reduce lock profile ifdefs
>>> xen/spinlock: make spinlock initializers more readable
>>> xen/spinlock: introduce new type for recursive spinlocks
>>> xen/spinlock: rename recursive lock functions
>>> xen/spinlock: add rspin_[un]lock_irq[save|restore]()
>>> xen/spinlock: make struct lock_profile rspinlock_t aware
>>> xen/spinlock: add explicit non-recursive locking functions
>>> xen/spinlock: add another function level
>>> xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
>>> xen/spinlock: split recursive spinlocks from normal ones
>>> xen/spinlock: remove indirection through macros for spin_*() functions
>>> xen/spinlock: support higher number of cpus
>> Before looking at patches 4 and onwards, I'd like us to settle on the future
>> of recursive locking in Xen, considering in particular Andrew's objections
>> to their use in the code base. If the plan was to eliminate them, I'd see
>> little point in reworking the infrastructure. I'd like to suggest that one
>> of us tries to remember to put this up as an agenda item for the next
>> Community Call. But of course the discussion can also happen right here; I
>> merely expect there might not be much of a reaction.
>
> Actually, I consider this series an improvement. The CPU limit is the
> most urgent problem to fix.
>
> XenServer has just jumped to NR_CPUS=2k in order to support 2024's range
> of hardware, and it's only going to be a couple of years more before
> we're stuck given the current spinlocks.
>
> I do genuinely think the code and logic would be better without
> recursive locks, but making that happen is going to be very invasive and
> complicated.
>
> But in the meantime with spinlocks properly separated from recursive
> locks, it becomes easier IMO to dissuade the introduction of new cases
> while we unpick the existing ones.
>
> And so what if we do end up deleting recursive locks in a few years
> time? That's not an argument against doing this untangling now.
Of course if that was happening only in a few years time, the series
here is still worthwhile to have. My question was rather towards us
possibly eliminating recursive locks in the next release cycle.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 00/13] xen/spinlock: make recursive spinlocks a dedicated type
2023-10-19 9:39 ` Jan Beulich
@ 2023-10-19 11:05 ` Andrew Cooper
0 siblings, 0 replies; 23+ messages in thread
From: Andrew Cooper @ 2023-10-19 11:05 UTC (permalink / raw)
To: Jan Beulich
Cc: javi.merino, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, Roger Pau Monné, Paul Durrant, Bertrand Marquis,
Volodymyr Babchuk, Tamas K Lengyel, Lukasz Hawrylko,
Daniel P. Smith, Mateusz Mówka, xen-devel, Juergen Gross
On 19/10/2023 10:39 am, Jan Beulich wrote:
> On 19.10.2023 11:35, Andrew Cooper wrote:
>> On 19/10/2023 8:48 am, Jan Beulich wrote:
>>> On 13.10.2023 11:42, Juergen Gross wrote:
>>>> Instead of being able to use normal spinlocks as recursive ones, too,
>>>> make recursive spinlocks a special lock type.
>>>>
>>>> This will make the spinlock structure smaller in production builds and
>>>> add type-safety.
>>>>
>>>> This allows to increase the maximum number of physical cpus from 8191
>>>> to 65535 without increasing the size of the lock structure in production
>>>> builds (the size of recursive spinlocks in debug builds will grow to
>>>> 12 bytes due to that change).
>>>>
>>>> Changes in V2:
>>>> - addressed comments by Jan Beulich
>>>> - lots of additional cleanups
>>>> - reorganized complete series
>>>>
>>>> Juergen Gross (13):
>>>> xen/spinlock: fix coding style issues
>>>> xen/spinlock: reduce lock profile ifdefs
>>>> xen/spinlock: make spinlock initializers more readable
>>>> xen/spinlock: introduce new type for recursive spinlocks
>>>> xen/spinlock: rename recursive lock functions
>>>> xen/spinlock: add rspin_[un]lock_irq[save|restore]()
>>>> xen/spinlock: make struct lock_profile rspinlock_t aware
>>>> xen/spinlock: add explicit non-recursive locking functions
>>>> xen/spinlock: add another function level
>>>> xen/spinlock: add missing rspin_is_locked() and rspin_barrier()
>>>> xen/spinlock: split recursive spinlocks from normal ones
>>>> xen/spinlock: remove indirection through macros for spin_*() functions
>>>> xen/spinlock: support higher number of cpus
>>> Before looking at patches 4 and onwards, I'd like us to settle on the future
>>> of recursive locking in Xen, considering in particular Andrew's objections
>>> to their use in the code base. If the plan was to eliminate them, I'd see
>>> little point in reworking the infrastructure. I'd like to suggest that one
>>> of us tries to remember to put this up as an agenda item for the next
>>> Community Call. But of course the discussion can also happen right here; I
>>> merely expect there might not be much of a reaction.
>> Actually, I consider this series an improvement. The CPU limit is the
>> most urgent problem to fix.
>>
>> XenServer has just jumped to NR_CPUS=2k in order to support 2024's range
>> of hardware, and it's only going to be a couple of years more before
>> we're stuck given the current spinlocks.
>>
>> I do genuinely think the code and logic would be better without
>> recursive locks, but making that happen is going to be very invasive and
>> complicated.
>>
>> But in the meantime with spinlocks properly separated from recursive
>> locks, it becomes easier IMO to dissuade the introduction of new cases
>> while we unpick the existing ones.
>>
>> And so what if we do end up deleting recursive locks in a few years
>> time? That's not an argument against doing this untangling now.
> Of course if that was happening only in a few years time, the series
> here is still worthwhile to have. My question was rather towards us
> possibly eliminating recursive locks in the next release cycle.
I find it unlikely that we'd manage to do it in the next release cycle,
even if we did dedicate a significant portion of effort to it. There
are more urgent things too.
But the same argument works with several years substituted for months.
~Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread