[PATCH v2 29/45] arm_mpam: resctrl: Pre-allocate free running monitors

Ben Horgan posted 45 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v2 29/45] arm_mpam: resctrl: Pre-allocate free running monitors
Posted by Ben Horgan 1 month, 3 weeks ago
From: James Morse <james.morse@arm.com>

When there are enough monitors, the resctrl mbm local and total files can
be exposed. These need all the monitors that resctrl may use to be
allocated up front.

Add helpers to do this.

If a different candidate class is discovered, the old array should be
free'd and the allocated monitors returned to the driver.

Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Ben Horgan <ben.horgan@arm.com>
---
 drivers/resctrl/mpam_internal.h |  8 +++-
 drivers/resctrl/mpam_resctrl.c  | 84 ++++++++++++++++++++++++++++++++-
 2 files changed, 89 insertions(+), 3 deletions(-)

diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 6a2231b28cad..c25524f75c2e 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -356,7 +356,13 @@ struct mpam_resctrl_res {
 struct mpam_resctrl_mon {
 	struct mpam_class	*class;
 
-	/* per-class data that resctrl needs will live here */
+	/*
+	 * Array of allocated MBWU monitors, indexed by (closid, rmid).
+	 * When ABMC is not in use, this array directly maps (closid, rmid)
+	 * to the allocated monitor. Otherwise this array is sparse, and
+	 * un-assigned (closid, rmid) are -1.
+	 */
+	int			*mbwu_idx_to_mon;
 };
 
 static inline int mpam_alloc_csu_mon(struct mpam_class *class)
diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
index 51caf3b82392..bea16bc096f7 100644
--- a/drivers/resctrl/mpam_resctrl.c
+++ b/drivers/resctrl/mpam_resctrl.c
@@ -558,10 +558,58 @@ static void mpam_resctrl_pick_mba(void)
 	}
 }
 
+static void __free_mbwu_mon(struct mpam_class *class, int *array,
+			    u16 num_mbwu_mon)
+{
+	for (int i = 0; i < num_mbwu_mon; i++) {
+		if (array[i] < 0)
+			continue;
+
+		mpam_free_mbwu_mon(class, array[i]);
+		array[i] = ~0;
+	}
+}
+
+static int __alloc_mbwu_mon(struct mpam_class *class, int *array,
+			    u16 num_mbwu_mon)
+{
+	for (int i = 0; i < num_mbwu_mon; i++) {
+		int mbwu_mon = mpam_alloc_mbwu_mon(class);
+
+		if (mbwu_mon < 0) {
+			__free_mbwu_mon(class, array, num_mbwu_mon);
+			return mbwu_mon;
+		}
+		array[i] = mbwu_mon;
+	}
+
+	return 0;
+}
+
+static int *__alloc_mbwu_array(struct mpam_class *class, u16 num_mbwu_mon)
+{
+	int err;
+	size_t array_size = num_mbwu_mon * sizeof(int);
+	int *array __free(kfree) = kmalloc(array_size, GFP_KERNEL);
+
+	if (!array)
+		return ERR_PTR(-ENOMEM);
+
+	memset(array, -1, array_size);
+
+	err = __alloc_mbwu_mon(class, array, num_mbwu_mon);
+	if (err)
+		return ERR_PTR(err);
+	return_ptr(array);
+}
+
 static void counter_update_class(enum resctrl_event_id evt_id,
 				 struct mpam_class *class)
 {
-	struct mpam_class *existing_class = mpam_resctrl_counters[evt_id].class;
+	struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[evt_id];
+	struct mpam_class *existing_class = mon->class;
+	u16 num_mbwu_mon = class->props.num_mbwu_mon;
+	int *existing_array = mon->mbwu_idx_to_mon;
 
 	if (existing_class) {
 		if (class->level == 3) {
@@ -576,8 +624,40 @@ static void counter_update_class(enum resctrl_event_id evt_id,
 		}
 	}
 
-	mpam_resctrl_counters[evt_id].class = class;
+	pr_debug("Updating event %u to use class %u\n", evt_id, class->level);
+	mon->class = class;
 	exposed_mon_capable = true;
+
+	if (evt_id == QOS_L3_OCCUP_EVENT_ID)
+		return;
+
+	/* Might not need all the monitors */
+	num_mbwu_mon = __mpam_monitors_free_running(num_mbwu_mon);
+	if (!num_mbwu_mon) {
+		pr_debug("Not pre-allocating free-running counters\n");
+		return;
+	}
+
+	/*
+	 * This is the pre-allocated free-running monitors path. It always
+	 * allocates one monitor per PARTID * PMG.
+	 */
+	WARN_ON_ONCE(num_mbwu_mon != resctrl_arch_system_num_rmid_idx());
+
+	mon->mbwu_idx_to_mon = __alloc_mbwu_array(class, num_mbwu_mon);
+	if (IS_ERR(mon->mbwu_idx_to_mon)) {
+		pr_debug("Failed to allocate MBWU array\n");
+		mon->class = existing_class;
+		mon->mbwu_idx_to_mon = existing_array;
+		return;
+	}
+
+	if (existing_array) {
+		pr_debug("Releasing previous class %u's monitors\n",
+			 existing_class->level);
+		__free_mbwu_mon(existing_class, existing_array, num_mbwu_mon);
+		kfree(existing_array);
+	}
 }
 
 static void mpam_resctrl_pick_counters(void)
-- 
2.43.0
Re: [PATCH v2 29/45] arm_mpam: resctrl: Pre-allocate free running monitors
Posted by Jonathan Cameron 1 month ago
On Fri, 19 Dec 2025 18:11:31 +0000
Ben Horgan <ben.horgan@arm.com> wrote:

> From: James Morse <james.morse@arm.com>
> 
> When there are enough monitors, the resctrl mbm local and total files can
> be exposed. These need all the monitors that resctrl may use to be
> allocated up front.
> 
> Add helpers to do this.
> 
> If a different candidate class is discovered, the old array should be
> free'd and the allocated monitors returned to the driver.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Signed-off-by: Ben Horgan <ben.horgan@arm.com>

See below.  Maybe there isn't a nicer way to handle the error
cases in counter_update_class() but I think it can be improved at least
a little.

> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
> index 51caf3b82392..bea16bc096f7 100644
> --- a/drivers/resctrl/mpam_resctrl.c
> +++ b/drivers/resctrl/mpam_resctrl.c

> +
>  static void counter_update_class(enum resctrl_event_id evt_id,
>  				 struct mpam_class *class)
The stuff below is very much a case of me trying to figure out a nicer
code flow in here and stumbling a bit over what it can be, however
I do think this function would benefit from another look.

>  {
> -	struct mpam_class *existing_class = mpam_resctrl_counters[evt_id].class;
> +	struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[evt_id];
> +	struct mpam_class *existing_class = mon->class;
> +	u16 num_mbwu_mon = class->props.num_mbwu_mon;
> +	int *existing_array = mon->mbwu_idx_to_mon;
>  
>  	if (existing_class) {
>  		if (class->level == 3) {
> @@ -576,8 +624,40 @@ static void counter_update_class(enum resctrl_event_id evt_id,
>  		}
>  	}
>  
> -	mpam_resctrl_counters[evt_id].class = class;
> +	pr_debug("Updating event %u to use class %u\n", evt_id, class->level);
> +	mon->class = class;
>  	exposed_mon_capable = true;
> +
> +	if (evt_id == QOS_L3_OCCUP_EVENT_ID)
> +		return;
> +
> +	/* Might not need all the monitors */
> +	num_mbwu_mon = __mpam_monitors_free_running(num_mbwu_mon);
> +	if (!num_mbwu_mon) {

For the below suggestion would need duplicate setting 
		mon->class = class;
here (which is the one disadvantage I can see in setting it later than
currently.  One option would be to flip the logic here and
not exit early, allowing sharing of that statement after the
rest of the code.
	if (num_mbw_mon) {
		stuff to allocate array.
	} else {
		pr_debug("Not pre-allocating...);
	}
	mon->class = class;
}



> +		pr_debug("Not pre-allocating free-running counters\n");
> +		return;
> +	}
> +
> +	/*
> +	 * This is the pre-allocated free-running monitors path. It always
> +	 * allocates one monitor per PARTID * PMG.
> +	 */
> +	WARN_ON_ONCE(num_mbwu_mon != resctrl_arch_system_num_rmid_idx());
> +
> +	mon->mbwu_idx_to_mon = __alloc_mbwu_array(class, num_mbwu_mon);

This might be cleaner with a local so as to avoid setting and reverting on error.
Also, does anything stop mon->class being set after this?
Seems like that would be cleaner.

	int *new_array; // up top obviously.

	new_array = __alloc_mbwu_array(class, num_mbwu_mon);
	if (IS_ERR(new_array)) {
		pr_debug("Failed to allocate MBWU array\n");
		return;
	}

//this bit less obviously a good idea.
	if (mon->mbwu_idx_to_mon) {
		pr_debug("Releasing previous class %u's monitors\n",
			 mon->mbwu_idx_to_mon->level);
		__free_mbwu_mon(mon->class, mon->mbwu_idx_to_mon, num_mbwu_mon);
		kfree(mon->mbwu_idx_to_mon);
	}
//so maybe skip that bit.
//Anyhow aim is to set these only after we know we are good carry on.
	mon->mbwu_idx_to_mon = new_array;
	mon->class = class;


Some parts of this might make sense without going the whole way. Maybe
just setting the array late and leaving the class set early then
reverting that on error.


> +	if (IS_ERR(mon->mbwu_idx_to_mon)) {
> +		pr_debug("Failed to allocate MBWU array\n");
> +		mon->class = existing_class;
> +		mon->mbwu_idx_to_mon = existing_array;
> +		return;
> +	}
> +
> +	if (existing_array) {
> +		pr_debug("Releasing previous class %u's monitors\n",
> +			 existing_class->level);
> +		__free_mbwu_mon(existing_class, existing_array, num_mbwu_mon);
> +		kfree(existing_array);
> +	}
>  }
>  
>  static void mpam_resctrl_pick_counters(void)
Re: [PATCH v2 29/45] arm_mpam: resctrl: Pre-allocate free running monitors
Posted by Ben Horgan 1 month ago
Hi Jonathan,

On 1/6/26 14:22, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 18:11:31 +0000
> Ben Horgan <ben.horgan@arm.com> wrote:
> 
>> From: James Morse <james.morse@arm.com>
>>
>> When there are enough monitors, the resctrl mbm local and total files can
>> be exposed. These need all the monitors that resctrl may use to be
>> allocated up front.
>>
>> Add helpers to do this.
>>
>> If a different candidate class is discovered, the old array should be
>> free'd and the allocated monitors returned to the driver.
>>
>> Signed-off-by: James Morse <james.morse@arm.com>
>> Signed-off-by: Ben Horgan <ben.horgan@arm.com>
> 
> See below.  Maybe there isn't a nicer way to handle the error
> cases in counter_update_class() but I think it can be improved at least
> a little.
> 
>> diff --git a/drivers/resctrl/mpam_resctrl.c b/drivers/resctrl/mpam_resctrl.c
>> index 51caf3b82392..bea16bc096f7 100644
>> --- a/drivers/resctrl/mpam_resctrl.c
>> +++ b/drivers/resctrl/mpam_resctrl.c
> 
>> +
>>  static void counter_update_class(enum resctrl_event_id evt_id,
>>  				 struct mpam_class *class)
> The stuff below is very much a case of me trying to figure out a nicer
> code flow in here and stumbling a bit over what it can be, however
> I do think this function would benefit from another look.
> 
>>  {
>> -	struct mpam_class *existing_class = mpam_resctrl_counters[evt_id].class;
>> +	struct mpam_resctrl_mon *mon = &mpam_resctrl_counters[evt_id];
>> +	struct mpam_class *existing_class = mon->class;
>> +	u16 num_mbwu_mon = class->props.num_mbwu_mon;
>> +	int *existing_array = mon->mbwu_idx_to_mon;
>>  
>>  	if (existing_class) {
>>  		if (class->level == 3) {
>> @@ -576,8 +624,40 @@ static void counter_update_class(enum resctrl_event_id evt_id,
>>  		}
>>  	}
>>  
>> -	mpam_resctrl_counters[evt_id].class = class;
>> +	pr_debug("Updating event %u to use class %u\n", evt_id, class->level);
>> +	mon->class = class;
>>  	exposed_mon_capable = true;
>> +
>> +	if (evt_id == QOS_L3_OCCUP_EVENT_ID)
>> +		return;
>> +
>> +	/* Might not need all the monitors */
>> +	num_mbwu_mon = __mpam_monitors_free_running(num_mbwu_mon);
>> +	if (!num_mbwu_mon) {
> 
> For the below suggestion would need duplicate setting 
> 		mon->class = class;
> here (which is the one disadvantage I can see in setting it later than
> currently.  One option would be to flip the logic here and
> not exit early, allowing sharing of that statement after the
> rest of the code.
> 	if (num_mbw_mon) {
> 		stuff to allocate array.
> 	} else {
> 		pr_debug("Not pre-allocating...);
> 	}
> 	mon->class = class;
> }
> 
> 
> 
>> +		pr_debug("Not pre-allocating free-running counters\n");
>> +		return;
>> +	}
>> +
>> +	/*
>> +	 * This is the pre-allocated free-running monitors path. It always
>> +	 * allocates one monitor per PARTID * PMG.
>> +	 */
>> +	WARN_ON_ONCE(num_mbwu_mon != resctrl_arch_system_num_rmid_idx());
>> +
>> +	mon->mbwu_idx_to_mon = __alloc_mbwu_array(class, num_mbwu_mon);
> 
> This might be cleaner with a local so as to avoid setting and reverting on error.
> Also, does anything stop mon->class being set after this?
> Seems like that would be cleaner.
> 
> 	int *new_array; // up top obviously.
> 
> 	new_array = __alloc_mbwu_array(class, num_mbwu_mon);
> 	if (IS_ERR(new_array)) {
> 		pr_debug("Failed to allocate MBWU array\n");
> 		return;
> 	}
> 
> //this bit less obviously a good idea.
> 	if (mon->mbwu_idx_to_mon) {
> 		pr_debug("Releasing previous class %u's monitors\n",
> 			 mon->mbwu_idx_to_mon->level);
> 		__free_mbwu_mon(mon->class, mon->mbwu_idx_to_mon, num_mbwu_mon);
> 		kfree(mon->mbwu_idx_to_mon);
> 	}
> //so maybe skip that bit.
> //Anyhow aim is to set these only after we know we are good carry on.
> 	mon->mbwu_idx_to_mon = new_array;
> 	mon->class = class;
> 
> 
> Some parts of this might make sense without going the whole way. Maybe
> just setting the array late and leaving the class set early then
> reverting that on error.
> 
> 

I was considering moving the memory allocation out of the conditional but 
didn't get anywhere better with that idea so I've taken your suggestion and 
included the csu case as well.

	pr_debug("Updating event %u to use class %u\n", evt_id, class->level);

	/* Might not need all the monitors */
	num_mbwu_mon = __mpam_monitors_free_running(num_mbwu_mon);

	if (evt_id != QOS_L3_OCCUP_EVENT_ID && !num_mbwu_mon) {
		/*
		 * This is the pre-allocated free-running monitors path. It always
		 * allocates one monitor per PARTID * PMG.
		 */
		WARN_ON_ONCE(num_mbwu_mon != resctrl_arch_system_num_rmid_idx());

		new_array = __alloc_mbwu_array(class, num_mbwu_mon);
		if (IS_ERR(new_array)) {
			pr_debug("Failed to allocate MBWU array\n");
			return;
		}
		mon->mbwu_idx_to_mon = new_array;

		if (existing_array) {
			pr_debug("Releasing previous class %u's monitors\n",
				 existing_class->level);
			__free_mbwu_mon(existing_class, existing_array, num_mbwu_mon);
			kfree(existing_array);
		}
	} else if (evt_id != QOS_L3_OCCUP_EVENT_ID) {
		pr_debug("Not pre-allocating free-running counters\n");
	}

	exposed_mon_capable = true;
	mon->class = class;


>> +	if (IS_ERR(mon->mbwu_idx_to_mon)) {
>> +		pr_debug("Failed to allocate MBWU array\n");
>> +		mon->class = existing_class;
>> +		mon->mbwu_idx_to_mon = existing_array;
>> +		return;
>> +	}
>> +
>> +	if (existing_array) {
>> +		pr_debug("Releasing previous class %u's monitors\n",
>> +			 existing_class->level);
>> +		__free_mbwu_mon(existing_class, existing_array, num_mbwu_mon);
>> +		kfree(existing_array);
>> +	}
>>  }
>>  
>>  static void mpam_resctrl_pick_counters(void)
> 
 
Thanks,

Ben