Re: [PATCH gnumach] Enable MACH_HOST and fix non-addressable bitfields

[Samuel Thibault]

Damien Zammit, le dim. 11 févr. 2024 11:30:31 +0000, a ecrit:
> On 2/11/24 10:07 PM, Samuel Thibault wrote:
> > Damien Zammit, le dim. 11 févr. 2024 10:55:26 +0000, a ecrit:
> >>>> diff --git a/kern/task.h b/kern/task.h
> >>>> index dec3a530..27970620 100644
> >>>> --- a/kern/task.h
> >>>> +++ b/kern/task.h
> >>>> @@ -61,11 +61,11 @@ struct task {
> >>>>          decl_simple_lock_data(,lock)    /* Task's lock */
> >>>>          int             ref_count;      /* Number of references to me */
> >>>>
> >>>> -        /* Flags */
> >>>> -        unsigned int    active:1,       /* Task has not been terminated 
> >>>> */
> >>>> -        /* boolean_t */ may_assign:1,   /* can assigned pset be 
> >>>> changed? */
> >>>> -                        assign_active:1,        /* waiting for 
> >>>> may_assign */
> >>>> -                        essential:1;    /* Is this task essential for 
> >>>> the system? */
> >>>> +        /* Addressable flags */
> >>>> +        unsigned char   active;         /* Task has not been terminated 
> >>>> */
> >>>> +        unsigned char   may_assign;     /* can assigned pset be 
> >>>> changed? */
> >>>> +        unsigned char   assign_active;  /* waiting for may_assign */
> >>>> +        unsigned char   essential;      /* Is this task essential for 
> >>>> the system? */
> >>> AIUI only assign_active need to be adressable? Better make only that one
> >>> addressable.
> >> Looking at the existing flag types, it needs to fit into part of a 
> >> cacheline.
> > Why would it need to fit in a cacheline?
> 
> See comment above struct task:
> 
>   /*
>    * Task name buffer size.  The size is chosen so that struct task fits
>    * into three cache lines.  The size of a cache line on a typical CPU
>    * is 64 bytes.
>    */
>   #define TASK_NAME_SIZE 32

Ok, but that's only for optimization.

> >> The way I have done it, I rearranged the existing 4 byte integer to be 4 
> >> separate single byte flags.
> >> If you want me to put only one of the values into an addressable field,
> >> how will I retain the same size for the rest of the fields?
> > What do you mean by "the same size"?
> 
> I think the actual sizeof(struct task) and its layout matters.

Ok

> Therefore, my original code change here preserves most of the layout and 
> sizeof(struct task).

The original code uses 4 bits, so a byte. Your proposed change uses 4
bytes, so 3 more bytes. By using e.g.

        unsigned char may_assign,       /* can assigned pset be changed? */
        unsigned int    active:1,       /* Task has not been terminated */
                        assign_active:1,        /* waiting for may_assign */
                        essential:1;    /* Is this task essential for the 
system? */

That'll be 2 bytes, so even better for cache line size.

Samuel