Re: [PATCH v11 26/31] fs/resctrl: Move RMID initialization to first mount

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony,

On 9/25/25 1:03 PM, Tony Luck wrote:
> L3 monitor features are enumerated during resctrl initialization
> and rmid_ptrs[] that tracks all RMIDs and depends on the
> number of supported RMIDs is allocated during this time.
> 
> Telemetry monitor features are enumerated during first resctrl mount and
> may support a different number of RMIDs compared to L3 monitor features.
> 
> Delay allocation and initialization of rmid_ptrs[] until first mount.
> Since the number of RMIDs cannot change on later mounts, keep the same
> set of rmid_ptrs[] until resctrl_exit(). This is required because the
> limbo handler keeps running after resctrl is unmounted and may likely
> need to access rmid_ptrs[] as it keeps tracking busy RMIDs after unmount.
> 
> Rename routines to match what they now do:
> dom_data_init() -> setup_rmid_lru_list()
> dom_data_exit() -> free_rmid_lru_list()
> 
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
>  fs/resctrl/internal.h |  4 ++++
>  fs/resctrl/monitor.c  | 50 +++++++++++++++++++------------------------
>  fs/resctrl/rdtgroup.c |  5 +++++
>  3 files changed, 31 insertions(+), 28 deletions(-)
> 
> diff --git a/fs/resctrl/internal.h b/fs/resctrl/internal.h
> index aee6c4684f81..223a6cc6a64a 100644
> --- a/fs/resctrl/internal.h
> +++ b/fs/resctrl/internal.h
> @@ -369,6 +369,10 @@ int closids_supported(void);
>  
>  void closid_free(int closid);
>  
> +int setup_rmid_lru_list(void);
> +
> +void free_rmid_lru_list(void);
> +
>  int alloc_rmid(u32 closid);
>  
>  void free_rmid(u32 closid, u32 rmid);
> diff --git a/fs/resctrl/monitor.c b/fs/resctrl/monitor.c
> index 5960a0afd0ca..c0e1b672afce 100644
> --- a/fs/resctrl/monitor.c
> +++ b/fs/resctrl/monitor.c
> @@ -880,20 +880,27 @@ void mbm_setup_overflow_handler(struct rdt_l3_mon_domain *dom, unsigned long del
>  		schedule_delayed_work_on(cpu, &dom->mbm_over, delay);
>  }
>  
> -static int dom_data_init(struct rdt_resource *r)
> +int setup_rmid_lru_list(void)
>  {
>  	u32 idx_limit = resctrl_arch_system_num_rmid_idx();

Should this be done after the resctrl_arch_mon_capable() check? Seems
unnecessary to determine the number of RMID on a system that does not
support monitoring.

>  	struct rmid_entry *entry = NULL;
> -	int err = 0, i;
>  	u32 idx;
> +	int i;
>  
> -	mutex_lock(&rdtgroup_mutex);
> +	if (!resctrl_arch_mon_capable())
> +		return 0;
> +
> +	/*
> +	 * Called on every mount, but the number of RMIDs cannot change
> +	 * after the first mount, so keep using the same set of rmid_ptrs[]
> +	 * until resctrl_exit().

Could you please add the motivation that limbo handler accesses rmid_ptrs[]
after unmount? This seems a much stronger motivation about why this is not freed on
unmount and thus valuable for anyone that wants to refactor this later.

> +	 */
> +	if (rmid_ptrs)
> +		return 0;
>  
>  	rmid_ptrs = kcalloc(idx_limit, sizeof(struct rmid_entry), GFP_KERNEL);
> -	if (!rmid_ptrs) {
> -		err = -ENOMEM;
> -		goto out_unlock;
> -	}
> +	if (!rmid_ptrs)
> +		return -ENOMEM;
>  
>  	for (i = 0; i < idx_limit; i++) {
>  		entry = &rmid_ptrs[i];

Reinette