[PATCH] arm_mpam: Only schedule mpam_enable work after first successful MSC probe

[PATCH] arm_mpam: Only schedule mpam_enable work after first successful MSC probe

[ping.li]

mpam_discovery_cpu_online() sets new_device_probed unconditionally after
processing each reachable MSC. Once an MSC has already been probed
(msc->probed is true), later CPUs sharing it skip
mpam_msc_hw_probe() but still leave err at its default value of 0.
As a result, new_device_probed is still set to true, causing
mpam_enable_work to be scheduled again even though no new hardware was
probed.

Set new_device_probed only when mpam_msc_hw_probe() is called and
succeeds.

Fixes: 8f8d0ac1da78 ("arm_mpam: Add cpuhp callbacks to probe MSC hardware")
Signed-off-by: ping.li <ping.li@horizon.auto>
---
 drivers/resctrl/mpam_devices.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 2f09f4b78bd3..fefdcf588932 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -1866,13 +1866,15 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
 			continue;
 
 		mutex_lock(&msc->probe_lock);
-		if (!msc->probed)
+		if (!msc->probed) {
 			err = mpam_msc_hw_probe(msc);
+			if (!err)
+				new_device_probed = true;
+		}
 		mutex_unlock(&msc->probe_lock);
 
 		if (err)
 			break;
-		new_device_probed = true;
 	}
 
 	if (new_device_probed && !err)
-- 
2.34.1

[PATCH v2] arm_mpam: Only schedule mpam_enable work after first successful MSC probe

[ping.li]

From: Ping Li <leeonion.muyu@gmail.com>

mpam_discovery_cpu_online() sets new_device_probed unconditionally after
processing each reachable MSC. Once an MSC has already been probed
(msc->probed is true), later CPUs sharing it skip
mpam_msc_hw_probe() but still leave err at its default value of 0.
As a result, new_device_probed is still set to true, causing
mpam_enable_work to be scheduled again even though no new hardware was
probed.

Set new_device_probed only when mpam_msc_hw_probe() is called and
succeeds.

Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
---
Changes in v2:
- Drop the Fixes: tag, as the extra mpam_enable() calls cause no real
  harm: schedule_work() merges the duplicate work, and mpam_enable()
  is a no-op until all MSCs have been probed. This is a cleanup, not a
  bug fix.

 drivers/resctrl/mpam_devices.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 2f09f4b78bd3..fefdcf588932 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -1866,13 +1866,15 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
 			continue;
 
 		mutex_lock(&msc->probe_lock);
-		if (!msc->probed)
+		if (!msc->probed) {
 			err = mpam_msc_hw_probe(msc);
+			if (!err)
+				new_device_probed = true;
+		}
 		mutex_unlock(&msc->probe_lock);
 
 		if (err)
 			break;
-		new_device_probed = true;
 	}
 
 	if (new_device_probed && !err)
-- 
2.34.1

Re: [PATCH v2] arm_mpam: Only schedule mpam_enable work after first successful MSC probe

[Ben Horgan]

Hi Ping,

On 08/09/2026 03:48, ping.li wrote:
> From: Ping Li <leeonion.muyu@gmail.com>
> 
> mpam_discovery_cpu_online() sets new_device_probed unconditionally after
> processing each reachable MSC. Once an MSC has already been probed
> (msc->probed is true), later CPUs sharing it skip
> mpam_msc_hw_probe() but still leave err at its default value of 0.
> As a result, new_device_probed is still set to true, causing
> mpam_enable_work to be scheduled again even though no new hardware was
> probed.

This patch is an improvement but, thinking again, it looks there is scope getting rid of
mpam_enable() altogether. Rather than walking the list after each hw probe we could increment an
atomic variable, similar to what is done in mpam_msc_drv_probe(), and then just schedule
mpam_enable_once(). What do you think?

Thanks,

Ben

> 
> Set new_device_probed only when mpam_msc_hw_probe() is called and
> succeeds.
> 
> Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
> ---
> Changes in v2:
> - Drop the Fixes: tag, as the extra mpam_enable() calls cause no real
>   harm: schedule_work() merges the duplicate work, and mpam_enable()
>   is a no-op until all MSCs have been probed. This is a cleanup, not a
>   bug fix.
> 
>  drivers/resctrl/mpam_devices.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 2f09f4b78bd3..fefdcf588932 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1866,13 +1866,15 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
>  			continue;
>  
>  		mutex_lock(&msc->probe_lock);
> -		if (!msc->probed)
> +		if (!msc->probed) {
>  			err = mpam_msc_hw_probe(msc);
> +			if (!err)
> +				new_device_probed = true;
> +		}
>  		mutex_unlock(&msc->probe_lock);
>  
>  		if (err)
>  			break;
> -		new_device_probed = true;
>  	}
>  
>  	if (new_device_probed && !err)

[PATCH v3] arm_mpam: Use an atomic counter to schedule mpam_enable_once()

[Ping Li]

mpam_discovery_cpu_online() scheduled mpam_enable() after each MSC
probe, and mpam_enable() walked the whole MSC list taking each probe
lock in turn to check if all the MSCs had been probed.

Count the MSCs that have had their hardware probed with
mpam_num_msc_probed, and schedule mpam_enable_once() directly when the
count reaches mpam_num_msc, the number of MSCs the driver discovered.
MSCs that were already probed no longer cause work to be scheduled.

Suggested-by: Ben Horgan <ben.horgan@arm.com>
Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
---
Changes in v3:
- Rework per review feedback: instead of scheduling mpam_enable() after
  each probe and walking the MSC list to check whether all MSCs have
  been probed, count the probed MSCs and schedule mpam_enable_once()
  directly when the last one is probed.

 drivers/resctrl/mpam_devices.c  | 47 +++++++++------------------------
 drivers/resctrl/mpam_internal.h |  1 -
 2 files changed, 12 insertions(+), 36 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 2f09f4b78bd3..c6000bc2157a 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -55,6 +55,11 @@ struct srcu_struct mpam_srcu;
  */
 static atomic_t mpam_num_msc;
 
+/* Number of MSCs that have had their hardware probed */
+static atomic_t mpam_num_msc_probed;
+
+static void mpam_enable_once(struct work_struct *work);
+
 static int mpam_cpuhp_state;
 static DEFINE_MUTEX(mpam_cpuhp_state_lock);
 
@@ -72,7 +77,7 @@ static DEFINE_SPINLOCK(partid_max_lock);
  * scheduled via this work_struct. If access to an MSC depends on a CPU that
  * was not brought online at boot, this can happen surprisingly late.
  */
-static DECLARE_WORK(mpam_enable_work, &mpam_enable);
+static DECLARE_WORK(mpam_enable_work, &mpam_enable_once);
 
 /*
  * All mpam error interrupts indicate a software bug. On receipt, disable the
@@ -1854,7 +1859,6 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
 {
 	int err = 0;
 	struct mpam_msc *msc;
-	bool new_device_probed = false;
 
 	if (mpam_is_enabled())
 		return 0;
@@ -1866,17 +1870,18 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
 			continue;
 
 		mutex_lock(&msc->probe_lock);
-		if (!msc->probed)
+		if (!msc->probed) {
 			err = mpam_msc_hw_probe(msc);
+			if (!err && atomic_add_return(1, &mpam_num_msc_probed) ==
+			    atomic_read(&mpam_num_msc))
+				schedule_work(&mpam_enable_work);
+		}
 		mutex_unlock(&msc->probe_lock);
 
 		if (err)
 			break;
-		new_device_probed = true;
 	}
 
-	if (new_device_probed && !err)
-		schedule_work(&mpam_enable_work);
 	if (err) {
 		mpam_disable_reason = "error during probing";
 		schedule_work(&mpam_broken_work);
@@ -2713,7 +2718,7 @@ static int mpam_allocate_config(void)
 	return 0;
 }
 
-static void mpam_enable_once(void)
+static void mpam_enable_once(struct work_struct *work)
 {
 	int err;
 
@@ -2867,34 +2872,6 @@ void mpam_disable(struct work_struct *ignored)
 	pr_err_once("MPAM disabled due to %s\n", mpam_disable_reason);
 }
 
-/*
- * Enable mpam once all devices have been probed.
- * Scheduled by mpam_discovery_cpu_online() once all devices have been created.
- * Also scheduled when new devices are probed when new CPUs come online.
- */
-void mpam_enable(struct work_struct *work)
-{
-	static atomic_t once;
-	struct mpam_msc *msc;
-	bool all_devices_probed = true;
-
-	/* Have we probed all the hw devices? */
-	guard(srcu)(&mpam_srcu);
-	list_for_each_entry_srcu(msc, &mpam_all_msc, all_msc_list,
-				 srcu_read_lock_held(&mpam_srcu)) {
-		mutex_lock(&msc->probe_lock);
-		if (!msc->probed)
-			all_devices_probed = false;
-		mutex_unlock(&msc->probe_lock);
-
-		if (!all_devices_probed)
-			break;
-	}
-
-	if (all_devices_probed && !atomic_fetch_inc(&once))
-		mpam_enable_once();
-}
-
 #define maybe_update_config(cfg, feature, newcfg, member, changes) do { \
 	if (mpam_has_feature(feature, newcfg) &&			\
 	    (newcfg)->member != (cfg)->member) {			\
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 04d1a59f02af..72bf296c846e 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -453,7 +453,6 @@ extern u16 mpam_partid_max;
 extern u8 mpam_pmg_max;
 
 /* Scheduled work callback to enable mpam once all MSC have been probed */
-void mpam_enable(struct work_struct *work);
 void mpam_disable(struct work_struct *work);
 
 /* Reset all the RIS in a class under cpus_read_lock() */
-- 
2.34.1

Re: [PATCH v3] arm_mpam: Use an atomic counter to schedule mpam_enable_once()

[Ben Horgan]

Hi Ping,

On 15/09/2026 04:18, Ping Li wrote:
> mpam_discovery_cpu_online() scheduled mpam_enable() after each MSC
> probe, and mpam_enable() walked the whole MSC list taking each probe
> lock in turn to check if all the MSCs had been probed.
> 
> Count the MSCs that have had their hardware probed with
> mpam_num_msc_probed, and schedule mpam_enable_once() directly when the
> count reaches mpam_num_msc, the number of MSCs the driver discovered.
> MSCs that were already probed no longer cause work to be scheduled.
> 
> Suggested-by: Ben Horgan <ben.horgan@arm.com>

Thanks for implementing the suggestion :)

> Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
> ---
> Changes in v3:
> - Rework per review feedback: instead of scheduling mpam_enable() after
>   each probe and walking the MSC list to check whether all MSCs have
>   been probed, count the probed MSCs and schedule mpam_enable_once()
>   directly when the last one is probed.
> 
>  drivers/resctrl/mpam_devices.c  | 47 +++++++++------------------------
>  drivers/resctrl/mpam_internal.h |  1 -
>  2 files changed, 12 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 2f09f4b78bd3..c6000bc2157a 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -55,6 +55,11 @@ struct srcu_struct mpam_srcu;
>   */
>  static atomic_t mpam_num_msc;
>  
> +/* Number of MSCs that have had their hardware probed */
> +static atomic_t mpam_num_msc_probed;
> +
> +static void mpam_enable_once(struct work_struct *work);
> +
>  static int mpam_cpuhp_state;
>  static DEFINE_MUTEX(mpam_cpuhp_state_lock);
>  
> @@ -72,7 +77,7 @@ static DEFINE_SPINLOCK(partid_max_lock);
>   * scheduled via this work_struct. If access to an MSC depends on a CPU that
>   * was not brought online at boot, this can happen surprisingly late.
>   */
> -static DECLARE_WORK(mpam_enable_work, &mpam_enable);
> +static DECLARE_WORK(mpam_enable_work, &mpam_enable_once);
>  
>  /*
>   * All mpam error interrupts indicate a software bug. On receipt, disable the
> @@ -1854,7 +1859,6 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
>  {
>  	int err = 0;
>  	struct mpam_msc *msc;
> -	bool new_device_probed = false;
>  
>  	if (mpam_is_enabled())
>  		return 0;
> @@ -1866,17 +1870,18 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
>  			continue;
>  
>  		mutex_lock(&msc->probe_lock);

If we switch to use guard(mutex)(&msc->probe_lock) then we can get rid of the nested ifs.
This allows us to...

> -		if (!msc->probed)
... 'continue' if already probed

> +		if (!msc->probed) {
>  			err = mpam_msc_hw_probe(msc);
... return early on error

> +			if (!err && atomic_add_return(1, &mpam_num_msc_probed) ==
> +			    atomic_read(&mpam_num_msc))

... and remove the !err from this check.

Instead of mpam_num_msc check against fw_num_msc so that we don't need to take into account extra
calls to mpam_msc_drv_probe() due to unbind/bind potentially incrementing mpam_num_msc past
fw_num_msc. As we disable bind/unbind by setting 'suppress_bind_attrs' this can't actually occur but
I think using fw_num_msc is more robust.

> +				schedule_work(&mpam_enable_work);
> +		}
>  		mutex_unlock(&msc->probe_lock);
>  
>  		if (err)
>  			break;
> -		new_device_probed = true;
>  	}
>  
> -	if (new_device_probed && !err)
> -		schedule_work(&mpam_enable_work);
>  	if (err) {
>  		mpam_disable_reason = "error during probing";
>  		schedule_work(&mpam_broken_work);
> @@ -2713,7 +2718,7 @@ static int mpam_allocate_config(void)
>  	return 0;
>  }
>  
> -static void mpam_enable_once(void)
> +static void mpam_enable_once(struct work_struct *work)
>  {
>  	int err;
>  
> @@ -2867,34 +2872,6 @@ void mpam_disable(struct work_struct *ignored)
>  	pr_err_once("MPAM disabled due to %s\n", mpam_disable_reason);
>  }
>  
> -/*
> - * Enable mpam once all devices have been probed.
> - * Scheduled by mpam_discovery_cpu_online() once all devices have been created.
> - * Also scheduled when new devices are probed when new CPUs come online.
> - */
> -void mpam_enable(struct work_struct *work)
> -{
> -	static atomic_t once;
> -	struct mpam_msc *msc;
> -	bool all_devices_probed = true;
> -
> -	/* Have we probed all the hw devices? */
> -	guard(srcu)(&mpam_srcu);
> -	list_for_each_entry_srcu(msc, &mpam_all_msc, all_msc_list,
> -				 srcu_read_lock_held(&mpam_srcu)) {
> -		mutex_lock(&msc->probe_lock);
> -		if (!msc->probed)
> -			all_devices_probed = false;
> -		mutex_unlock(&msc->probe_lock);
> -
> -		if (!all_devices_probed)
> -			break;
> -	}
> -
> -	if (all_devices_probed && !atomic_fetch_inc(&once))
> -		mpam_enable_once();
> -}
> -
>  #define maybe_update_config(cfg, feature, newcfg, member, changes) do { \
>  	if (mpam_has_feature(feature, newcfg) &&			\
>  	    (newcfg)->member != (cfg)->member) {			\
> diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
> index 04d1a59f02af..72bf296c846e 100644
> --- a/drivers/resctrl/mpam_internal.h
> +++ b/drivers/resctrl/mpam_internal.h
> @@ -453,7 +453,6 @@ extern u16 mpam_partid_max;
>  extern u8 mpam_pmg_max;
>  
>  /* Scheduled work callback to enable mpam once all MSC have been probed */

This comment should be removed along with the declaration.

Thanks,

Ben

> -void mpam_enable(struct work_struct *work);
>  void mpam_disable(struct work_struct *work);
>  
>  /* Reset all the RIS in a class under cpus_read_lock() */

[PATCH v4] arm_mpam: Use an atomic counter to schedule mpam_enable_once()

[Ping Li]

mpam_discovery_cpu_online() scheduled mpam_enable() after each MSC
probe, and mpam_enable() walked the whole MSC list taking each probe
lock in turn to check if all the MSCs had been probed.

Count the MSCs that have had their hardware probed with
mpam_num_msc_probed, and schedule mpam_enable_once() directly when the
count reaches fw_num_msc, the number of MSCs reported by firmware.
MSCs that were already probed no longer cause work to be scheduled.

Suggested-by: Ben Horgan <ben.horgan@arm.com>
Signed-off-by: Ping Li <leeonion.muyu@gmail.com>
---
Changes in v4:
- Use guard(mutex) for the probe lock and continue/break to flatten
  the nesting, per review feedback.
- Compare the probed count against fw_num_msc instead of
  mpam_num_msc, which is more robust against unbind/bind incrementing
  mpam_num_msc past fw_num_msc.
- Remove the stale comment for the removed mpam_enable() declaration.

 drivers/resctrl/mpam_devices.c  | 57 ++++++++++-----------------------
 drivers/resctrl/mpam_internal.h |  2 --
 2 files changed, 17 insertions(+), 42 deletions(-)

diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
index 2f09f4b78bd3..adfb734755e6 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -49,12 +49,20 @@ static LIST_HEAD(mpam_all_msc);
 
 struct srcu_struct mpam_srcu;
 
+/* Number of MSCs reported by firmware */
+static int fw_num_msc;
+
 /*
  * Number of MSCs that have been probed. Once all MSCs have been probed MPAM
  * can be enabled.
  */
 static atomic_t mpam_num_msc;
 
+/* Number of MSCs that have had their hardware probed */
+static atomic_t mpam_num_msc_probed;
+
+static void mpam_enable_once(struct work_struct *work);
+
 static int mpam_cpuhp_state;
 static DEFINE_MUTEX(mpam_cpuhp_state_lock);
 
@@ -72,7 +80,7 @@ static DEFINE_SPINLOCK(partid_max_lock);
  * scheduled via this work_struct. If access to an MSC depends on a CPU that
  * was not brought online at boot, this can happen surprisingly late.
  */
-static DECLARE_WORK(mpam_enable_work, &mpam_enable);
+static DECLARE_WORK(mpam_enable_work, &mpam_enable_once);
 
 /*
  * All mpam error interrupts indicate a software bug. On receipt, disable the
@@ -1854,7 +1862,6 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
 {
 	int err = 0;
 	struct mpam_msc *msc;
-	bool new_device_probed = false;
 
 	if (mpam_is_enabled())
 		return 0;
@@ -1865,18 +1872,18 @@ static int mpam_discovery_cpu_online(unsigned int cpu)
 		if (!cpumask_test_cpu(cpu, &msc->accessibility))
 			continue;
 
-		mutex_lock(&msc->probe_lock);
-		if (!msc->probed)
-			err = mpam_msc_hw_probe(msc);
-		mutex_unlock(&msc->probe_lock);
+		guard(mutex)(&msc->probe_lock);
+		if (msc->probed)
+			continue;
 
+		err = mpam_msc_hw_probe(msc);
 		if (err)
 			break;
-		new_device_probed = true;
+
+		if (atomic_add_return(1, &mpam_num_msc_probed) == fw_num_msc)
+			schedule_work(&mpam_enable_work);
 	}
 
-	if (new_device_probed && !err)
-		schedule_work(&mpam_enable_work);
 	if (err) {
 		mpam_disable_reason = "error during probing";
 		schedule_work(&mpam_broken_work);
@@ -2106,8 +2113,6 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
 	return msc;
 }
 
-static int fw_num_msc;
-
 static int mpam_msc_drv_probe(struct platform_device *pdev)
 {
 	int err;
@@ -2713,7 +2718,7 @@ static int mpam_allocate_config(void)
 	return 0;
 }
 
-static void mpam_enable_once(void)
+static void mpam_enable_once(struct work_struct *work)
 {
 	int err;
 
@@ -2867,34 +2872,6 @@ void mpam_disable(struct work_struct *ignored)
 	pr_err_once("MPAM disabled due to %s\n", mpam_disable_reason);
 }
 
-/*
- * Enable mpam once all devices have been probed.
- * Scheduled by mpam_discovery_cpu_online() once all devices have been created.
- * Also scheduled when new devices are probed when new CPUs come online.
- */
-void mpam_enable(struct work_struct *work)
-{
-	static atomic_t once;
-	struct mpam_msc *msc;
-	bool all_devices_probed = true;
-
-	/* Have we probed all the hw devices? */
-	guard(srcu)(&mpam_srcu);
-	list_for_each_entry_srcu(msc, &mpam_all_msc, all_msc_list,
-				 srcu_read_lock_held(&mpam_srcu)) {
-		mutex_lock(&msc->probe_lock);
-		if (!msc->probed)
-			all_devices_probed = false;
-		mutex_unlock(&msc->probe_lock);
-
-		if (!all_devices_probed)
-			break;
-	}
-
-	if (all_devices_probed && !atomic_fetch_inc(&once))
-		mpam_enable_once();
-}
-
 #define maybe_update_config(cfg, feature, newcfg, member, changes) do { \
 	if (mpam_has_feature(feature, newcfg) &&			\
 	    (newcfg)->member != (cfg)->member) {			\
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/mpam_internal.h
index 04d1a59f02af..085f086e2b2d 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -452,8 +452,6 @@ extern struct list_head mpam_classes;
 extern u16 mpam_partid_max;
 extern u8 mpam_pmg_max;
 
-/* Scheduled work callback to enable mpam once all MSC have been probed */
-void mpam_enable(struct work_struct *work);
 void mpam_disable(struct work_struct *work);
 
 /* Reset all the RIS in a class under cpus_read_lock() */
-- 
2.34.1