Re: [PATCH v4 08/39] x86/resctrl: Generate default_ctrl instead of sharing it

From: James Morse
Date: Fri Sep 13 2024 - 14:08:04 EST


Hi Reinette,

On 14/08/2024 05:00, Reinette Chatre wrote:
> On 8/2/24 10:28 AM, James Morse wrote:
>
>> +/**
>> + * resctrl_get_default_ctrl() - Return the default control value for this
>> + *                              resource.
>> + * @r:        The resource whose default control type is queried.
>> + */
>> +static inline u32 resctrl_get_default_ctrl(struct rdt_resource *r)
>> +{
>> +    switch (r->schema_fmt) {
>> +    case RESCTRL_SCHEMA_BITMAP:
>> +        return BIT_MASK(r->cache.cbm_len) - 1;
>> +    case RESCTRL_SCHEMA_PERCENTAGE:
>> +        return 100u;
>> +    case RESCTRL_SCHEMA_MBPS:
>> +        return r->membw.max_bw;
>> +    }
>> +
>> +    return WARN_ON_ONCE(1);
>> +}
>> +

> I am concerned where this is headed. Since RESCTRL_SCHEMA_PERCENTAGE remains
> in use when resctrl is mounted with mba_MBps the default cannot always
> be 100u (it should be MBA_MAX_MBPS when software controller is active).

I agree - and we can certainly tidy that up.
But today when mba_sc is enable the bandwidth_gran and min-bandwidth files both report
'10' (%?), which isn't particularly meaningful.
I think these should both report '1'. There will be a minimum bandwidth, buts it not
something that can be discovered by the mba_sc code.

This was an oversight because the mba_sc mode doesn't update default_ctrl or the format
strings - it hijacks the parsing elsewhere. The default_ctrl isn't visible to user-space,
the value used when reading the schema file comes from the mbps_val array, instead of
ctrl_val.


Some of this has been booted over the horizon - I'll add straightening out the mba_sc
behaviour here to that list.



Thanks,

James