[PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain

Babu Moger posted 23 patches 1 year ago
There is a newer version of this series
[PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Babu Moger 1 year ago
In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
MBM event of a monitor group. Hardware counters are assigned/unassigned
at monitoring domain level.

Manage a monitoring domain's hardware counters using a per monitoring
domain array of struct mbm_cntr_cfg that is indexed by the hardware
counter	ID. A hardware counter's configuration contains the MBM event
ID and points to the monitoring group that it is assigned to, with a
NULL pointer meaning that the hardware counter is available for assignment.

There is no direct way to determine which hardware counters are	assigned
to a particular monitoring group. Check every entry of every hardware
counter	configuration array in every monitoring domain to query which
MBM events of a monitoring group is tracked by hardware. Such queries
are acceptable because of a very small number of assignable counters.

Suggested-by: Peter Newman <peternewman@google.com>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
v11: Refined the change log based on Reinette's feedback.
     Fixed few style issues.

v10: Patch changed completely to handle the counters at domain level.
     https://lore.kernel.org/lkml/CALPaoCj+zWq1vkHVbXYP0znJbe6Ke3PXPWjtri5AFgD9cQDCUg@mail.gmail.com/
     Removed Reviewed-by tag.
     Did not see the need to add cntr_id in mbm_state structure. Not used in the code.

v9: Added Reviewed-by tag. No other changes.

v8: Minor commit message changes.

v7: Added check mbm_cntr_assignable for allocating bitmap mbm_cntr_map

v6: New patch to add domain level assignment.
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++++
 include/linux/resctrl.h                | 14 ++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index 18110a1afb6d..75a3b56996ca 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -4009,6 +4009,7 @@ static void __init rdtgroup_setup_default(void)
 
 static void domain_destroy_mon_state(struct rdt_mon_domain *d)
 {
+	kfree(d->cntr_cfg);
 	bitmap_free(d->rmid_busy_llc);
 	kfree(d->mbm_total);
 	kfree(d->mbm_local);
@@ -4082,6 +4083,16 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_mon_domain
 			return -ENOMEM;
 		}
 	}
+	if (is_mbm_enabled() && r->mon.mbm_cntr_assignable) {
+		tsize = sizeof(*d->cntr_cfg);
+		d->cntr_cfg = kcalloc(r->mon.num_mbm_cntrs, tsize, GFP_KERNEL);
+		if (!d->cntr_cfg) {
+			bitmap_free(d->rmid_busy_llc);
+			kfree(d->mbm_total);
+			kfree(d->mbm_local);
+			return -ENOMEM;
+		}
+	}
 
 	return 0;
 }
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 511cfce8fc21..9a54e307d340 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
 	u32				*mbps_val;
 };
 
+/**
+ * struct mbm_cntr_cfg - assignable counter configuration
+ * @evtid:		 MBM event to which the counter is assigned. Only valid
+ *			 if @rdtgroup is not NULL.
+ * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
+ *			 counter is free.
+ */
+struct mbm_cntr_cfg {
+	enum resctrl_event_id	evtid;
+	struct rdtgroup		*rdtgrp;
+};
+
 /**
  * struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
  * @hdr:		common header for different domain types
@@ -105,6 +117,7 @@ struct rdt_ctrl_domain {
  * @cqm_limbo:		worker to periodically read CQM h/w counters
  * @mbm_work_cpu:	worker CPU for MBM h/w counters
  * @cqm_work_cpu:	worker CPU for CQM h/w counters
+ * @cntr_cfg:		assignable counters configuration
  */
 struct rdt_mon_domain {
 	struct rdt_domain_hdr		hdr;
@@ -116,6 +129,7 @@ struct rdt_mon_domain {
 	struct delayed_work		cqm_limbo;
 	int				mbm_work_cpu;
 	int				cqm_work_cpu;
+	struct mbm_cntr_cfg		*cntr_cfg;
 };
 
 /**
-- 
2.34.1
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by James Morse 11 months, 3 weeks ago
Hi Babu,

On 22/01/2025 20:20, Babu Moger wrote:
> In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
> MBM event of a monitor group. Hardware counters are assigned/unassigned
> at monitoring domain level.
> 
> Manage a monitoring domain's hardware counters using a per monitoring
> domain array of struct mbm_cntr_cfg that is indexed by the hardware
> counter	ID. A hardware counter's configuration contains the MBM event
> ID and points to the monitoring group that it is assigned to, with a
> NULL pointer meaning that the hardware counter is available for assignment.
> 
> There is no direct way to determine which hardware counters are	assigned
> to a particular monitoring group. Check every entry of every hardware
> counter	configuration array in every monitoring domain to query which
> MBM events of a monitoring group is tracked by hardware. Such queries
> are acceptable because of a very small number of assignable counters.

> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 511cfce8fc21..9a54e307d340 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
>  	u32				*mbps_val;
>  };
>  
> +/**
> + * struct mbm_cntr_cfg - assignable counter configuration
> + * @evtid:		 MBM event to which the counter is assigned. Only valid
> + *			 if @rdtgroup is not NULL.
> + * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
> + *			 counter is free.
> + */
> +struct mbm_cntr_cfg {
> +	enum resctrl_event_id	evtid;
> +	struct rdtgroup		*rdtgrp;
> +};

struct rdtgroup here suggests this shouldn't be something the arch code is touching.

If its not needed by any arch specific code, (I couldn't find a resctrl_arch helper that
takes this) - could it be moved to resctrl's internal.h.

(If this does need to be visible to the arch code, one option would be to replace rdtgroup
with the closid/rmid, and a valid flag so that memset() continues to reset these entries)


Thanks,

James
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Reinette Chatre 11 months, 3 weeks ago
Hi James,

On 2/21/25 10:07 AM, James Morse wrote:
> Hi Babu,
> 
> On 22/01/2025 20:20, Babu Moger wrote:
>> In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
>> MBM event of a monitor group. Hardware counters are assigned/unassigned
>> at monitoring domain level.
>>
>> Manage a monitoring domain's hardware counters using a per monitoring
>> domain array of struct mbm_cntr_cfg that is indexed by the hardware
>> counter	ID. A hardware counter's configuration contains the MBM event
>> ID and points to the monitoring group that it is assigned to, with a
>> NULL pointer meaning that the hardware counter is available for assignment.
>>
>> There is no direct way to determine which hardware counters are	assigned
>> to a particular monitoring group. Check every entry of every hardware
>> counter	configuration array in every monitoring domain to query which
>> MBM events of a monitoring group is tracked by hardware. Such queries
>> are acceptable because of a very small number of assignable counters.
> 
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 511cfce8fc21..9a54e307d340 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
>>  	u32				*mbps_val;
>>  };
>>  
>> +/**
>> + * struct mbm_cntr_cfg - assignable counter configuration
>> + * @evtid:		 MBM event to which the counter is assigned. Only valid
>> + *			 if @rdtgroup is not NULL.
>> + * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
>> + *			 counter is free.
>> + */
>> +struct mbm_cntr_cfg {
>> +	enum resctrl_event_id	evtid;
>> +	struct rdtgroup		*rdtgrp;
>> +};
> 
> struct rdtgroup here suggests this shouldn't be something the arch code is touching.
> 
> If its not needed by any arch specific code, (I couldn't find a resctrl_arch helper that
> takes this) - could it be moved to resctrl's internal.h.
> 
> (If this does need to be visible to the arch code, one option would be to replace rdtgroup
> with the closid/rmid, and a valid flag so that memset() continues to reset these entries)
> 

Thank you for catching this!

Reinette
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Moger, Babu 11 months, 3 weeks ago

On 2/21/2025 12:35 PM, Reinette Chatre wrote:
> Hi James,
> 
> On 2/21/25 10:07 AM, James Morse wrote:
>> Hi Babu,
>>
>> On 22/01/2025 20:20, Babu Moger wrote:
>>> In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
>>> MBM event of a monitor group. Hardware counters are assigned/unassigned
>>> at monitoring domain level.
>>>
>>> Manage a monitoring domain's hardware counters using a per monitoring
>>> domain array of struct mbm_cntr_cfg that is indexed by the hardware
>>> counter	ID. A hardware counter's configuration contains the MBM event
>>> ID and points to the monitoring group that it is assigned to, with a
>>> NULL pointer meaning that the hardware counter is available for assignment.
>>>
>>> There is no direct way to determine which hardware counters are	assigned
>>> to a particular monitoring group. Check every entry of every hardware
>>> counter	configuration array in every monitoring domain to query which
>>> MBM events of a monitoring group is tracked by hardware. Such queries
>>> are acceptable because of a very small number of assignable counters.
>>
>>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>>> index 511cfce8fc21..9a54e307d340 100644
>>> --- a/include/linux/resctrl.h
>>> +++ b/include/linux/resctrl.h
>>> @@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
>>>   	u32				*mbps_val;
>>>   };
>>>   
>>> +/**
>>> + * struct mbm_cntr_cfg - assignable counter configuration
>>> + * @evtid:		 MBM event to which the counter is assigned. Only valid
>>> + *			 if @rdtgroup is not NULL.
>>> + * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
>>> + *			 counter is free.
>>> + */
>>> +struct mbm_cntr_cfg {
>>> +	enum resctrl_event_id	evtid;
>>> +	struct rdtgroup		*rdtgrp;
>>> +};
>>
>> struct rdtgroup here suggests this shouldn't be something the arch code is touching.
>>
>> If its not needed by any arch specific code, (I couldn't find a resctrl_arch helper that
>> takes this) - could it be moved to resctrl's internal.h.
>>
>> (If this does need to be visible to the arch code, one option would be to replace rdtgroup
>> with the closid/rmid, and a valid flag so that memset() continues to reset these entries)
>>
> 
> Thank you for catching this!
> 
> Reinette
> 
> 

Sure. Will move it to arch/x86/kernel/cpu/resctrl/internal.h.

thanks
Babu
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Reinette Chatre 1 year ago
Hi Babu,

On 1/22/25 12:20 PM, Babu Moger wrote:
> In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
> MBM event of a monitor group. Hardware counters are assigned/unassigned
> at monitoring domain level.
> 
> Manage a monitoring domain's hardware counters using a per monitoring
> domain array of struct mbm_cntr_cfg that is indexed by the hardware
> counter	ID. A hardware counter's configuration contains the MBM event

Something strange in this changelog with a few random \t in the text.

> ID and points to the monitoring group that it is assigned to, with a
> NULL pointer meaning that the hardware counter is available for assignment.
> 
> There is no direct way to determine which hardware counters are	assigned

... another \t above 

> to a particular monitoring group. Check every entry of every hardware
> counter	configuration array in every monitoring domain to query which

... one more \t above

> MBM events of a monitoring group is tracked by hardware. Such queries
> are acceptable because of a very small number of assignable counters.

It is not obvious what "very small number" means. Is it possible to give
a range to help reader understand the motivation?

> 
> Suggested-by: Peter Newman <peternewman@google.com>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---

> ---
>  arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++++
>  include/linux/resctrl.h                | 14 ++++++++++++++
>  2 files changed, 25 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> index 18110a1afb6d..75a3b56996ca 100644
> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
> @@ -4009,6 +4009,7 @@ static void __init rdtgroup_setup_default(void)
>  
>  static void domain_destroy_mon_state(struct rdt_mon_domain *d)
>  {
> +	kfree(d->cntr_cfg);
>  	bitmap_free(d->rmid_busy_llc);
>  	kfree(d->mbm_total);
>  	kfree(d->mbm_local);
> @@ -4082,6 +4083,16 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_mon_domain
>  			return -ENOMEM;
>  		}
>  	}
> +	if (is_mbm_enabled() && r->mon.mbm_cntr_assignable) {
> +		tsize = sizeof(*d->cntr_cfg);
> +		d->cntr_cfg = kcalloc(r->mon.num_mbm_cntrs, tsize, GFP_KERNEL);
> +		if (!d->cntr_cfg) {
> +			bitmap_free(d->rmid_busy_llc);
> +			kfree(d->mbm_total);
> +			kfree(d->mbm_local);
> +			return -ENOMEM;
> +		}
> +	}
>  
>  	return 0;
>  }
> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
> index 511cfce8fc21..9a54e307d340 100644
> --- a/include/linux/resctrl.h
> +++ b/include/linux/resctrl.h
> @@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
>  	u32				*mbps_val;
>  };
>  
> +/**
> + * struct mbm_cntr_cfg - assignable counter configuration
> + * @evtid:		 MBM event to which the counter is assigned. Only valid
> + *			 if @rdtgroup is not NULL.
> + * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
> + *			 counter is free.
> + */
> +struct mbm_cntr_cfg {
> +	enum resctrl_event_id	evtid;
> +	struct rdtgroup		*rdtgrp;
> +};
> +

$ scripts/kernel-doc -v -none include/linux/resctrl.h                           
...                                                                             
include/linux/resctrl.h:107: warning: Function parameter or struct member 'rdtgrp' not described in 'mbm_cntr_cfg'
include/linux/resctrl.h:107: warning: Excess struct member 'rdtgroup' description in 'mbm_cntr_cfg'
...                                            

>  /**
>   * struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
>   * @hdr:		common header for different domain types
> @@ -105,6 +117,7 @@ struct rdt_ctrl_domain {
>   * @cqm_limbo:		worker to periodically read CQM h/w counters
>   * @mbm_work_cpu:	worker CPU for MBM h/w counters
>   * @cqm_work_cpu:	worker CPU for CQM h/w counters
> + * @cntr_cfg:		assignable counters configuration
>   */
>  struct rdt_mon_domain {
>  	struct rdt_domain_hdr		hdr;
> @@ -116,6 +129,7 @@ struct rdt_mon_domain {
>  	struct delayed_work		cqm_limbo;
>  	int				mbm_work_cpu;
>  	int				cqm_work_cpu;
> +	struct mbm_cntr_cfg		*cntr_cfg;
>  };
>  
>  /**

Reinette
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Moger, Babu 1 year ago
Hi Reinette,

On 2/5/2025 5:57 PM, Reinette Chatre wrote:
> Hi Babu,
> 
> On 1/22/25 12:20 PM, Babu Moger wrote:
>> In mbm_cntr_assign mode hardware counters are assigned/unassigned to an
>> MBM event of a monitor group. Hardware counters are assigned/unassigned
>> at monitoring domain level.
>>
>> Manage a monitoring domain's hardware counters using a per monitoring
>> domain array of struct mbm_cntr_cfg that is indexed by the hardware
>> counter	ID. A hardware counter's configuration contains the MBM event
> 
> Something strange in this changelog with a few random \t in the text.

Not sure how it got in there. I can only see with "set list" option.

I Will remove it.

> 
>> ID and points to the monitoring group that it is assigned to, with a
>> NULL pointer meaning that the hardware counter is available for assignment.
>>
>> There is no direct way to determine which hardware counters are	assigned
> 
> ... another \t above

Sure.
> 
>> to a particular monitoring group. Check every entry of every hardware
>> counter	configuration array in every monitoring domain to query which
> 
> ... one more \t above

Sure

> 
>> MBM events of a monitoring group is tracked by hardware. Such queries
>> are acceptable because of a very small number of assignable counters.
> 
> It is not obvious what "very small number" means. Is it possible to give
> a range to help reader understand the motivation?

How about?

MBM events of a monitoring group is tracked by hardware. Such queries
are acceptable because of a very small number of assignable counters(32 
to 64).

> 
>>
>> Suggested-by: Peter Newman <peternewman@google.com>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
> 
>> ---
>>   arch/x86/kernel/cpu/resctrl/rdtgroup.c | 11 +++++++++++
>>   include/linux/resctrl.h                | 14 ++++++++++++++
>>   2 files changed, 25 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> index 18110a1afb6d..75a3b56996ca 100644
>> --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
>> @@ -4009,6 +4009,7 @@ static void __init rdtgroup_setup_default(void)
>>   
>>   static void domain_destroy_mon_state(struct rdt_mon_domain *d)
>>   {
>> +	kfree(d->cntr_cfg);
>>   	bitmap_free(d->rmid_busy_llc);
>>   	kfree(d->mbm_total);
>>   	kfree(d->mbm_local);
>> @@ -4082,6 +4083,16 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_mon_domain
>>   			return -ENOMEM;
>>   		}
>>   	}
>> +	if (is_mbm_enabled() && r->mon.mbm_cntr_assignable) {
>> +		tsize = sizeof(*d->cntr_cfg);
>> +		d->cntr_cfg = kcalloc(r->mon.num_mbm_cntrs, tsize, GFP_KERNEL);
>> +		if (!d->cntr_cfg) {
>> +			bitmap_free(d->rmid_busy_llc);
>> +			kfree(d->mbm_total);
>> +			kfree(d->mbm_local);
>> +			return -ENOMEM;
>> +		}
>> +	}
>>   
>>   	return 0;
>>   }
>> diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
>> index 511cfce8fc21..9a54e307d340 100644
>> --- a/include/linux/resctrl.h
>> +++ b/include/linux/resctrl.h
>> @@ -94,6 +94,18 @@ struct rdt_ctrl_domain {
>>   	u32				*mbps_val;
>>   };
>>   
>> +/**
>> + * struct mbm_cntr_cfg - assignable counter configuration
>> + * @evtid:		 MBM event to which the counter is assigned. Only valid
>> + *			 if @rdtgroup is not NULL.
>> + * @rdtgroup:		 resctrl group assigned to the counter. NULL if the
>> + *			 counter is free.
>> + */
>> +struct mbm_cntr_cfg {
>> +	enum resctrl_event_id	evtid;
>> +	struct rdtgroup		*rdtgrp;
>> +};
>> +
> 
> $ scripts/kernel-doc -v -none include/linux/resctrl.h
> ...
> include/linux/resctrl.h:107: warning: Function parameter or struct member 'rdtgrp' not described in 'mbm_cntr_cfg'
> include/linux/resctrl.h:107: warning: Excess struct member 'rdtgroup' description in 'mbm_cntr_cfg'


Yes. Will fix this.

> ...
> 
>>   /**
>>    * struct rdt_mon_domain - group of CPUs sharing a resctrl monitor resource
>>    * @hdr:		common header for different domain types
>> @@ -105,6 +117,7 @@ struct rdt_ctrl_domain {
>>    * @cqm_limbo:		worker to periodically read CQM h/w counters
>>    * @mbm_work_cpu:	worker CPU for MBM h/w counters
>>    * @cqm_work_cpu:	worker CPU for CQM h/w counters
>> + * @cntr_cfg:		assignable counters configuration
>>    */
>>   struct rdt_mon_domain {
>>   	struct rdt_domain_hdr		hdr;
>> @@ -116,6 +129,7 @@ struct rdt_mon_domain {
>>   	struct delayed_work		cqm_limbo;
>>   	int				mbm_work_cpu;
>>   	int				cqm_work_cpu;
>> +	struct mbm_cntr_cfg		*cntr_cfg;
>>   };
>>   
>>   /**
> 
> Reinette
> 
> 

Thanks
Babu
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Reinette Chatre 1 year ago
Hi Babu,

On 2/7/25 10:23 AM, Moger, Babu wrote:
> On 2/5/2025 5:57 PM, Reinette Chatre wrote:
>> On 1/22/25 12:20 PM, Babu Moger wrote:
>>
>>> to a particular monitoring group. Check every entry of every hardware
>>> counter    configuration array in every monitoring domain to query which
>>
>> ... one more \t above
> 
> Sure
> 
>>
>>> MBM events of a monitoring group is tracked by hardware. Such queries
>>> are acceptable because of a very small number of assignable counters.
>>
>> It is not obvious what "very small number" means. Is it possible to give
>> a range to help reader understand the motivation?
> 
> How about?
> 
> MBM events of a monitoring group is tracked by hardware. Such queries
> are acceptable because of a very small number of assignable counters(32 to 64).

Yes, thank you. This helps to understand the claim.

Reinette

Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Dave Martin 11 months, 3 weeks ago
Hi,

On Mon, Feb 10, 2025 at 10:10:26AM -0800, Reinette Chatre wrote:
> Hi Babu,
> 
> On 2/7/25 10:23 AM, Moger, Babu wrote:
> > On 2/5/2025 5:57 PM, Reinette Chatre wrote:
> >> On 1/22/25 12:20 PM, Babu Moger wrote:

[...]

> >>> MBM events of a monitoring group is tracked by hardware. Such queries
> >>> are acceptable because of a very small number of assignable counters.
> >>
> >> It is not obvious what "very small number" means. Is it possible to give
> >> a range to help reader understand the motivation?
> > 
> > How about?
> > 
> > MBM events of a monitoring group is tracked by hardware. Such queries
> > are acceptable because of a very small number of assignable counters(32 to 64).
> 
> Yes, thank you. This helps to understand the claim.
> 
> Reinette

Do these queries only happen when userspace reads an mbm_assign_control
file?

It might be worth documenting somewhere that writing and (especially)
reading an mbm_assign_control file is not intended to be super-fast.

It feels like userspace should not generally rely on reading
mbm_assign_control files except for diagnostic purposes, or occasional
read-modify-write transformations.  Or do expect some other usage model
that makes this a hotter path?

Cheers
---Dave
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Moger, Babu 11 months, 3 weeks ago
Hi Dave,

On 2/19/25 07:30, Dave Martin wrote:
> Hi,
> 
> On Mon, Feb 10, 2025 at 10:10:26AM -0800, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 2/7/25 10:23 AM, Moger, Babu wrote:
>>> On 2/5/2025 5:57 PM, Reinette Chatre wrote:
>>>> On 1/22/25 12:20 PM, Babu Moger wrote:
> 
> [...]
> 
>>>>> MBM events of a monitoring group is tracked by hardware. Such queries
>>>>> are acceptable because of a very small number of assignable counters.
>>>>
>>>> It is not obvious what "very small number" means. Is it possible to give
>>>> a range to help reader understand the motivation?
>>>
>>> How about?
>>>
>>> MBM events of a monitoring group is tracked by hardware. Such queries
>>> are acceptable because of a very small number of assignable counters(32 to 64).
>>
>> Yes, thank you. This helps to understand the claim.
>>
>> Reinette
> 
> Do these queries only happen when userspace reads an mbm_assign_control
> file?

Yes. All these queries are initiated by userspace in the form of
individual assignments or creating a group(mkdir).

> 
> It might be worth documenting somewhere that writing and (especially)
> reading an mbm_assign_control file is not intended to be super-fast.


We can drop the last sentence if it is creating confusion.

> 
> It feels like userspace should not generally rely on reading
> mbm_assign_control files except for diagnostic purposes, or occasional
> read-modify-write transformations.  Or do expect some other usage model
> that makes this a hotter path?
> 
> Cheers
> ---Dave

Our earlier interface was intended to query each group separately. After
the input from Peter, we changed it to batched query. One query from
userspace can list all the assignments. I am not aware of any other usage
model.
-- 
Thanks
Babu Moger
Re: [PATCH v11 11/23] x86/resctrl: Introduce mbm_cntr_cfg to track assignable counters at domain
Posted by Dave Martin 11 months, 3 weeks ago
On Wed, Feb 19, 2025 at 12:07:30PM -0600, Moger, Babu wrote:
> Hi Dave,
> 
> On 2/19/25 07:30, Dave Martin wrote:
> > Hi,
> > 
> > On Mon, Feb 10, 2025 at 10:10:26AM -0800, Reinette Chatre wrote:
> >> Hi Babu,
> >>
> >> On 2/7/25 10:23 AM, Moger, Babu wrote:
> >>> On 2/5/2025 5:57 PM, Reinette Chatre wrote:
> >>>> On 1/22/25 12:20 PM, Babu Moger wrote:
> > 
> > [...]
> > 
> >>>>> MBM events of a monitoring group is tracked by hardware. Such queries
> >>>>> are acceptable because of a very small number of assignable counters.
> >>>>
> >>>> It is not obvious what "very small number" means. Is it possible to give
> >>>> a range to help reader understand the motivation?
> >>>
> >>> How about?
> >>>
> >>> MBM events of a monitoring group is tracked by hardware. Such queries
> >>> are acceptable because of a very small number of assignable counters(32 to 64).
> >>
> >> Yes, thank you. This helps to understand the claim.
> >>
> >> Reinette
> > 
> > Do these queries only happen when userspace reads an mbm_assign_control
> > file?
> 
> Yes. All these queries are initiated by userspace in the form of
> individual assignments or creating a group(mkdir).
> 
> > 
> > It might be worth documenting somewhere that writing and (especially)
> > reading an mbm_assign_control file is not intended to be super-fast.
> 
> 
> We can drop the last sentence if it is creating confusion.
> 
> > 
> > It feels like userspace should not generally rely on reading
> > mbm_assign_control files except for diagnostic purposes, or occasional
> > read-modify-write transformations.  Or do expect some other usage model
> > that makes this a hotter path?
> > 
> > Cheers
> > ---Dave
> 
> Our earlier interface was intended to query each group separately. After
> the input from Peter, we changed it to batched query. One query from
> userspace can list all the assignments. I am not aware of any other usage
> model.

Right, that's what I thought.

I'll defer to Reinette on whether it's important to keep the statement
about rationale -- it might indeed be easier to drop it if it just
raises more questions.

Cheers
---Dave