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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux