Re: [PATCH v6 09/18] fs/resctrl: Add interface to display kernel mode status

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Babu,

On 10/16/26 12:10 PM, Moger, Babu wrote:
> On 10/15/2026 10:16 AM, Reinette Chatre wrote:
>> On 10/1/26 8:10 AM, Babu Moger wrote:

>>
>>> +        and whichever of ctrl= and mon= the platform supports::
>>> +
>>> +          global_enable_per_cpu:ctrl=<assign|inherit>;mon=<assign|inherit>;group=<CTRL_MON>/<MON>/
>>> +
>>> +        "ctrl":
>>> +            "assign" if kernel mode work uses the allocation
>>> +            association of the resource group named by "group",
>>> +            "inherit" if it uses the allocation association of
>>> +            the user-space task. Present only on systems that
>>> +            support allocation.
>>
>> ok - this is already late in the series but this seems to make it clear that
>> "allocation association" means the portions of resources allocated to this group.
>> Thus the earlier " enum kmode_state - Control or monitoring association" is wrong?
> 
> I think, it should be " enum kmode_state - Allocation or monitoring association"

Please don't. This is exactly the problem that I keep coming back to. If you
change the name as you suggest then you solidify that you use the same term to mean
two different things. Specifically, what above change implies is that the reader is left
to decipher when:

"allocation and monitoring association" means
	MSR_IA32_PQR_PLZA_ASSOC.closid and MSR_IA32_PQR_PLZA_ASSOC.rmid

vs when

"allocation and monitoring association" means
	MSR_IA32_PQR_PLZA_ASSOC.closid_en and MSR_IA32_PQR_PLZA_ASSOC.rmid_en


>>> +        "mon":
>>> +            "assign" if kernel mode work uses the monitoring
>>> +            association of the resource group named by "group",
>>> +            "inherit" if it uses the monitoring association of
>>> +            the user-space task. Present only on systems that
>>> +            support monitoring.
>>> +        "group":
>>> +            Path of the resource group, relative to the resctrl
>>> +            mount point. A CTRL_MON group is shown as <CTRL_MON>//,
>>
>> Shouldn't it just be <CTRL_MON>/ ?
> 
> Or We can say "<CTRL_MON>/<MON>/" to be generic for both cases.
> 
> "The group is shown as  "<CTRL_MON>/<MON>/".

No, this depends on whether monitoring is enabled or not, or (if monitoring is
supported) whether mon=inherit.


...

>>> +
>>> +            sep = ':';
>>> +            if (resctrl_arch_alloc_capable()) {
>>> +                seq_printf(seq, "%cctrl=%s", sep,
>>> +                       resctrl_kmode_state_str(resctrl_kmode.ctrl));
>>> +                sep = ';';
>>> +            }
>>> +            if (resctrl_arch_mon_capable()) {
>>> +                seq_printf(seq, "%cmon=%s", sep,
>>> +                       resctrl_kmode_state_str(resctrl_kmode.mon));
>>> +                sep = ';';
>>> +            }
>>> +            seq_printf(seq, "%cgroup=%s/%s/", sep, ctrl_grp, mon_grp);
>>
>> Looks like this will print a monitoring group on a system that does not support
>> monitoring? I understand that the monitoring group will be the empty string in this
>> scenario but in resctrl that has meaning of "default monitoring group" so printing
>> this when monitoring is not supported is unexpected.
>>
> 
> Ok. If monitoring not supported it will be "CTRL_MON/" (Example group=ctrl1/") and then default group is "/".
> 
> Is that what you are suggesting?

Yes. This matches the proc output.

> 
> 
>> I also do not expect the monitoring group to be displayed if "mon=inherit" since
>> that presents a contradiction with stating that "monitoring group is inherited"
>> while printing a monitoring group.
> 
> Yes. We should not display "mon=inherit" when monitoring in not supported. Will check on this.
> 
This is needed yes, but what I was actually referring to was that if "mon=inherit"
then the group parameter should not include a monitoring group. For example, "group=ctrl/".
Including the name of a monitoring group when "mon=inherit" is confusing.

Reinette