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

From: Moger, Babu
Date: Wed Jan 11 2023 - 19:41:21 EST


Hi Reinette,

On 1/11/2023 4:56 PM, Reinette Chatre wrote:
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@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?
I referred to the guidance in https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FY7xjxUj%2BKnOEJssZ%40zn.tnic%2F&data=05%7C01%7CBabu.Moger%40amd.com%7C900eb41c0e6049dd342208daf4270d2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090745842366944%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=%2F5GVOhnxq1%2B3nJwGtlApvLfC%2FeX3X9RDaUZa9R92NiY%3D&reserved=0
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.
ok. Got it.

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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2FMW3PR12MB455384130AF0BDE3AF88BCF095FE9%40MW3PR12MB4553.namprd12.prod.outlook.com%2F&data=05%7C01%7CBabu.Moger%40amd.com%7C900eb41c0e6049dd342208daf4270d2b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638090745842366944%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kE7d0cFYyJq1n4ZKKeeF%2FC%2FFDDJy0Sc%2Fd5MZ%2Bc56WQw%3D&reserved=0
can be dropped.
Sure.
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.


How about this? I want to get this right.. Hopefully next version can be final.

diff --git a/Documentation/x86/resctrl.rst b/Documentation/x86/resctrl.rst
index 2860856f4463..7df5889237f4 100644
--- a/Documentation/x86/resctrl.rst
+++ b/Documentation/x86/resctrl.rst
@@ -24,10 +24,15 @@ 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"
+SMBA (Slow Memory Bandwidth Allocation)         ""
+BMEC (Bandwidth Monitoring Event Configuration) ""
 =============================================== ================================

+Historically, new features were made visible by default in /proc/cpuinfo. This
+resulted in the feature flags becoming hard to parse by the 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.
+
 To use the feature mount the file system::

  # mount -t resctrl resctrl [-o cdp[,cdpl2][,mba_MBps]] /sys/fs/resctrl


thanks

Babu