Re: [PATCH v3 5/7] x86/resctrl: Display the RMID and COSID for resctrl groups

[Moger, Babu]

Hi Reinette,

On 3/15/23 13:42, Reinette Chatre wrote:
> Hi Babu,
> 
> Please note "COSID" (CLOSID?) in the subject. Even so, you already
> know and have been reminded when you submitted an earlier version
> that resctrl needs to support Arm. Adding x86 specific bits to the
> user interface complicates this enabling.
> 
> What happened to:
> https://lore.kernel.org/lkml/9ca06669-7826-b3b7-0977-02185be7ce08@xxxxxxx/

Yes. It kind of loses meaning if is renamed differently. Kept it same.
I will change it to mon_hw_id and ctrl_hw_id respectively. Will change
subject line accordingly.

> 
> On 3/2/2023 12:24 PM, Babu Moger wrote:
>> When a user creates a control or monitor group, the CLOSID or RMID
>> are not visible to the user. These are architecturally defined entities.
>> There is no harm in displaying these in resctrl groups. Sometimes it
>> can help to debug the issues.
>>
>> Add CLOSID and RMID to the control/monitor groups display in resctrl
>> interface.
>>
>> $cat /sys/fs/resctrl/clos1/closid
>> 1
>> $cat /sys/fs/resctrl/mon_groups/mon1/rmid
>> 3
>>
>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>> ---
>>  Documentation/x86/resctrl.rst          |   17 ++++++++++++
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c |   44 ++++++++++++++++++++++++++++++++
>>  2 files changed, 61 insertions(+)
>>
>> diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
>> index 25203f20002d..67eae74fe40c 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -321,6 +321,15 @@ All groups contain the following files:
>>  	Just like "cpus", only using ranges of CPUs instead of bitmasks.
>>  
>>  
>> +"rmid":
>> +	Available only with debug option.Reading this file shows the
> 
> Where can the user find documentation about "debug option"? 

I can change the patch order. I can bring patch 7 before this.

> 
> Also please watch spacing.

Ok sure.

> 
>> +	Resource Monitoring ID (RMID) for monitoring the resource
>> +	utilization. Monitoring is performed by tagging each core (or
>> +	thread) or process via a RMID. Kernel assigns a new RMID when
>> +	a group is created depending on the available RMIDs. Multiple
>> +	cores (or threads) or processes can share a same RMID in a resctrl
>> +	domain.
> 
> Please make this not to be x86 specific. You can surely document the
> x86 details but that should start with something like "on x86 this ..."

ok. Sure
> 
> What does "a resctrl domain" mean to user space? Did you mean "resource
> group"?

Yes. it should have been "resource group".
> 
>> +
>>  When control is enabled all CTRL_MON groups will also contain:
>>  
>>  "schemata":
>> @@ -342,6 +351,14 @@ When control is enabled all CTRL_MON groups will also contain:
>>  	file. On successful pseudo-locked region creation the mode will
>>  	automatically change to "pseudo-locked".
>>  
>> +"closid":
>> +	Available only with debug option. Reading this file shows the
>> +	Class of Service (CLOS) id which acts as a resource control tag
>> +	on which the resources can be throttled. Kernel assigns a new
>> +	CLOSID a control group is created depending on the available
>> +	CLOSIDs. Multiple cores(or threads) or processes can share a
>> +	same CLOSID in a resctrl domain.
>> +
> 
> This also should not be x86 specific. Also, please check the language
> and watch spacing. 

ok
> for example, "Kernel assigns a new CLOSID a control group" -> "Kernel
> assigns a new CLOSID to a control group", "can share a same" -> "can
> share the same".

Sure
> What does "a resctrl domain" mean to user space?

It should have been "resource group".

> 
> 
>>  When monitoring is enabled all MON groups will also contain:
>>  
> 
> Shouldn't the "rmid" (to be renamed) entry be in this section of the
> documentation?

Not sure about this comment. Did you mean to move the rmid (to be renamed
mon_hw_id) documentation here?

Thanks
Babu

> 
>>  "mon_data":
> 
> Reinette
> 

-- 
Thanks
Babu Moger