Re: [PATCH 06/11] fs/resctrl: Pass error reading event through to user space

[Ben Horgan]

Hi Reinette,

On 3/2/26 18:46, Reinette Chatre wrote:
> Reading of event data is managed through populating a struct rmid_read with
> properties of event needing to be read. This data is dispatched to an
> appropriate CPU and upon completion any error can be found in rmid_read::err,
> or on success the event data will be in rmid_read::val.
> 
> rmid_read::err is not updated in the unlikely scenario that the reading
> of the event was dispatched to a wrong CPU. If this ever occurs due to
> a bug in resctrl the user space read will return "success" but the data
> reported will be invalid.
> 
> Ensure accurate error reporting so that if there may be an issue with
> how resctrl picks a CPU it could be learned with an error to user space
> instead of silent failure.

Looks good to me. mon_event_count() won't discard the error as the same
cpu and domain choice is used for each __mon_event_count() call.

Reviewed-by: Ben Horgan <ben.horgan@xxxxxxx>

> 
> Signed-off-by: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> ---
>  fs/resctrl/monitor.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 421c70f96426..f1a08fdbdd61 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -453,8 +453,10 @@ static int __l3_mon_event_count(struct rdtgroup *rdtgrp, struct rmid_read *rr)
>  	}
>  
>  	/* Reading a single domain, must be on a CPU in that domain. */
> -	if (!cpumask_test_cpu(cpu, &d->hdr.cpu_mask))
> +	if (!cpumask_test_cpu(cpu, &d->hdr.cpu_mask)) {
> +		rr->err = -EIO;
>  		return -EINVAL;
> +	}
>  	if (rr->is_mbm_cntr)
>  		rr->err = resctrl_arch_cntr_read(rr->r, d, closid, rmid, cntr_id,
>  						 rr->evt->evtid, &tval);
> @@ -491,8 +493,10 @@ static int __l3_mon_event_count_sum(struct rdtgroup *rdtgrp, struct rmid_read *r
>  	}
>  
>  	/* Summing domains that share a cache, must be on a CPU for that cache. */
> -	if (!cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map))
> +	if (!cpumask_test_cpu(cpu, &rr->ci->shared_cpu_map)) {
> +		rr->err = -EIO;
>  		return -EINVAL;
> +	}
>  
>  	/*
>  	 * Legacy files must report the sum of an event across all