Re: 32bit resctrl? (was Re: [PATCH v2] fs/resctrl: fix domid loss precision issue)

[James Morse <james.morse@arm.com>]

Hi guys,

On 15/03/2024 16:56, Peter Newman wrote:
> On Fri, Mar 15, 2024 at 9:17 AM Moger, Babu <bmoger@amd.com> wrote:
>> On 3/14/2024 10:25 AM, Reinette Chatre wrote:
>>> +x86 maintainers, Tony, Babu, Peter
>>>
>>> Hi Everybody,
>>>
>>> On 3/12/2024 12:53 AM, Rex Nie wrote:
>>>> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
>>>> index 7a6f46b4edd0..096317610949 100644
>>>> --- a/fs/resctrl/internal.h
>>>> +++ b/fs/resctrl/internal.h
>>>> @@ -94,7 +94,7 @@ union mon_data_bits {
>>>>      struct {
>>>>              unsigned int rid                : 10;
>>>>              enum resctrl_event_id evtid     : 8;
>>>> -            unsigned int domid              : 14;
>>>> +            u32                             domid;
>>>>      } u;
>>>>   };
>>>>
>>> resctrl currently supports 32bit builds. Fixing this issue* in this way
>>
>> I have never bothered about 32bit builds.   Is Intel still testing 32bit
>> builds?
> 
> I can confirm we don't have any 32-bit builds.
> 
> 
>> The structure pointer "union mon_data_bits priv;" is created in stack
>> and passed to create mondata directory. We are reading it later again in
>> rdtgroup_mondata_show.
>>
>> How is this pointer valid again?  Shouldn't we use static pointer or
>> allocate memory for the pointer?
> 
> The union is copied by value into the pointer-sized field, hence the
> need for pointers to be large enough to hold this value.

Couldn't we allocate the memory for a structure to hold the values we want, then use the
pointer as a pointer?

I suspect whether 32bit builds are important depends on if anyone is using it, which we
can't really know. Debian has 32bit builds, and while its unlikely anyone runs that on a
server part, whatever an "Intel Celeron J3455" is supports RDT too. I'd be keen not to
break it!


As for these eye-sore-ids ... I'm in two minds as to whether we should clean them up in
the kernel. It would be fairly straightforward to scan the PPTT to find them all and map
them to 0,1,2,. But this loses the values provided by the vendor.
x86 and arm64:device-tree systems generate them, so its not clear that user-space needs a
value provided by the vendor here.


Thanks,

James