On 12.12.23 19:42, Julien Grall wrote:
Hi,On 12/12/2023 09:47, Juergen Gross wrote: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 <[email protected]> Acked-by: Alejandro Vallejo <[email protected]> --- 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 c1a9ba1304..7d611d3d7d 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 05b97c1e03..ac3bef267a 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 */This is meant to act like a bool. So I would prefer if we use: bool is_rwlock:1; And then use true/false when set.
Do we want to do that? AFAIK it would depend on the compiler what the size of the struct is when mixing types in bitfields (in this case: bool and uint64_t).
Acked-by: Julien Grall <[email protected]>
Thanks, Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
