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

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

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

> 
> Add CLOSID(ctrl_hw_id) and RMID(mon_hw_id) to the control/monitor groups

Please highlight that CLOSID and RMID are x86 concepts.

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

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

Similar feedback to the "mon_hw_id" portion.

Reinette