Re: [PATCH v4 06/12] xen/spinlock: make struct lock_profile rspinlock_t aware

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.12.2023 07:05, Juergen Gross wrote:
> On 12.12.23 19:42, Julien Grall wrote:
>> On 12/12/2023 09:47, Juergen Gross wrote:
>>> --- 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).

I thought in a similar way as you did when Andrew introduced similar
patterns (see Julien's reply for an example), and was then convinced
that the compiler really is supposed to be doing what we want here.
So yes, I second Julien's desire to have bool used when boolean is
meant.

Jan