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

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

Hi Reinette,

On 10/19/26 11:20, Reinette Chatre wrote:
> 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
> 
> 

That is correct. It is confusing. Let me try to revise these terms in 
next revision and go from there.


>>>> +        "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.

ok. Will try to handle it.

> 
> 
> ...
> 
>>>> +
>>>> +            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.
> 

Good point. This is something new that I need to handle here, so I'll 
work on fixing it.

Thanks,
Babu
Babu