Re: [PATCH 2/8] mm/memory_hotplug: extract __add_memory_resource() and __offline_memory()

["David Hildenbrand (Red Hat)" <david@kernel.org>]

On 1/14/26 09:51, Gregory Price wrote:
> Extract internal helper functions with explicit parameters to prepare
> for adding new APIs that allow explicit online type control:
> 
>    - __add_memory_resource(): accepts an explicit online_type parameter.
>      Add MMOP_SYSTEM_DEFAULT as a new value that instructs the function
>      to use mhp_get_default_online_type() for the actual online type.
>      The existing add_memory_resource() becomes a thin wrapper that
>      passes MMOP_SYSTEM_DEFAULT to preserve existing behavior.
> 
>    - __offline_memory(): extracted from offline_and_remove_memory() to
>      handle the offline operation with rollback support. The caller
>      now handles locking and the remove step separately.


I don't understand why this change is even part of this patch, can you 
elaborate? You don't add any "explicit parameters to prepare for adding 
new APIs that allow explicit online type control" there.

So likely you squeezed two independent things into a single patch? :)

Likely you should pair the __add_memory_resource() change with the 
add_memory_driver_managed() changed and vice versa.

> 
> This refactoring enables future callers to specify explicit online
> types (MMOP_OFFLINE, MMOP_ONLINE, MMOP_ONLINE_MOVABLE) or use
> MMOP_SYSTEM_DEFAULT for the system default policy. The offline logic
> can also be used independently of the remove step.
> 
> Mild functional change: if try_remove_memory() failed after successfully
> offlining, we would re-online the memory.  We no longer do this, and in
> practice removal doesn't fail if offline succeeds.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---
>   include/linux/memory_hotplug.h |  2 +
>   mm/memory_hotplug.c            | 69 ++++++++++++++++++++++------------
>   2 files changed, 48 insertions(+), 23 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index f2f16cdd73ee..d5407264d72a 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -29,6 +29,8 @@ enum {
>   	MMOP_ONLINE_KERNEL,
>   	/* Online the memory to ZONE_MOVABLE. */
>   	MMOP_ONLINE_MOVABLE,
> +	/* Use system default online type from mhp_get_default_online_type(). */
> +	MMOP_SYSTEM_DEFAULT,

I don't like having fake options as part of this interface.

Why can't we let selected users use mhp_get_default_online_type() 
instead? Like add_memory_resource(). We can export that function.


-- 
Cheers

David