Re: [Xen-devel] [PATCH 1/4] xen/spinlocks: in debug builds store cpu holding the lock

[Juergen Gross <jgross@xxxxxxxx>]

On 08.08.19 14:18, Jan Beulich wrote:
> On 08.08.2019 13:53, Juergen Gross wrote:
> > On 08.08.19 12:28, Julien Grall wrote:
> > > On 08/08/2019 08:51, Juergen Gross wrote:
> > > > On 08.08.19 08:58, Jan Beulich wrote:
> > > > > On 07.08.2019 16:31, Juergen Gross wrote:
> > > > > Do we have an implied assumption somewhere that unsigned short is
> > > > > exactly 16 bits wide? I think "val" wants to be uint16_t here (as
> > > > > you really mean "exactly 16 bits"), the two boolean fields want
> > > > > to be bool, and the remaining two ones unsigned int.
> > > > But that would increase the size of the union to 4 bytes instead of 2.
> > > > So at least pad and cpu must be unsigned short or (better) uint16_t.
> > > How about bool irq_safe:1?
> > I didn't question that, but OTOH I'm not sure doing something like:
> >
> > struct {
> >     bool     unseen:1;
> >     bool     irq_safe:1;
> >     uint16_t pad:2;
> >     uint16_t cpu:12;
> > }
> >
> > is guaranteed to be 2 bytes long. I think pad will be still put into the
> > first byte, but the compiler might decide that the 4 bits left won't be
> > able to hold the next 12 bits so it could start a new uint16_t at offset
> > 2.
> >
> > Moving the bool types to the end of the struct would avoid that IMHO.
> Moving the two bool-s further down will also simplify extraction and
> insertion of the "cpu" field.
Okay, lets reverse above struct.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel