Messages in this thread Patch in this message | | | Date | Thu, 9 Nov 2023 09:39:44 -0800 | Subject | Re: [PATCH v7 02/24] x86/resctrl: kfree() rmid_ptrs from rdtgroup_exit() | From | Reinette Chatre <> |
| |
Hi James,
Subject refers to rdtgroup_exit() but the patch is actually changing resctrl_exit().
On 10/25/2023 11:03 AM, James Morse wrote: > rmid_ptrs[] is allocated from dom_data_init() but never free()d. > > While the exit text ends up in the linker script's DISCARD section, > the direction of travel is for resctrl to be/have loadable modules. > > Add resctrl_exit_mon_l3_config() to cleanup any memory allocated > by rdt_get_mon_l3_config().
To match what patch actually does it looks like this should rather be: "Add resctrl_exit_mon_l3_config()" -> "Add resctrl_put_mon_l3_config()"
> > There is no reason to backport this to a stable kernel. > > Signed-off-by: James Morse <james.morse@arm.com> > --- > Changes since v5: > * This patch is new > > Changes since v6: > * Removed struct rdt_resource argument, added __exit markers to match the > only caller. > * Adedd a whole stack of functions to maintain symmetry. > --- > arch/x86/kernel/cpu/resctrl/core.c | 6 ++++++ > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > arch/x86/kernel/cpu/resctrl/monitor.c | 15 +++++++++++++++ > 3 files changed, 22 insertions(+) > > diff --git a/arch/x86/kernel/cpu/resctrl/core.c b/arch/x86/kernel/cpu/resctrl/core.c > index 19e0681f0435..0056c9962a44 100644 > --- a/arch/x86/kernel/cpu/resctrl/core.c > +++ b/arch/x86/kernel/cpu/resctrl/core.c > @@ -992,7 +992,13 @@ late_initcall(resctrl_late_init); > > static void __exit resctrl_exit(void) > { > + struct rdt_resource *r = &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl; > + > cpuhp_remove_state(rdt_online); > + > + if (r->mon_capable) > + rdt_put_mon_l3_config(r); > + > rdtgroup_exit(); > }
I expect cleanup to do the inverse of init. I do not know what was the motivation for the rdtgroup_exit() to follow cpuhp_remove_state() but I was expecting this new cleanup to be done after rdtgroup_exit() to be inverse of init. This cleanup is inserted in middle of two existing cleanup - could you please elaborate how this location was chosen?
Reinette
| |