[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c092359-c407-461a-be0c-289c5e424f05@intel.com>
Date: Fri, 3 Oct 2025 17:12:09 -0700
From: Reinette Chatre <reinette.chatre@...el.com>
To: Tony Luck <tony.luck@...el.com>, Fenghua Yu <fenghuay@...dia.com>, "Maciej
Wieczor-Retman" <maciej.wieczor-retman@...el.com>, Peter Newman
<peternewman@...gle.com>, James Morse <james.morse@....com>, Babu Moger
<babu.moger@....com>, Drew Fustini <dfustini@...libre.com>, Dave Martin
<Dave.Martin@....com>, Chen Yu <yu.c.chen@...el.com>
CC: <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
<patches@...ts.linux.dev>
Subject: Re: [PATCH v11 26/31] fs/resctrl: Move RMID initialization to first
mount
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@...el.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
Powered by blists - more mailing lists