On 20/11/2023 11:38, 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 <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 17716fc4eb..65f180203a 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 */
LGTM.
Acked-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Cheers,
Alejandro