[PATCH] arm_mpam: Try reading again if MPAM instance returns not ready

[Zeng Heng]

On 2025/9/20 0:11, James Morse wrote:
> Hi Zeng,
> 
> On 16/09/2025 14:17, Zeng Heng wrote:
>> After updating the monitor configuration, the first read of the monitoring
>> result requires waiting for the "not ready" duration before an effective
>> value can be obtained.
> 
> May need to wait - some platforms need to do this, some don't.
> Yours is the first I've heard of that does this!
> 

I'm afraid similar platforms do exist. As long as one component has more
than one MSC, after first updating the component’s monitor every MSC
instance needs to wait for MAX_NRDY_USEC us before reading the monitor
result.

In fact, most platforms don’t have nearly as many performance monitors
as PARTIDs, so the monitors often have to be time-shared, which making
the problem even more pronounced.

> 
>> Because a component consists of multiple MPAM instances, after updating the
>> configuration of each instance, should wait for the "not ready" period of
>> per single instance before the valid monitoring value can be obtained, not
>> just wait for once interval per component.
> 
> I'm really trying to avoid that ... if you have ~200 MSC pretending to be one thing, you'd
> wait 200x the maximum period. On systems with CMN, the number of MSC scales with the
> number of CPUs, so 200x isn't totally crazy.
> > I think the real problem here is the driver doesn't go on to 
reconfigure MSC-2 if MSC-1
> returned not-ready, meaning the "I'll only wait once" logic kicks in and returns not-ready
> to the user. (which is presumably what you're seeing?)

Yes, exactly.

> 
> Does this solve your problem?:
> -----------------%<-----------------
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 404bd4c1fd5e..2f39d0339349 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1395,7 +1395,7 @@ static void __ris_msmon_read(void *arg)
> 
>   static int _msmon_read(struct mpam_component *comp, struct mon_read *arg)
>   {
> -       int err, idx;
> +       int err, any_err = 0, idx;
>          struct mpam_msc *msc;
>          struct mpam_vmsc *vmsc;
>          struct mpam_msc_ris *ris;
> @@ -1412,15 +1412,19 @@ static int _msmon_read(struct mpam_component *comp, stru
> ct mon_read *arg)
>                                                      true);
>                          if (!err && arg->err)
>                                  err = arg->err;
> +
> +                       /*
> +                        * Save one error to be returned to the caller, but
> +                        * keep reading counters so that the get reprogrammed.
> +                        * On platforms with NRDY this lets us wait once.
> +                        */
>                          if (err)
> -                               break;
> +                               any_err = err;
>                  }
> -               if (err)
> -                       break;
>          }
>          srcu_read_unlock(&mpam_srcu, idx);
> 
> -       return err;
> +       return any_err;
>   }
> 
>   int mpam_msmon_read(struct mpam_component *comp, struct mon_cfg *ctx,
> -----------------%<-----------------
> 

I agree with this modification: Reconfigure all MSCs first, then if any
of them returns EBUSY, wait just once for MAX_NRDY_USEC and re-read
monitor result, this guarantees that the monitor result is valid.



Thanks,
Zeng Heng