[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 06/12] xen/spinlock: make struct lock_profile rspinlock_t aware
On 28.02.2024 16:43, Jürgen Groß wrote: > On 28.02.24 16:19, Jan Beulich wrote: >> On 12.12.2023 10:47, Juergen Gross wrote: >>> --- 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; >> >> Any reason for this not being unsigned int as well? The more that ... >> >>> + 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; >>> + } > > I've used the same type as 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); >> >> ... it's then printed with plain x as the format char. > > Which hasn't been changed by the patch. I can change it to PRIx32 if you want. As per ./CODING_STYLE unsigned int is preferred. >>> --- 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 */ >>> + }; >> >> _LOCK_PROFILE() wants to initialize this field, unconditionally using >> .lock. While I expect that problem to be taken care of in one of the >> later patches, use of the macro won't work anymore with this union in >> use with very old gcc that formally we still support. While a road to >> generally raising the baseline requirements is still pretty unclear to >> me, an option might be to require (and document) that to enable >> DEBUG_LOCK_PROFILE somewhat newer gcc needs using. > > Patch 8 is using either .lock or .rlock depending on the lock type. > > What is the problem with the old gcc version? Static initializers of > anonymous union members? Yes. Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |