[PATCH v7 08/24] x86/resctrl: Introduce interface to display number of monitoring counters

Babu Moger posted 24 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH v7 08/24] x86/resctrl: Introduce interface to display number of monitoring counters
Posted by Babu Moger 1 year, 3 months ago
The mbm_assign_cntr mode provides an option to the user to assign a
hardware counter to an RMID, event pair and monitor the bandwidth as
long as the counter is assigned. Number of assignments depend on number
of monitoring counters available.

Provide the interface to display the number of monitoring counters
supported.

Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v7: Minor commit log text changes.

v6: No changes.

v5: Changed the display name from num_cntrs to num_mbm_cntrs.
    Updated the commit message.
    Moved the patch after mbm_mode is introduced.

v4: Changed the counter name to num_cntrs. And few text changes.

v3: Changed the field name to mbm_assign_cntrs.

v2: Changed the field name to mbm_assignable_counters from abmc_counte
---
 Documentation/arch/x86/resctrl.rst     |  3 +++
 arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
 3 files changed, 20 insertions(+)

diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
index a7b17ad8acb9..3e9302971faf 100644
--- a/Documentation/arch/x86/resctrl.rst
+++ b/Documentation/arch/x86/resctrl.rst
@@ -290,6 +290,9 @@ with the following files:
 	than 'num_mbm_cntrs' to be created. Reading the mbm files may report 'Unavailable'
 	if there is no hardware resource assigned.
 
+"num_mbm_cntrs":
+	The number of monitoring counters available for assignment.
+
 "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/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
index 71fab31e20da..e3e71843401a 100644
--- a/arch/x86/kernel/cpu/resctrl/monitor.c
+++ b/arch/x86/kernel/cpu/resctrl/monitor.c
@@ -1236,6 +1236,7 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
 			r->mon.mbm_cntr_assignable = true;
 			cpuid_count(0x80000020, 5, &eax, &ebx, &ecx, &edx);
 			r->mon.num_mbm_cntrs = (ebx & 0xFFFF) + 1;
+			resctrl_file_fflags_init("num_mbm_cntrs", RFTYPE_MON_INFO);
 		}
 	}
 
diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index dbc8c5e63213..ba737890d5c2 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -869,6 +869,16 @@ static int rdtgroup_mbm_assign_mode_show(struct kernfs_open_file *of,
 	return 0;
 }
 
+static int rdtgroup_num_mbm_cntrs_show(struct kernfs_open_file *of,
+				       struct seq_file *s, void *v)
+{
+	struct rdt_resource *r = of->kn->parent->priv;
+
+	seq_printf(s, "%d\n", r->mon.num_mbm_cntrs);
+
+	return 0;
+}
+
 #ifdef CONFIG_PROC_CPU_RESCTRL
 
 /*
@@ -1940,6 +1950,12 @@ static struct rftype res_common_files[] = {
 		.seq_show	= rdtgroup_cpus_show,
 		.fflags		= RFTYPE_BASE,
 	},
+	{
+		.name		= "num_mbm_cntrs",
+		.mode		= 0444,
+		.kf_ops		= &rdtgroup_kf_single_ops,
+		.seq_show	= rdtgroup_num_mbm_cntrs_show,
+	},
 	{
 		.name		= "cpus_list",
 		.mode		= 0644,
-- 
2.34.1
Re: [PATCH v7 08/24] x86/resctrl: Introduce interface to display number of monitoring counters
Posted by Reinette Chatre 1 year, 3 months ago
Hi Babu,

On 9/4/24 3:21 PM, Babu Moger wrote:
> The mbm_assign_cntr mode provides an option to the user to assign a
> hardware counter to an RMID, event pair and monitor the bandwidth as

Could you please be consistent in this series in how you refer to
an RMID, event pair ? For example later it becomes RMID-event pair.


> long as the counter is assigned. Number of assignments depend on number
> of monitoring counters available.
> 
> Provide the interface to display the number of monitoring counters
> supported.
> 
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> v7: Minor commit log text changes.
> 
> v6: No changes.
> 
> v5: Changed the display name from num_cntrs to num_mbm_cntrs.
>     Updated the commit message.
>     Moved the patch after mbm_mode is introduced.
> 
> v4: Changed the counter name to num_cntrs. And few text changes.
> 
> v3: Changed the field name to mbm_assign_cntrs.
> 
> v2: Changed the field name to mbm_assignable_counters from abmc_counte
> ---
>  Documentation/arch/x86/resctrl.rst     |  3 +++
>  arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
> index a7b17ad8acb9..3e9302971faf 100644
> --- a/Documentation/arch/x86/resctrl.rst
> +++ b/Documentation/arch/x86/resctrl.rst
> @@ -290,6 +290,9 @@ with the following files:
>  	than 'num_mbm_cntrs' to be created. Reading the mbm files may report 'Unavailable'
>  	if there is no hardware resource assigned.
>  
> +"num_mbm_cntrs":
> +	The number of monitoring counters available for assignment.
> +

I think it will be helpful if the changelog and the above doc notes when this file can
be expected to be visible since its visibility is not connected to visibility of
"mbm_assign_mode" that refers to it. There also seems to be a conflict here where
"mbm_assign_mode" documentation contains section about "default" that refers to
"num_mbm_cntrs", but "num_mbm_cntrs" may not be visible if "default" is the only mode.

Reinette
Re: [PATCH v7 08/24] x86/resctrl: Introduce interface to display number of monitoring counters
Posted by Moger, Babu 1 year, 2 months ago
Hi Reinette,

On 9/19/24 11:32, Reinette Chatre wrote:
> Hi Babu,
> 
> On 9/4/24 3:21 PM, Babu Moger wrote:
>> The mbm_assign_cntr mode provides an option to the user to assign a
>> hardware counter to an RMID, event pair and monitor the bandwidth as
> 
> Could you please be consistent in this series in how you refer to
> an RMID, event pair ? For example later it becomes RMID-event pair.

Will keep it as "an RMID, event pair" in all the references.

> 
> 
>> long as the counter is assigned. Number of assignments depend on number
>> of monitoring counters available.
>>
>> Provide the interface to display the number of monitoring counters
>> supported.
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
>> v7: Minor commit log text changes.
>>
>> v6: No changes.
>>
>> v5: Changed the display name from num_cntrs to num_mbm_cntrs.
>>     Updated the commit message.
>>     Moved the patch after mbm_mode is introduced.
>>
>> v4: Changed the counter name to num_cntrs. And few text changes.
>>
>> v3: Changed the field name to mbm_assign_cntrs.
>>
>> v2: Changed the field name to mbm_assignable_counters from abmc_counte
>> ---
>>  Documentation/arch/x86/resctrl.rst     |  3 +++
>>  arch/x86/kernel/cpu/resctrl/monitor.c  |  1 +
>>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 16 ++++++++++++++++
>>  3 files changed, 20 insertions(+)
>>
>> diff --git a/Documentation/arch/x86/resctrl.rst b/Documentation/arch/x86/resctrl.rst
>> index a7b17ad8acb9..3e9302971faf 100644
>> --- a/Documentation/arch/x86/resctrl.rst
>> +++ b/Documentation/arch/x86/resctrl.rst
>> @@ -290,6 +290,9 @@ with the following files:
>>  	than 'num_mbm_cntrs' to be created. Reading the mbm files may report 'Unavailable'
>>  	if there is no hardware resource assigned.
>>  
>> +"num_mbm_cntrs":
>> +	The number of monitoring counters available for assignment.
>> +
> 
> I think it will be helpful if the changelog and the above doc notes when this file can
> be expected to be visible since its visibility is not connected to visibility of

Sure.

> "mbm_assign_mode" that refers to it. There also seems to be a conflict here where
> "mbm_assign_mode" documentation contains section about "default" that refers to
> "num_mbm_cntrs", but "num_mbm_cntrs" may not be visible if "default" is the only mode.
> 

Yes. Need to change the reference to "num_rmids" in default section.

-- 
Thanks
Babu Moger