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

Zeng Heng
2025-09-20T10:14:16Z

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