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

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

Hi Babu,

On 1/11/2023 2:39 PM, Moger, Babu wrote:
> [AMD Official Use Only - General]
> 
> Hi Reinette,
> 
>> -----Original Message-----
>> From: Reinette Chatre <reinette.chatre@intel.com>
>> Sent: Wednesday, January 11, 2023 4:07 PM
>> To: Moger, Babu <Babu.Moger@amd.com>; corbet@lwn.net;
>> tglx@linutronix.de; mingo@redhat.com; bp@alien8.de
>> Cc: fenghua.yu@intel.com; dave.hansen@linux.intel.com; x86@kernel.org;
>> hpa@zytor.com; paulmck@kernel.org; akpm@linux-foundation.org;
>> quic_neeraju@quicinc.com; rdunlap@infradead.org;
>> damien.lemoal@opensource.wdc.com; songmuchun@bytedance.com;
>> peterz@infradead.org; jpoimboe@kernel.org; pbonzini@redhat.com;
>> chang.seok.bae@intel.com; pawan.kumar.gupta@linux.intel.com;
>> jmattson@google.com; daniel.sneddon@linux.intel.com; Das1, Sandipan
>> <Sandipan.Das@amd.com>; tony.luck@intel.com; james.morse@arm.com;
>> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org;
>> bagasdotme@gmail.com; eranian@google.com; christophe.leroy@csgroup.eu;
>> jarkko@kernel.org; adrian.hunter@intel.com; quic_jiles@quicinc.com;
>> peternewman@google.com
>> 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@amd.com>
>>> ---
>>>  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?

I referred to the guidance in https://lore.kernel.org/lkml/Y7xjxUj+KnOEJssZ@zn.tnic/
Since the SMBA and BMEC features have never appeared in /proc/cpuinfo there cannot
be a user space that expects these flags in /proc/cpuinfo and thus no risk of
breaking user space. User space can get information about SMBA and BMEC
from the info directory. 

Later that thread discussed removal of existing resctrl feature flags from
/proc/cpuinfo - that is what I think we shouldn't do since there are 
user space consumers of those flags. I thus agree that the task described in
https://lore.kernel.org/lkml/MW3PR12MB455384130AF0BDE3AF88BCF095FE9@MW3PR12MB4553.namprd12.prod.outlook.com/
can be dropped.

I do not think this is a big change ... just add the empty quotes to the
two cpufeatures.h patches and a new snippet to the resctrl documentation.

Reinette