Received-SPF: pass (google.com: domain of linux-kernel+bounces-2082-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <6ebf108a-49e7-4fc7-a61e-b993ab696c6c@intel.com>
Date: Fri, 15 Dec 2023 20:57:49 -0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v8 02/24] x86/resctrl: kfree() rmid_ptrs from
 resctrl_exit()
Content-Language: en-US
To: James Morse <james.morse@arm.com>, <x86@kernel.org>,
	<linux-kernel@vger.kernel.org>
CC: Fenghua Yu <fenghua.yu@intel.com>, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, H Peter Anvin
	<hpa@zytor.com>, Babu Moger <Babu.Moger@amd.com>,
	<shameerali.kolothum.thodi@huawei.com>, D Scott Phillips OS
	<scott@os.amperecomputing.com>, <carl@os.amperecomputing.com>,
	<lcherian@marvell.com>, <bobo.shaobowang@huawei.com>,
	<tan.shaopeng@fujitsu.com>, <baolin.wang@linux.alibaba.com>, Jamie Iles
	<quic_jiles@quicinc.com>, Xin Hao <xhao@linux.alibaba.com>,
	<peternewman@google.com>, <dfustini@baylibre.com>, <amitsinght@marvell.com>
References: <20231215174343.13872-1-james.morse@arm.com>
 <20231215174343.13872-3-james.morse@arm.com>
From: Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <20231215174343.13872-3-james.morse@arm.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dTRrRVpkWVFyZlpYaS9FSHk0SWpEaFU5dHJXcVdaKzZ0eGNwZGRnSjRZWEQr?=
 =?utf-8?B?TDNZOVhxVWdDc05VZmhPT1dsTjFsVHlXbHUvRnhBWXlvQkZ1YzJUeksyQjFq?=
 =?utf-8?B?bm9sS2VKMEd2OWVMVFF4NTR0ZUJ1YnpBbFdrQ3dza01QN20xNEIyM0ZlaTVY?=
 =?utf-8?B?QmN5ZVdYRHhPbXI4UGlHaXVaOThVVTBWNktCRmRKejhLd0lwWVZRVVk2YlY3?=
 =?utf-8?B?QU54ZTAzNUxHREE1TzhvUEVSc2lUOENlZ2FZdWZRQmp2WDM2TndSdnU0aElW?=
 =?utf-8?B?bFI5TllsbVJacVhaWWRkSU9vQmRoeElha3FYd2VoZVRIMUo0b2pmTXlQc2lh?=
 =?utf-8?B?TlFOZm4xYzFEYWFmSEx5NmExSHlWcmVsY1B2enhKODNHeWpNRVB2Y09NbHdD?=
 =?utf-8?B?TldoS29OdTdjU1V5azVNOC9FTVUyRWdTU3l2bXVOYlk2ZTdJRnY3VGJuL0ti?=
 =?utf-8?B?Y0g2NVZ1MXBmRTVEVkZ5Rjh0clVHSkZudmZSRmI5Q1VaNmNhN0dvQ2RXWnRU?=
 =?utf-8?B?MWtzZmg5RUxBaDdPcVpzYy9YVFlxemZNci9DNDRDUFBSY2tjbHNMVFdnVndF?=
 =?utf-8?B?SFd5eXpDYm5ydko4V1JmUXRpd21BSW8vUnNNR05Pb0dSWUZwell4bFNNRTlw?=
 =?utf-8?B?VjQ3V21URS8yNGx1V3ptLzZYK2RQYjlZNnBTQlNBM0Z2K3pWcDVJVk9vWlp3?=
 =?utf-8?B?eEVIUVZPeERiaHplTnBPbGx0Q0tLRGgvM1VMaW5GUFNjc0dFWFM4YzJPNVdU?=
 =?utf-8?B?MlRYaUMzaUlEeXdFby9wQ0s1RkxROVl6NUNGRml6NHllakhWb2ZNV2VVSUc1?=
 =?utf-8?B?NmlmbitqZFZ4aC9iekplSFdiQlhlRDBxT3hYdklEV1ZxMjI0cmtSdkNnVTcz?=
 =?utf-8?B?bGo3eEtNMjY1MmlLL3BwQWR5WEpTbFE0cUE3bGRlNzUrWEtTZGNjQUVNa1d3?=
 =?utf-8?B?Ym83RStqQ1RrS3paamprQit1bkdaQWlvOXFqUnRCaFlXU0FPNWhPQVBwQjht?=
 =?utf-8?B?bVVsTFlBL2Q4aHlDQkxmQlhYU0lORHBNNXBTNGRZUDdUcEVoZmpldVJlZlFV?=
 =?utf-8?B?MGVLVEFSNUNCUVZ5TVQ2WGpJalBPNkUzSmZaS1lEZDB4c09lZ0VrTmhSSVI1?=
 =?utf-8?B?M3RCSjlpQlJ0WHp3YUdZZ2R3Qmk0bUtwV0diUVFCL3ZvSkpMV2kvMnhnMllH?=
 =?utf-8?B?Nm1mcjF2alplbVZuS1NDVkgyN1RSeVJvS1ZabEt5YmVabk9vSjdsVXNjOFlr?=
 =?utf-8?B?bWFiRjMxcXNKQ2JsZWRseHpmdktDMmY4bTY0U3V0VEpoSDAvODFlRCtiOVYw?=
 =?utf-8?B?NTVVRG9rMk5RSEx5TnZqZE1tZkI4K0dpNEFENWNVTndGTEpVc1VmMGZSMmlz?=
 =?utf-8?B?Nm42alNaYWh1cGtIbjBMTSs1d1NLbjc2ZmJTQUswWUppVVU1bHhJQnpSRzhR?=
 =?utf-8?B?di9najVraytKRE9xTHVNZHhnQVV5MXNXelFRSjhzVHlqNm94bFVZNFZ6WjJY?=
 =?utf-8?B?OFBOMnVrTXNNNGkvdEIycjNBVDM4MFRjWHorMUZFeDI0eEJvWE83RUxHc2NZ?=
 =?utf-8?B?Y1p6dFhhR1J2YlpVNjRwZzNwSXpzVWVsbENheHI5OWQ0YkFNaE9Wc0c2Yks1?=
 =?utf-8?B?Z0pSYjRtYzB6cFB4WmxWcXJSajdtVkFudHkzdVZ3NytyKytKNko3L2NzZUJ6?=
 =?utf-8?B?VnlTVmF2eG5VckVtbEtQcytOY3cwbDhIZVArY2Z1WnlYeDJ4aXk4MFdTdnU2?=
 =?utf-8?B?UC9SMzRxQkltYm5iMXk2dFR5UnNBWWFwKzI2VElUT1daQzY1K0ZBbUdzK05r?=
 =?utf-8?B?TWVtSGF1d0RCa2VvV0p1SU4zWW52NFNkRXF2eGg3RkcweEJmSDJhZmt0Wk1W?=
 =?utf-8?B?Uy8rVE5raFJzL05ESEFQc01KdHB5VEtjLy90VXJ6d3MwQXZCekZ1eFl2U1Q3?=
 =?utf-8?B?RE84QzVQanFrckNpUmNia2pCVVFWODA3UERIa0ROdkNHZDdoVEpIcmdlQzZ3?=
 =?utf-8?B?TU1hbE9JMVZPQmlPSXl2b3piaEY1bUROYkhoaHppRjdwdVVVWk9qelBEbG9Z?=
 =?utf-8?B?UkJVNmZabmx1RmdkMyszRVVkRmhnaURFdUJ5SVJONWtJMmhLTTlUWGFXNW9a?=
 =?utf-8?B?NWhGZkM5ZDc2a1JGZUtRQjEzVGFaZkRDSDdrdVJPT3lNM2xHZjQ4UVpxZ3FK?=
 =?utf-8?B?cmc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 9ea03c98-1af7-450a-6256-08dbfdf38f38
X-MS-Exchange-CrossTenant-AuthSource: SJ2PR11MB7573.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Dec 2023 04:57:52.6399
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2YEEwa7T5256CMMdIwRyoruZrhNMHjGuX+YCRkWAhVjLI+xl60gROXVDHsjDchEPCFQWEMgH5Nbm1Inq9WqFTApcfAFNy/SPsNvfdSflQo0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6147
X-OriginatorOrg: intel.com

Hi James,

On 12/15/2023 9:43 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_put_mon_l3_config() to cleanup any memory allocated
> by rdt_get_mon_l3_config().
> 
> There is no reason to backport this to a stable kernel.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Tested-by: Babu Moger <babu.moger@amd.com>
> Reviewed-by: Babu Moger <babu.moger@amd.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.
> 
> Changes since v7:
>  * Moved the eventual kfree() call to be after rdtgroup_exit()
> ---
>  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..b5d4b76f97c5 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -992,8 +992,14 @@ 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);
> +
>  	rdtgroup_exit();
> +
> +	if (r->mon_capable)
> +		rdt_put_mon_l3_config(r);
>  }
>  
>  __exitcall(resctrl_exit);
> diff --git a/arch/x86/kernel/cpu/resctrl/internal.h b/arch/x86/kernel/cpu/resctrl/internal.h
> index a4f1aa15f0a2..f68c6aecfa66 100644
> --- a/arch/x86/kernel/cpu/resctrl/internal.h
> +++ b/arch/x86/kernel/cpu/resctrl/internal.h
> @@ -546,6 +546,7 @@ void closid_free(int closid);
>  int alloc_rmid(void);
>  void free_rmid(u32 rmid);
>  int rdt_get_mon_l3_config(struct rdt_resource *r);
> +void __exit rdt_put_mon_l3_config(struct rdt_resource *r);
>  bool __init rdt_cpu_has(int flag);
>  void mon_event_count(void *info);
>  int rdtgroup_mondata_show(struct seq_file *m, void *arg);
> diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c b/arch/x86/kernel/cpu/resctrl/monitor.c
> index f136ac046851..5d9864919f1c 100644
> --- a/arch/x86/kernel/cpu/resctrl/monitor.c
> +++ b/arch/x86/kernel/cpu/resctrl/monitor.c
> @@ -741,6 +741,16 @@ static int dom_data_init(struct rdt_resource *r)
>  	return 0;
>  }
>  
> +static void __exit dom_data_exit(struct rdt_resource *r)
> +{
> +	mutex_lock(&rdtgroup_mutex);
> +
> +	kfree(rmid_ptrs);
> +	rmid_ptrs = NULL;
> +
> +	mutex_unlock(&rdtgroup_mutex);
> +}
> +
>  static struct mon_evt llc_occupancy_event = {
>  	.name		= "llc_occupancy",
>  	.evtid		= QOS_L3_OCCUP_EVENT_ID,
> @@ -830,6 +840,11 @@ int __init rdt_get_mon_l3_config(struct rdt_resource *r)
>  	return 0;
>  }
>  
> +void __exit rdt_put_mon_l3_config(struct rdt_resource *r)
> +{
> +	dom_data_exit(r);
> +}
> +

I expected the unused resource parameter to be removed because you said ([1]) it would.

>  void __init intel_rdt_mbm_apply_quirk(void)
>  {
>  	int cf_index;

Reinette

[1] https://lore.kernel.org/lkml/c4157536-7e57-0250-dc44-8914f5965ce6@arm.com/