Re: [PATCH v7 2/3] xen/events: modify struct evtchn layout

[Jürgen Groß <jgross@xxxxxxxx>]

On 24.11.20 13:37, Jan Beulich wrote:
> On 24.11.2020 13:18, Jürgen Groß wrote:
> > On 24.11.20 12:42, Jan Beulich wrote:
> > > On 24.11.2020 08:01, Juergen Gross wrote:
> > > > @@ -94,9 +93,10 @@ struct evtchn
> > > >    #define ECS_VIRQ         5 /* Channel is bound to a virtual IRQ line.        
> > > > */
> > > >    #define ECS_IPI          6 /* Channel is bound to a virtual IPI line.        
> > > > */
> > > >        u8  state;             /* ECS_* */
> > > > -    u8  xen_consumer:XEN_CONSUMER_BITS; /* Consumer in Xen if nonzero */
> > > I see no reason to use a full byte for this one; in fact I
> > > was considering whether it, state, and old_state couldn't
> > > share storage (the latest when we run into space issues with
> > > this struct). (In this context I'm also observing that
> > > old_state could get away with just 2 bits, i.e. all three
> > > fields would fit in a single byte.)
> > I think doing further compression now isn't really helping. It would
> > just add more padding bytes and result in larger code.
> I'm not meaning to ask to widen the use of bitfields right now
> (unless this helps avoiding holes). But I'd like to not see the
> one non-problematic use go away without this really being
> necessary.
Okay.

> > > Also for all fields you touch anyway, may I ask that you switch to
> > > uint<N>_t or, in the case of "pending", bool?
> > Fine with me.
> >
> > Would you object to switching the whole structure in this regard?
> I didn't dare to suggest you doing so. So no, I wouldn't mind.
> However, there's more room then for what some would possibly
> call bike shedding: The wider the scope of the conversion you
> do the more relevant it'll become that strictly speaking there
> ought to be (almost?) no use of fixed width types here, as per
> ./CODING_STYLE.
No problem with that.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature