Hi Tony,
On 8/11/25 11:17 AM, Tony Luck wrote:
> resctrl assumes that only the L3 resource supports monitor events, so
> it simply takes the rdt_resource::num_rmid from RDT_RESOURCE_L3 as
> the number of RMIDs.
There does not seem to be anything wrong with "taking num_rmid ... as
the number of RMIDs". How about:
"the number of RMIDs" -> "the system's number of RMIDs"?
>
> The addition of telemetry events in a different resource breaks that
> assumption.
>
> Compute the number of available RMIDs as the minimum value across
> all mon capable resources (analogous to how the number of CLOSIDs
"mon capable" -> "monitoring capable" or "mon_capable"
> is computed across alloc capable resources).
"alloc capable" -> "allocation capable" or "alloc_capable"
>
> Note that mount time enumeration of the telemetry resource means that
> this number can be reduced. If this happens, then some memory will
> be wasted as the allocations for rdt_l3_mon_domain::states[] will be
"rdt_l3_mon_domain::states[]" -> "rdt_l3_mon_domain::mbm_states[] and
rdt_l3_mon_domain::rmid_busy_llc created during resctrl initialization"
> larger than needed.
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> arch/x86/kernel/cpu/resctrl/core.c | 15 +++++++++++++--
> fs/resctrl/rdtgroup.c | 5 +++++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c
> index 2b011f9efc73..0284da075ea6 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -110,12 +110,23 @@ struct rdt_hw_resource rdt_resources_all[RDT_NUM_RESOURCES] = {
> },
> };
>
> +/**
> + * resctrl_arch_system_num_rmid_idx - Compute number of supported RMIDs
> + * (minimum across all mon capable resource)
"mon capable" -> "monitoring capable" or "mon_capable"
> + *
> + * Return: Number of supported RMIDs at time of call. Note that mount time
> + * enumeration of resources may reduce the number.
> + */
> u32 resctrl_arch_system_num_rmid_idx(void)
> {
> - struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl;
> + u32 num_rmids = U32_MAX;
> + struct rdt_resource *r;
> +
> + for_each_mon_capable_rdt_resource(r)
> + num_rmids = min(num_rmids, r->num_rmid);
>
> /* RMID are independent numbers for x86. num_rmid_idx == num_rmid */
> - return r->num_rmid;
> + return num_rmids == U32_MAX ? 0 : num_rmids;
> }
>
> struct rdt_resource *resctrl_arch_get_resource(enum resctrl_res_level l)
> diff --git a/fs/resctrl/rdtgroup.c b/fs/resctrl/rdtgroup.c
> index 55ad99bd77d2..5352480eb55c 100644
> --- a/fs/resctrl/rdtgroup.c
> +++ b/fs/resctrl/rdtgroup.c
> @@ -4116,6 +4116,11 @@ void resctrl_offline_mon_domain(struct rdt_resource *r, struct rdt_domain_hdr *h
> * During boot this may be called before global allocations have been made by
> * resctrl_mon_l3_resource_init().
> *
> + * This routine is called at resctrl init time. The number of supported RMIDs
> + * may be reduced if additional mon capable resources are enumerated at mount
"mon capable" -> "monitoring capable" or "mon_capable"
> + * time. This means the rdt_l3_mon_domain::states[] allocations may be larger
"rdt_l3_mon_domain::states[]" -> "rdt_l3_mon_domain::mbm_states[] and rdt_l3_mon_domain::rmid_busy_llc"
> + * than needed.
> + *
> * Returns 0 for success, or -ENOMEM.
> */
> static int domain_setup_l3_mon_state(struct rdt_resource *r, struct rdt_l3_mon_domain *d)
Reinette