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

[Moger, Babu]

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@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 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