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

[Juergen Gross <jgross@xxxxxxxx>]

On 13.12.23 09:36, Jan Beulich wrote:
> 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.
Okay, fine with me.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature