RE: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new features

[Moger, Babu]

[AMD Official Use Only - General]

Hi Reinette,

> -----Original Message-----
> From: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Sent: Wednesday, January 11, 2023 4:07 PM
> To: Moger, Babu <Babu.Moger@xxxxxxx>; corbet@xxxxxxx;
> tglx@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; bp@xxxxxxxxx
> Cc: fenghua.yu@xxxxxxxxx; dave.hansen@xxxxxxxxxxxxxxx; x86@xxxxxxxxxx;
> hpa@xxxxxxxxx; paulmck@xxxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx;
> quic_neeraju@xxxxxxxxxxx; rdunlap@xxxxxxxxxxxxx;
> damien.lemoal@xxxxxxxxxxxxxxxxxx; songmuchun@xxxxxxxxxxxxx;
> peterz@xxxxxxxxxxxxx; jpoimboe@xxxxxxxxxx; pbonzini@xxxxxxxxxx;
> chang.seok.bae@xxxxxxxxx; pawan.kumar.gupta@xxxxxxxxxxxxxxx;
> jmattson@xxxxxxxxxx; daniel.sneddon@xxxxxxxxxxxxxxx; Das1, Sandipan
> <Sandipan.Das@xxxxxxx>; tony.luck@xxxxxxxxx; james.morse@xxxxxxx;
> linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> bagasdotme@xxxxxxxxx; eranian@xxxxxxxxxx; christophe.leroy@xxxxxxxxxx;
> jarkko@xxxxxxxxxx; adrian.hunter@xxxxxxxxx; quic_jiles@xxxxxxxxxxx;
> peternewman@xxxxxxxxxx
> Subject: Re: [PATCH v11 13/13] Documentation/x86: Update resctrl.rst for new
> features
> 
> Hi Babu,
> 
> On 1/9/2023 8:44 AM, Babu Moger wrote:
> > Update the documentation for the new features:
> > 1. Slow Memory Bandwidth allocation (SMBA).
> >    With this feature, the QOS  enforcement policies can be applied
> >    to the external slow memory connected to the host. QOS enforcement
> >    is accomplished by assigning a Class Of Service (COS) to a processor
> >    and specifying allocations or limits for that COS for each resource
> >    to be allocated.
> >
> > 2. Bandwidth Monitoring Event Configuration (BMEC).
> >    The bandwidth monitoring events mbm_total_bytes and mbm_local_bytes
> >    are set to count all the total and local reads/writes respectively.
> >    With the introduction of slow memory, the two counters are not
> >    enough to count all the different types of memory events. With the
> >    feature BMEC, the users have the option to configure mbm_total_bytes
> >    and mbm_local_bytes to count the specific type of events.
> >
> > Also add configuration instructions with examples.
> >
> > Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
> > ---
> >  Documentation/x86/resctrl.rst | 142
> > +++++++++++++++++++++++++++++++++-
> >  1 file changed, 140 insertions(+), 2 deletions(-)
> >
> > diff --git a/Documentation/x86/resctrl.rst
> > b/Documentation/x86/resctrl.rst index 71a531061e4e..2860856f4463
> > 100644
> > --- a/Documentation/x86/resctrl.rst
> > +++ b/Documentation/x86/resctrl.rst
> > @@ -17,14 +17,16 @@ AMD refers to this feature as AMD Platform Quality
> of Service(AMD QoS).
> >  This feature is enabled by the CONFIG_X86_CPU_RESCTRL and the x86
> > /proc/cpuinfo  flag bits:
> >
> > -=============================================
> 	================================
> > +===============================================
> 	================================
> >  RDT (Resource Director Technology) Allocation	"rdt_a"
> >  CAT (Cache Allocation Technology)		"cat_l3", "cat_l2"
> >  CDP (Code and Data Prioritization)		"cdp_l3", "cdp_l2"
> >  CQM (Cache QoS Monitoring)			"cqm_llc",
> "cqm_occup_llc"
> >  MBM (Memory Bandwidth Monitoring)		"cqm_mbm_total",
> "cqm_mbm_local"
> >  MBA (Memory Bandwidth Allocation)		"mba"
> > -=============================================
> 	================================
> > +SMBA (Slow Memory Bandwidth Allocation)         "smba"
> > +BMEC (Bandwidth Monitoring Event Configuration) "bmec"
> > +===============================================
> 	================================
> >
> 
> I expect that you will follow Boris's guidance here and not make these flags
> visible in /proc/cpuinfo. That would imply that this addition will have no entries
> in the second column. Perhaps this could be made easier to parse by using
> empty quotes ("") in the second column to match syntax used in the existing
> flags as well as the cpufeatures.h change?

Hmm.. I thought we dropped that idea for now. Did I miss understand that?
Thanks
Babu 

> 
> If/when making this change, could you please also add a note that documents
> this new guidance for other resctrl developers? Something like below but I am
> looking forward to
> improvements:
> "Historically new features were made visible by default in /proc/cpuinfo. This
> resulted in the flags field becoming hard to parse by humans. Adding a new
> flag to /proc/cpuinfo should be avoided if user space can obtain information
> about the feature from resctrl's info directory."
> 
> The rest of the document looks good to me.
> 
> Thank you
> 
> Reinette