Re: [PATCH v4 6/7] x86/resctrl: Display CLOSID and RMID for the resctrl groups

["Moger, Babu" <bmoger@amd.com>]

Hi Reinette,

On 5/4/2023 2:04 PM, Reinette Chatre wrote:
> Hi Babu,
>
> On 4/17/2023 4:34 PM, Babu Moger wrote:
>> When a user creates a control or monitor group, the CLOSID or RMID
>> are not visible to the user. It can help to debug the issues in some
>> cases. There are only available with "-o debug" option.
> Please see: Documentation/process/maintainer-tip.rst
>
> "It's also useful to structure the changelog into several paragraphs and not
> lump everything together into a single one. A good structure is to explain
> the context, the problem and the solution in separate paragraphs and this
> order."
ok Sure.
>> Add CLOSID(ctrl_hw_id) and RMID(mon_hw_id) to the control/monitor groups
> Please highlight that CLOSID and RMID are x86 concepts.
ok Sure.
>
>> display in resctrl interface.
>> $cat /sys/fs/resctrl/clos1/clos_hw_id
>> 1
> This example does not match what the patch does (clos_hw_id -> ctrl_hw_id).
My bad. Will fix it.
> I also think this change would be more palatable (to non x86 audience) if
> the example resource group has a generic (non-x86 concept) name.

ok. In this example the clos1 name sounds x86 specific. I can change it 
to ctrl_grp1. Hope this is what you meant.


>
>> $cat /sys/fs/resctrl/mon_groups/mon1/mon_hw_id
>> 3
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>>   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 be443251b484..5aff8c2beb08 100644
>> --- a/Documentation/x86/resctrl.rst
>> +++ b/Documentation/x86/resctrl.rst
>> @@ -345,6 +345,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".
>>   
>> +"ctrl_hw_id":
>> +	Available only with debug option. On x86, 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 group.
> Please keep other content from the documentation in mind when making
> this change. CLOSID is already documented, including the fact that it
> is a limited resource. Please see content under: "Notes on cache occupancy
> monitoring and control" where it, for example, states that "The number
> of CLOSid and RMID are limited by the hardware."
>
> Considering this the above could just read:
> "Available only with debug option. The identifier used by hardware
>   for the control group. On x86 this is the CLOSID."
ok. Sure.
>
> Similar feedback to the "mon_hw_id" portion.

Sure.

Thanks

Babu