Re: [RESEND PATCH v4 04/15] fs/resctrl: Introduce kernel mode (kmode) data structures

[Babu Moger <babu.moger@amd.com>]

Hi Reinette,

On 8/13/26 13:18, Reinette Chatre wrote:
> Hi Babu,
> 
> On 8/13/26 10:12 AM, Babu Moger wrote:
>> Hi Reinette,
>>
>> On 8/13/26 10:55, Reinette Chatre wrote:
>>> Hi Babu,
>>>
>>> On 8/13/26 8:17 AM, Babu Moger wrote:
>>>> On 8/12/26 18:28, Reinette Chatre wrote:
>>>
>>>>> Please always keep in mind all the requirements and use cases we learned about
>>>>> during and after RFC v1 of this work.
>>>>>
>>>>> For example, we already know that "per group" assignment is something resctrl
>>>>> needs to be ready for. Consider the example in
>>>>> https://lore.kernel.org/lkml/aYyxAPdTFejzsE42@e134344.arm.com/
>>>>>
>>>>> There may even be "per task" assignment in the future.
>>>>
>>>> Yes. That is correct.
>>>>
>>>>>
>>>>> Constraining this feature to PLZA will make it harder to enable the capabilities
>>>>> that we know resctrl need to support in the future.
>>>>>
>>>>> This is how we originally landed on the "global" assignment distinction
>>>>> (https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/)
>>>>> "global assignment" should be kept or replaced with a solution that continues to
>>>>> prepare resctrl for these other capabilities.
>>>>
>>>> Yes. Makes sense.
>>>>
>>>>>
>>>>> I am not able to see how resctrl could support "per group" assignment with the
>>>>> interface you propose above. If I am missing this, please highlight the solution.
>>>>>
>>>>> resctrl may need to explicitly split kernel mode from kernel mode properties. For
>>>>> example, below shows an "assign_global_enable_per_cpu" as the kernel mode, now with three
>>>>> properties:
>>>>> - "ctrl" - could be "assign" or "inherit"
>>>>> - "mon" - could be "assign" or "inherit"
>>>>> - "group" - required if "ctrl" or "mon" is set to "assign"
>>>>
>>>> ok.
>>>>
>>>>
>>>>>
>>>>>       # cat info/kernel_mode
>>>>>       [inherit]
>>>>>       assign_global_enable_per_cpu:ctrl=assign;mon=assign;group=uninitialized
>>>>
>>>> Shouldn't this be like below when default is inherit?
>>>>
>>>> # cat info/kernel_mode
>>>> [inherit]
>>>> assign_global_enable_per_cpu
>>>>
>>>
>>> I think it will be useful to let the interface:
>>>    (a) show to user space which properties are available for each supported mode, and
>>>    (b) show user space what the default value of those properties will be if they
>>>        are not set when the associated mode is enabled.
>>
>> The "inherit" is the default property if it is not explicitly set. Right?
> 
> "property" is different from "kernel mode"
> 
> Each "kernel mode" can have zero or more properties.
> "inherit" is the default "kernel mode", but it could have a better/more descriptive name.
> For example, "inherit_from_user" or ...?

Or "user_inherit_kernel" ?

> 
>>
>> # cat info/kernel_mode
>> [inherit]
>> assign_global_enable_per_cpu:ctrl=inherit;mon=inherit;group=uninitialized
>>
>>
>> If the intention is to display all supported values for each
>> property, then we should do so consistently for all properties. For
>> example:
> 
> No. The intention is not to display all supported values for properties of the
> different kernel modes. Just display which properties are supported and what value resctrl
> would use if the user enables that mode without providing a value for a particular
> property. There may be properties that could have values for which it will
> be difficult to provide all supported values.
> 
> For example, if "kernel_mode" contains:
>        # cat info/kernel_mode
>        [inherit]
>        assign_global_enable_per_cpu:ctrl=assign;mon=assign;group=//
> 
> Then user space knows that if they enable "assign_global_enable_per_cpu" kernel mode
> without providing any properties then all kernel work will use the default resource
> group's allocation and monitoring. If that is not what user space wants then they
> can change the value of only the properties they need to change.

That sounds reasonable.

When the group is bound to the global mode,

# cat info/kernel_mode
   inherit
   [assign_global_enable_per_cpu:ctrl=assign;group=ctrl1//]

> 
> 
>>
>> # cat info/kernel_mode
>> [inherit]
>> assign_global_enable_per_cpu:ctrl=inherit,assign;mon=inherit,assign;group=uninitialized
>>
>>
>>>
>>> To (b), since the default resource group is used as default when the group
>>> is not provided it may be better to use "//" (or just "/", depending on system
>>> supporting just one of allocation or monitoring) instead of "uninitialized".
>>>
>>
>> To me, showing the default group would be confusing. It could give
>> the impression that only the default group is associated with the
>> [inherit] mode, even when multiple resource groups exist.
> 
> I do not see how default group is associated with the "inherit" mode since inherit mode
> does not have a "group" property?

Yes. Got it.

Thanks,
Babu