Re: [PATCH] mm/memory_hotplug: Factor out altmap freeing checks

[Donet Tom <donettom@linux.ibm.com>]

On 5/11/26 2:13 PM, Muchun Song wrote:
> Use a small helper to centralize altmap freeing after verifying that all
> vmemmap pages were released. This keeps the check consistent between the
> normal teardown path and the memory hotplug error paths.
>
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Muchun Song <songmuchun@bytedance.com>


looks good to me.

Reviewed-by: Donet Tom <donettom@linux.ibm.com>

-Donet

> ---
>   mm/memory_hotplug.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 462d8dcd636d..af5489f03771 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1403,6 +1403,12 @@ bool mhp_supports_memmap_on_memory(void)
>   }
>   EXPORT_SYMBOL_GPL(mhp_supports_memmap_on_memory);
>   
> +static void altmap_free(struct vmem_altmap *altmap)
> +{
> +	WARN_ONCE(altmap->alloc, "Altmap not fully unmapped");
> +	kfree(altmap);
> +}
> +
>   static void remove_memory_blocks_and_altmaps(u64 start, u64 size)
>   {
>   	unsigned long memblock_size = memory_block_size_bytes();
> @@ -1427,12 +1433,8 @@ static void remove_memory_blocks_and_altmaps(u64 start, u64 size)
>   		put_device(&mem->dev);
>   
>   		remove_memory_block_devices(cur_start, memblock_size);
> -
>   		arch_remove_memory(cur_start, memblock_size, altmap, NULL);
> -
> -		/* Verify that all vmemmap pages have actually been freed. */
> -		WARN(altmap->alloc, "Altmap not fully unmapped");
> -		kfree(altmap);
> +		altmap_free(altmap);
>   	}
>   }
>   
> @@ -1463,7 +1465,7 @@ static int create_altmaps_and_memory_blocks(int nid, struct memory_group *group,
>   		/* call arch's memory hotadd */
>   		ret = arch_add_memory(nid, cur_start, memblock_size, &params);
>   		if (ret < 0) {
> -			kfree(params.altmap);
> +			altmap_free(params.altmap);
>   			goto out;
>   		}
>   
> @@ -1472,7 +1474,7 @@ static int create_altmaps_and_memory_blocks(int nid, struct memory_group *group,
>   						  params.altmap, group);
>   		if (ret) {
>   			arch_remove_memory(cur_start, memblock_size, params.altmap, NULL);
> -			kfree(params.altmap);
> +			altmap_free(params.altmap);
>   			goto out;
>   		}
>   	}
>
> base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83