Re: [PATCH] x86/amd/node: Release reserved config regions on init error

[Mario Limonciello]

On 9/15/26 11:22, yolezz wrote:
> In amd_smn_init(), if pci_request_config_region_exclusive() fails or
> if kzalloc_objs() fails to allocate memory for amd_roots, the already
> reserved PCI config regions are left allocated.
>
> Add amd_smn_release_config_regions() to clean up and release all
> reserved PCI config space regions before returning an error code.
>
> Signed-off-by: yolezz <yolezz.secret@xxxxxxxxx>
> ---
>   arch/x86/kernel/amd_node.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/amd_node.c b/arch/x86/kernel/amd_node.c
> index b7926ba3610a..6a96d888b5a7 100644
> --- a/arch/x86/kernel/amd_node.c
> +++ b/arch/x86/kernel/amd_node.c
> @@ -239,6 +239,15 @@ static struct pci_dev *get_next_root(struct pci_dev *root)
>   	return root;
>   }
>   
> +static void amd_smn_release_config_regions(u16 num_roots)
> +{
> +	struct pci_dev *root __free(pci_dev_put) = NULL;
> +
> +	/* Release the PCI config space for each root device. */
> +	while (num_roots-- && (root = get_next_root(root)))
> +		pci_release_config_region(root, 0, PCI_CFG_SPACE_SIZE);
> +}
> +
>   static bool enable_dfs;
>   
>   static int __init amd_smn_enable_dfs(char *str)
> @@ -273,6 +282,7 @@ static int __init amd_smn_init(void)
>   		 */
>   		if (!pci_request_config_region_exclusive(root, 0, PCI_CFG_SPACE_SIZE, NULL)) {
>   			pci_err(root, "Failed to reserve config space\n");
> +			amd_smn_release_config_regions(num_roots);
>   			return -EEXIST;
>   		}
>   
> @@ -286,8 +296,10 @@ static int __init amd_smn_init(void)
>   
>   	num_nodes = amd_num_nodes();
>   	amd_roots = kzalloc_objs(*amd_roots, num_nodes);
> -	if (!amd_roots)
> +	if (!amd_roots) {
> +		amd_smn_release_config_regions(num_roots);
>   		return -ENOMEM;
> +	}
>   
>   	roots_per_node = num_roots / num_nodes;
>   	if (!roots_per_node) {

Given this is a cleanup on failure exit path, would it be better to use 
a __free() helper perhaps?  That would mean just needing to set it to 
NULL on the success path.