Hi Reinette, On 9/19/24 11:28, Reinette Chatre wrote: > Hi Babu, > > On 9/4/24 3:21 PM, Babu Moger wrote: >> Introduce the interface file "mbm_assign_mode" to list monitor modes >> supported. >> >> The "mbm_cntr_assign" mode provides the option to assign a hardware >> counter to an RMID and monitor the bandwidth as long as it is assigned. >> >> On AMD systems "mbm_cntr_assign" is backed by the ABMC (Assignable >> Bandwidth Monitoring Counters) hardware feature. "mbm_cntr_assign" mode >> is enabled by default when supported. > > As I understand this series changed this behavior to let the architecture > dictate whether "mbm_cntr_assign" is enabled by default. Yes. Correct. Will change the test to mention that. > >> >> The "default" mode is the existing monitoring mode that works without the >> explicit counter assignment, instead relying on dynamic counter assignment >> by hardware that may result in hardware not dedicating a counter resulting >> in monitoring data reads returning "Unavailable". >> >> Provide an interface to display the monitor mode on the system. >> $cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode >> [mbm_cntr_assign] >> default >> >> Switching the mbm_assign_mode will reset all the MBM counters of all >> resctrl groups. >> >> Signed-off-by: Babu Moger <babu.moger@xxxxxxx> >> --- >> v7: Updated the descriptions/commit log in resctrl.rst to generic text. >> Thanks to James and Reinette. >> Rename mbm_mode to mbm_assign_mode. >> Introduced mutex lock in rdtgroup_mbm_mode_show(). >> >> v6: Added documentation for mbm_cntr_assign and legacy mode. >> Moved mbm_mode fflags initialization to static initialization. >> >> v5: Changed interface name to mbm_mode. >> It will be always available even if ABMC feature is not supported. >> Added description in resctrl.rst about ABMC mode. >> Fixed display abmc and legacy consistantly. >> >> v4: Fixed the checks for legacy and abmc mode. Default it ABMC. >> >> v3: New patch to display ABMC capability. >> --- >> Documentation/arch/x86/resctrl.rst | 33 ++++++++++++++++++++++++++ >> arch/x86/kernel/cpu/resctrl/rdtgroup.c | 31 ++++++++++++++++++++++++ >> 2 files changed, 64 insertions(+) >> >> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst >> index 30586728a4cd..a7b17ad8acb9 100644 >> --- a/Documentation/arch/x86/resctrl.rst >> +++ b/Documentation/arch/x86/resctrl.rst >> @@ -257,6 +257,39 @@ with the following files: >> # cat /sys/fs/resctrl/info/L3_MON/mbm_local_bytes_config >> 0=0x30;1=0x30;3=0x15;4=0x15 >> >> +"mbm_assign_mode": >> + Reports the list of monitoring modes supported. The enclosed brackets >> + indicate which feature is enabled. > > "which feature is enabled" -> "which mode is enabled"? Sure. > >> + :: >> + >> + cat /sys/fs/resctrl/info/L3_MON/mbm_assign_mode >> + [mbm_cntr_assign] >> + default >> + >> + "mbm_cntr_assign": >> + >> + In mbm_cntr_assign mode user-space is able to specify which control >> + or monitor groups in resctrl should have a hardware counter assigned > > This documentation should ideally also be appropriate for when the "soft-ABMC" > support lands. Considering that, should all the "hardware counter" instances perhaps be > changed to just be "counter"? Sure. > >> + using the 'mbm_control' file. The number of hardware counters available >> + is described in the 'num_mbm_cntrs' file. Changing to this mode will >> + cause all counters on a resource to reset. > > Should resctrl commit to this? Resetting of the counters as implemented here > does seem to be an architecture specific action so this text could be > made more generic by stating "may cause all counters on a resource to reset". Ok. Sure. > >> + >> + The feature is needed on platforms which support more control and monitor > > "The feature" -> "The mode"? Sure. > >> + groups than hardware counters, meaning 'unassigned' control or monitor >> + groups will report 'Unavailable' or not count all the traffic in an >> + unpredictable way. > > "or not count all the traffic in an unpredictable way" is a bit hard to parse ... how > about "or count traffic in an unpredictable way"? ok. Sure. > > >> + >> + AMD Platforms with ABMC (Assignable Bandwidth Monitoring Counters) feature >> + enable this mode by default so that counters remain assigned even when the >> + corresponding RMID is not in use by any processor. >> + >> + "default": >> + >> + By default resctrl assumes each control and monitor group has a hardware counter. >> + Hardware without this property will still allow more control or monitor groups >> + than 'num_mbm_cntrs' to be created. Reading the mbm files may report 'Unavailable' > Please be specific what is meant with "the mbm files" Sure. Will change it to mbm_total_bytes and mbm_local_bytes. > >> + if there is no hardware resource assigned. > > "no hardware resource" -> "no counter"? Sure. > >> + >> "max_threshold_occupancy": >> Read/write file provides the largest value (in >> bytes) at which a previously used LLC_occupancy >> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> index 0178555bf3f6..dbc8c5e63213 100644 >> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c >> @@ -845,6 +845,30 @@ static int rdtgroup_rmid_show(struct kernfs_open_file *of, >> return ret; >> } >> > > Reinette > -- Thanks Babu Moger
