Re: [RESEND PATCH v4 04/15] fs/resctrl: Introduce kernel mode (kmode) data structures
Babu Moger <[email protected]>
| Newsgroups | org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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/[email protected]/ >>>>> >>>>> 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/[email protected]/) >>>>> "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