[PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode

Babu Moger posted 25 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Babu Moger 1 month, 2 weeks ago
Introduce the interface file "mbm_assign_mode" to list monitor modes
supported.

The "mbm_cntr_assign" mode provides the option to assign a counter to
an RMID, event pair 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 and is enabled by default.

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@amd.com>
---
v8: Commit message update.

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     | 34 ++++++++++++++++++++++++++
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 31 +++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index 30586728a4cd..e4a7d6e815f6 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -257,6 +257,40 @@ 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 mode is enabled.
+	::
+
+	  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 counter assigned using the
+	'mbm_assign_control' file. The number of counters available is described
+	in the 'num_mbm_cntrs' file. Changing the mode may cause all counters on
+	a resource to reset.
+
+	The mode is useful on platforms which support more control and monitor
+	groups than hardware counters, meaning 'unassigned' control or monitor
+	groups will report 'Unavailable' or count the traffic in an unpredictable
+	way.
+
+	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 that does not support 'mbm_cntr_assign' mode will still
+	allow more control or monitor groups than 'num_rmids' to be created. In
+	that case reading the mbm_total_bytes and mbm_local_bytes may report
+	'Unavailable' if there is no counter associated with that group.
+
 "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 6bfa8312a4b2..895264c207c7 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;
 }
 
+static int rdtgroup_mbm_assign_mode_show(struct kernfs_open_file *of,
+					 struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+
+	mutex_lock(&rdtgroup_mutex);
+
+	if (r->mon.mbm_cntr_assignable) {
+		if (resctrl_arch_mbm_cntr_assign_enabled(r)) {
+			seq_puts(s, "[mbm_cntr_assign]\n");
+			seq_puts(s, "default\n");
+		} else {
+			seq_puts(s, "mbm_cntr_assign\n");
+			seq_puts(s, "[default]\n");
+		}
+	} else {
+		seq_puts(s, "[default]\n");
+	}
+
+	mutex_unlock(&rdtgroup_mutex);
+
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*
@@ -1901,6 +1925,13 @@ static struct rftype res_common_files[] = {
 		.seq_show	= mbm_local_bytes_config_show,
 		.write		= mbm_local_bytes_config_write,
 	},
+	{
+		.name		= "mbm_assign_mode",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_mbm_assign_mode_show,
+		.fflags		= RFTYPE_MON_INFO,
+	},
 	{
 		.name		= "cpus",
 		.mode		= 0644,
-- 
2.34.1
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Reinette Chatre 1 month, 1 week ago
Hi Babu,

On 10/9/24 10:39 AM, 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 counter to
> an RMID, event pair 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 and is enabled by default.
> 
> 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.

Please note that this now contradicts the documentation. Perhaps this sentence
can just be dropped since there is the documentation within the patch.	


> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

...

> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index 30586728a4cd..e4a7d6e815f6 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -257,6 +257,40 @@ 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 mode is enabled.
> +	::
> +
> +	  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 counter assigned using the

Counters cannot be assigned to control groups. How about replacing all instances
of "control and monitor groups" with "CTRL_MON and MON groups", similarly
"control or monitor groups" with "CTRL_MON or MON groups".

> +	'mbm_assign_control' file. The number of counters available is described

Looking at the rest of the doc it seems that the custom is actually to place
filenames in double quotes, like "mbm_assign_control".

> +	in the 'num_mbm_cntrs' file. Changing the mode may cause all counters on
> +	a resource to reset.
> +
> +	The mode is useful on platforms which support more control and monitor
> +	groups than hardware counters, meaning 'unassigned' control or monitor
> +	groups will report 'Unavailable' or count the traffic in an unpredictable
> +	way.

Note two more instances of "control groups" above.

Please note that the above description implies that counter assignment is per-group. For
example, "specify which control	or monitor groups in resctrl should have a counter
assigned" and "useful on platforms which support more control and monitor groups
than hardware counters". This needs to be reworked to reflect that counters
are assigned to events.

> +
> +	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.

I assume this should remain RMID since this specifically talks about an x86 system?

> +
> +	"default":
> +
> +	By default resctrl assumes each control and monitor group has a hardware
> +	counter. Hardware that does not support 'mbm_cntr_assign' mode will still
> +	allow more control or monitor groups than 'num_rmids' to be created. In
> +	that case reading the mbm_total_bytes and mbm_local_bytes may report
> +	'Unavailable' if there is no counter associated with that group.
> +

I reconsidered my earlier suggestion and I believe it needs a correction since
counter assignment is not per group:

	In default mode resctrl assumes there is a hardware counter for each
	event within every CTRL_MON and MON group. Reading mbm_total_bytes or
	mbm_local_bytes may report 'Unavailable' if there is no counter associated
	with that event.

Please feel free to improve.

>  "max_threshold_occupancy":
>  		Read/write file provides the largest value (in
>  		bytes) at which a previously used LLC_occupancy

The code change looks good to me.

Reinette
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Moger, Babu 1 month, 1 week ago
Hi Reinette

On 10/15/24 22:12, Reinette Chatre wrote:
> Hi Babu,
> 
> On 10/9/24 10:39 AM, 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 counter to
>> an RMID, event pair 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 and is enabled by default.
>>
>> 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.
> 
> Please note that this now contradicts the documentation. Perhaps this sentence
> can just be dropped since there is the documentation within the patch.	

Sure. Will drop it.

> 
> 
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
> 
> ...
> 
>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>> index 30586728a4cd..e4a7d6e815f6 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -257,6 +257,40 @@ 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 mode is enabled.
>> +	::
>> +
>> +	  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 counter assigned using the
> 
> Counters cannot be assigned to control groups. How about replacing all instances
> of "control and monitor groups" with "CTRL_MON and MON groups", similarly
> "control or monitor groups" with "CTRL_MON or MON groups".

Ok.

> 
>> +	'mbm_assign_control' file. The number of counters available is described
> 
> Looking at the rest of the doc it seems that the custom is actually to place
> filenames in double quotes, like "mbm_assign_control".

Sure.

> 
>> +	in the 'num_mbm_cntrs' file. Changing the mode may cause all counters on
>> +	a resource to reset.
>> +
>> +	The mode is useful on platforms which support more control and monitor
>> +	groups than hardware counters, meaning 'unassigned' control or monitor
>> +	groups will report 'Unavailable' or count the traffic in an unpredictable
>> +	way.
> 
> Note two more instances of "control groups" above.
> 
> Please note that the above description implies that counter assignment is per-group. For
> example, "specify which control	or monitor groups in resctrl should have a counter
> assigned" and "useful on platforms which support more control and monitor groups
> than hardware counters". This needs to be reworked to reflect that counters
> are assigned to events.

How about this?

The mode is useful on platforms which support more CTRL_MON and MON groups
than the hardware counters, meaning 'unassigned' events on CTRL_MON or MON
groups will report 'Unavailable' or count the traffic in an unpredictable
way.


> 
>> +
>> +	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.
> 
> I assume this should remain RMID since this specifically talks about an x86 system?

This was a suggestion from James. Let me know if you want me to change.

> 
>> +
>> +	"default":
>> +
>> +	By default resctrl assumes each control and monitor group has a hardware
>> +	counter. Hardware that does not support 'mbm_cntr_assign' mode will still
>> +	allow more control or monitor groups than 'num_rmids' to be created. In
>> +	that case reading the mbm_total_bytes and mbm_local_bytes may report
>> +	'Unavailable' if there is no counter associated with that group.
>> +
> 
> I reconsidered my earlier suggestion and I believe it needs a correction since
> counter assignment is not per group:
> 
> 	In default mode resctrl assumes there is a hardware counter for each
> 	event within every CTRL_MON and MON group. Reading mbm_total_bytes or
> 	mbm_local_bytes may report 'Unavailable' if there is no counter associated
> 	with that event.
> 
> Please feel free to improve.

Looks good.

> 
>>  "max_threshold_occupancy":
>>  		Read/write file provides the largest value (in
>>  		bytes) at which a previously used LLC_occupancy
> 
> The code change looks good to me.

-- 
Thanks
Babu Moger
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Reinette Chatre 1 month, 1 week ago
Hi Babu,

On 10/16/24 8:57 AM, Moger, Babu wrote:
> On 10/15/24 22:12, Reinette Chatre wrote:
>> On 10/9/24 10:39 AM, Babu Moger wrote:

>>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>>> index 30586728a4cd..e4a7d6e815f6 100644
>>> --- a/Documentation/arch/x86/resctrl.rst
>>> +++ b/Documentation/arch/x86/resctrl.rst
>>> @@ -257,6 +257,40 @@ 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 mode is enabled.
>>> +	::
>>> +
>>> +	  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 counter assigned using the
>>
>> Counters cannot be assigned to control groups. How about replacing all instances
>> of "control and monitor groups" with "CTRL_MON and MON groups", similarly
>> "control or monitor groups" with "CTRL_MON or MON groups".
> 
> Ok.
> 
>>
>>> +	'mbm_assign_control' file. The number of counters available is described
>>
>> Looking at the rest of the doc it seems that the custom is actually to place
>> filenames in double quotes, like "mbm_assign_control".
> 
> Sure.
> 
>>
>>> +	in the 'num_mbm_cntrs' file. Changing the mode may cause all counters on
>>> +	a resource to reset.
>>> +
>>> +	The mode is useful on platforms which support more control and monitor
>>> +	groups than hardware counters, meaning 'unassigned' control or monitor
>>> +	groups will report 'Unavailable' or count the traffic in an unpredictable
>>> +	way.
>>
>> Note two more instances of "control groups" above.
>>
>> Please note that the above description implies that counter assignment is per-group. For
>> example, "specify which control	or monitor groups in resctrl should have a counter
>> assigned" and "useful on platforms which support more control and monitor groups
>> than hardware counters". This needs to be reworked to reflect that counters
>> are assigned to events.
> 
> How about this?
> 
> The mode is useful on platforms which support more CTRL_MON and MON groups
> than the hardware counters, meaning 'unassigned' events on CTRL_MON or MON
> groups will report 'Unavailable' or count the traffic in an unpredictable
> way.

This rewrites the second paragraph of the section about "mbm_cntr_assign". It is
not clear to me how this section will end up looking since the first paragraph still
seems to refer to counters being assigned to groups ("specify which control or monitor
groups in resctrl should have a counter assigned") while the later addition
to this section by "x86/resctrl: Introduce the interface to switch between monitor
modes" starts by specifying how counters are assigned to the MBM events ("The MBM
events (mbm_total_bytes and/or mbm_local_bytes) associated counters").

>>> +
>>> +	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.
>>
>> I assume this should remain RMID since this specifically talks about an x86 system?
> 
> This was a suggestion from James. Let me know if you want me to change.

I can proceed to assume this is a paragraph intended to be x86 specific. No need to change.

Reinette
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Tony Luck 1 month, 2 weeks ago
On Wed, Oct 09, 2024 at 12:39:32PM -0500, Babu Moger wrote:
> +"mbm_assign_mode":
> +	Reports the list of monitoring modes supported. The enclosed brackets
> +	indicate which mode is enabled.
> +	::
> +
> +	  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 counter assigned using the
> +	'mbm_assign_control' file. The number of counters available is described
> +	in the 'num_mbm_cntrs' file. Changing the mode may cause all counters on
> +	a resource to reset.
> +
> +	The mode is useful on platforms which support more control and monitor
> +	groups than hardware counters, meaning 'unassigned' control or monitor
> +	groups will report 'Unavailable' or count the traffic in an unpredictable
> +	way.
> +
> +	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 that does not support 'mbm_cntr_assign' mode will still
> +	allow more control or monitor groups than 'num_rmids' to be created. In

Should that be s/num_rmids/num_mbm_cntrs/ ?

-Tony
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Moger, Babu 1 month, 2 weeks ago
Hi Tony,

Thanks for reviewing the patches.

On 10/9/24 17:42, Tony Luck wrote:
> On Wed, Oct 09, 2024 at 12:39:32PM -0500, Babu Moger wrote:
>> +"mbm_assign_mode":
>> +	Reports the list of monitoring modes supported. The enclosed brackets
>> +	indicate which mode is enabled.
>> +	::
>> +
>> +	  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 counter assigned using the
>> +	'mbm_assign_control' file. The number of counters available is described
>> +	in the 'num_mbm_cntrs' file. Changing the mode may cause all counters on
>> +	a resource to reset.
>> +
>> +	The mode is useful on platforms which support more control and monitor
>> +	groups than hardware counters, meaning 'unassigned' control or monitor
>> +	groups will report 'Unavailable' or count the traffic in an unpredictable
>> +	way.
>> +
>> +	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 that does not support 'mbm_cntr_assign' mode will still
>> +	allow more control or monitor groups than 'num_rmids' to be created. In
> 
> Should that be s/num_rmids/num_mbm_cntrs/ ?

It is actually num_rmids here as in default mode, num_rmid_cntrs are not
available.
-- 
Thanks
Babu Moger
RE: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Luck, Tony 1 month, 2 weeks ago
> >> +  By default resctrl assumes each control and monitor group has a hardware
> >> +  counter. Hardware that does not support 'mbm_cntr_assign' mode will still
> >> +  allow more control or monitor groups than 'num_rmids' to be created. In
> >
> > Should that be s/num_rmids/num_mbm_cntrs/ ?
>
> It is actually num_rmids here as in default mode, num_rmid_cntrs are not
> available.

Babu,

The code isn't working that way for me. I built & booted. Since I'm on
an Intel machine without ABMC I'm in "default" mode. But I can't make
more monitor groups that num_rmids.

-Tony
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Moger, Babu 1 month, 2 weeks ago
Hi Tony,

On 10/10/24 10:07, Luck, Tony wrote:
>>>> +  By default resctrl assumes each control and monitor group has a hardware
>>>> +  counter. Hardware that does not support 'mbm_cntr_assign' mode will still
>>>> +  allow more control or monitor groups than 'num_rmids' to be created. In
>>>
>>> Should that be s/num_rmids/num_mbm_cntrs/ ?
>>
>> It is actually num_rmids here as in default mode, num_rmid_cntrs are not
>> available.
> 
> Babu,
> 
> The code isn't working that way for me. I built & booted. Since I'm on
> an Intel machine without ABMC I'm in "default" mode. But I can't make
> more monitor groups that num_rmids.
> 

That is correct. We will have to change the text. How about?

"default":
By default resctrl assumes each control and monitor group has a hardware
counter. Hardware that does not support 'mbm_cntr_assign' mode will still
allow to create control or monitor groups up to num_rmids supported. In
that case reading the mbm_total_bytes and mbm_local_bytes may report
'Unavailable' if there is no counter associated with that group.

-- 
Thanks
Babu Moger
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Reinette Chatre 1 month, 2 weeks ago

On 10/10/24 8:30 AM, Moger, Babu wrote:
> On 10/10/24 10:07, Luck, Tony wrote:
>>>>> +  By default resctrl assumes each control and monitor group has a hardware
>>>>> +  counter. Hardware that does not support 'mbm_cntr_assign' mode will still
>>>>> +  allow more control or monitor groups than 'num_rmids' to be created. In
>>>>
>>>> Should that be s/num_rmids/num_mbm_cntrs/ ?
>>>
>>> It is actually num_rmids here as in default mode, num_rmid_cntrs are not
>>> available.
>>
>> Babu,
>>
>> The code isn't working that way for me. I built & booted. Since I'm on
>> an Intel machine without ABMC I'm in "default" mode. But I can't make
>> more monitor groups that num_rmids.
>>
> 
> That is correct. We will have to change the text. How about?
> 
> "default":
> By default resctrl assumes each control and monitor group has a hardware
> counter. Hardware that does not support 'mbm_cntr_assign' mode will still

I think this is independent from whether hardware supports 'mbm_cntr_assign'
mode since a user could enable 'default' mode on hardware that supports 
'mbm_cntr_assign'. This snippet is thus more about what is meant by 'default'
mode than what is supported by hardware.

The docs already contain:
	"num_rmids":
		...
		This is the upper bound for how many "CTRL_MON" + "MON"
		groups can be created.


Neither of the 'mbm_assign_mode' options change this meaning of 'num_rmids' (i.e.
no change in how many monitor groups can be created) so mentioning it in the
'default' portion but not in the 'mbm_cntr_assign' portion may create confusion.


Perhaps it can be simplified to:
	In default mode resctrl assumes each CTRL_MON and MON group has a
	hardware counter. Reading mbm_total_bytes or mbm_local_bytes may
	report 'Unavailable' if there is no counter associated with that
	group.


> allow to create control or monitor groups up to num_rmids supported. In
> that case reading the mbm_total_bytes and mbm_local_bytes may report
> 'Unavailable' if there is no counter associated with that group.
> 

Reinette
Re: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Moger, Babu 1 month, 1 week ago
Hi Reinette,

On 10/11/24 17:24,  wrote:
> 
> 
> On 10/10/24 8:30 AM, Moger, Babu wrote:
>> On 10/10/24 10:07, Luck, Tony wrote:
>>>>>> +  By default resctrl assumes each control and monitor group has a hardware
>>>>>> +  counter. Hardware that does not support 'mbm_cntr_assign' mode will still
>>>>>> +  allow more control or monitor groups than 'num_rmids' to be created. In
>>>>>
>>>>> Should that be s/num_rmids/num_mbm_cntrs/ ?
>>>>
>>>> It is actually num_rmids here as in default mode, num_rmid_cntrs are not
>>>> available.
>>>
>>> Babu,
>>>
>>> The code isn't working that way for me. I built & booted. Since I'm on
>>> an Intel machine without ABMC I'm in "default" mode. But I can't make
>>> more monitor groups that num_rmids.
>>>
>>
>> That is correct. We will have to change the text. How about?
>>
>> "default":
>> By default resctrl assumes each control and monitor group has a hardware
>> counter. Hardware that does not support 'mbm_cntr_assign' mode will still
> 
> I think this is independent from whether hardware supports 'mbm_cntr_assign'
> mode since a user could enable 'default' mode on hardware that supports 
> 'mbm_cntr_assign'. This snippet is thus more about what is meant by 'default'
> mode than what is supported by hardware.
> 
> The docs already contain:
> 	"num_rmids":
> 		...
> 		This is the upper bound for how many "CTRL_MON" + "MON"
> 		groups can be created.
> 
> 
> Neither of the 'mbm_assign_mode' options change this meaning of 'num_rmids' (i.e.
> no change in how many monitor groups can be created) so mentioning it in the
> 'default' portion but not in the 'mbm_cntr_assign' portion may create confusion.
> 
> 
> Perhaps it can be simplified to:
> 	In default mode resctrl assumes each CTRL_MON and MON group has a
> 	hardware counter. Reading mbm_total_bytes or mbm_local_bytes may
> 	report 'Unavailable' if there is no counter associated with that
> 	group.

Sure.

-- 
Thanks
Babu Moger
RE: [PATCH v8 07/25] x86/resctrl: Introduce the interface to display monitor mode
Posted by Luck, Tony 1 month, 2 weeks ago
> That is correct. We will have to change the text. How about?
>
> "default":
> By default resctrl assumes each control and monitor group has a hardware
> counter. Hardware that does not support 'mbm_cntr_assign' mode will still
> allow to create control or monitor groups up to num_rmids supported. In
> that case reading the mbm_total_bytes and mbm_local_bytes may report
> 'Unavailable' if there is no counter associated with that group.

Babu,

Looks good to me.

Thanks

-Tony