On Thu, 29 Jan 2026 16:04:35 -0500
Gregory Price <[email protected]> wrote:
> Enable dax kmem driver to select how to online the memory rather than
> implicitly depending on the system default. This will allow users of
> dax to plumb through a preferred auto-online policy for their region.
>
> Refactor and new interface:
> Add __add_memory_driver_managed() which accepts an explicit online_type
> and export mhp_get_default_online_type() so callers can pass it when
> they want the default behavior.
Hi Gregory,
I think maybe I'd have left the export for the first user outside of
memory_hotplug.c. Not particularly important however.
Maybe talk about why a caller of __add_memory_driver_managed() might want
the default? Feels like that's for the people who don't...
Or is this all a dance to avoid an
if (special mode)
__add_memory_driver_managed();
else
add_memory_driver_managed();
?
Other comments are mostly about using a named enum. I'm not sure
if there is some existing reason why that doesn't work? -Errno pushed through
this variable or anything like that?
>
> Refactor:
> Extract __add_memory_resource() to take an explicit online_type parameter,
> and update add_memory_resource() to pass the system default.
>
> No functional change for existing users.
>
> Cc: David Hildenbrand <[email protected]>
> Cc: Oscar Salvador <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Signed-off-by: Gregory Price <[email protected]>
> ---
> include/linux/memory_hotplug.h | 3 ++
> mm/memory_hotplug.c | 91 ++++++++++++++++++++++++----------
> 2 files changed, 67 insertions(+), 27 deletions(-)
>
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index f2f16cdd73ee..1eb63d1a247d 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -293,6 +293,9 @@ extern int __add_memory(int nid, u64 start, u64 size,
> mhp_t mhp_flags);
> extern int add_memory(int nid, u64 start, u64 size, mhp_t mhp_flags);
> extern int add_memory_resource(int nid, struct resource *resource,
> mhp_t mhp_flags);
> +int __add_memory_driver_managed(int nid, u64 start, u64 size,
> + const char *resource_name, mhp_t mhp_flags,
> + int online_type);
Given online_type values are from an enum anyway, maybe we can name that enum
and use
it explicitly?
> extern int add_memory_driver_managed(int nid, u64 start, u64 size,
> const char *resource_name,
> mhp_t mhp_flags);
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 87796b617d9e..d3ca95b872bd 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -239,6 +239,7 @@ int mhp_get_default_online_type(void)
>
> return mhp_default_online_type;
> }
> +EXPORT_SYMBOL_GPL(mhp_get_default_online_type);
>
> void mhp_set_default_online_type(int online_type)
> {
> @@ -1490,7 +1491,8 @@ static int create_altmaps_and_memory_blocks(int nid,
> struct memory_group *group,
> *
> * we are OK calling __meminit stuff here - we have CONFIG_MEMORY_HOTPLUG
> */
> -int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> +static int __add_memory_resource(int nid, struct resource *res, mhp_t
> mhp_flags,
> + int online_type)
> {
> struct mhp_params params = { .pgprot = pgprot_mhp(PAGE_KERNEL) };
> enum memblock_flags memblock_flags = MEMBLOCK_NONE;
> @@ -1580,12 +1582,9 @@ int add_memory_resource(int nid, struct resource *res,
> mhp_t mhp_flags)
> merge_system_ram_resource(res);
>
> /* online pages if requested */
> - if (mhp_get_default_online_type() != MMOP_OFFLINE) {
> - int online_type = mhp_get_default_online_type();
> -
> + if (online_type != MMOP_OFFLINE)
Ah. Fair enough, ignore comment in previous patch. I should have read on...
> walk_memory_blocks(start, size, &online_type,
> online_memory_block);
> - }
>
> return ret;
> error:
> @@ -1601,7 +1600,13 @@ int add_memory_resource(int nid, struct resource *res,
> mhp_t mhp_flags)
> return ret;
> }
>
> -/* requires device_hotplug_lock, see add_memory_resource() */
> +int add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
> +{
> + return __add_memory_resource(nid, res, mhp_flags,
> + mhp_get_default_online_type());
> +}
> +
> +/* requires device_hotplug_lock, see __add_memory_resource() */
> int __add_memory(int nid, u64 start, u64 size, mhp_t mhp_flags)
> {
> struct resource *res;
> @@ -1629,29 +1634,24 @@ int add_memory(int nid, u64 start, u64 size, mhp_t
> mhp_flags)
> }
> EXPORT_SYMBOL_GPL(add_memory);
>
> -/*
> - * Add special, driver-managed memory to the system as system RAM. Such
> - * memory is not exposed via the raw firmware-provided memmap as system
> - * RAM, instead, it is detected and added by a driver - during cold boot,
> - * after a reboot, and after kexec.
> - *
> - * Reasons why this memory should not be used for the initial memmap of a
> - * kexec kernel or for placing kexec images:
> - * - The booting kernel is in charge of determining how this memory will be
> - * used (e.g., use persistent memory as system RAM)
> - * - Coordination with a hypervisor is required before this memory
> - * can be used (e.g., inaccessible parts).
> +/**
> + * __add_memory_driver_managed - add driver-managed memory with explicit
> online_type
It's a little odd to add nice kernel-doc formatted documentation
when the non __ variant has free form docs. Maybe tidy that up first
if we want to go kernel-doc in this file? (I'm in favor, but no idea
on general feelings...)
> + * @nid: NUMA node ID where the memory will be added
> + * @start: Start physical address of the memory range
> + * @size: Size of the memory range in bytes
> + * @resource_name: Resource name in format "System RAM ($DRIVER)"
> + * @mhp_flags: Memory hotplug flags
> + * @online_type: Online behavior (MMOP_ONLINE, MMOP_ONLINE_KERNEL,
> + * MMOP_ONLINE_MOVABLE, or MMOP_OFFLINE)
Given that's currently the full set, seems like enum wins out here over
an int.
> *
> - * For this memory, no entries in /sys/firmware/memmap ("raw
> firmware-provided
> - * memory map") are created. Also, the created memory resource is flagged
> - * with IORESOURCE_SYSRAM_DRIVER_MANAGED, so in-kernel users can special-case
> - * this memory as well (esp., not place kexec images onto it).
> + * Add driver-managed memory with explicit online_type specification.
> + * The resource_name must have the format "System RAM ($DRIVER)".
> *
> - * The resource_name (visible via /proc/iomem) has to have the format
> - * "System RAM ($DRIVER)".
> + * Return: 0 on success, negative error code on failure.
> */
> -int add_memory_driver_managed(int nid, u64 start, u64 size,
> - const char *resource_name, mhp_t mhp_flags)
> +int __add_memory_driver_managed(int nid, u64 start, u64 size,
> + const char *resource_name, mhp_t mhp_flags,
> + int online_type)
> {
> struct resource *res;
> int rc;
> @@ -1661,6 +1661,9 @@ int add_memory_driver_managed(int nid, u64 start, u64
> size,
> resource_name[strlen(resource_name) - 1] != ')')
> return -EINVAL;
>
> + if (online_type < 0 || online_type > MMOP_ONLINE_MOVABLE)
This is where using an enum would help compiler know what is going on
and maybe warn if anyone writes something that isn't defined.
> + return -EINVAL;
> +
> lock_device_hotplug();
>
> res = register_memory_resource(start, size, resource_name);
> @@ -1669,7 +1672,7 @@ int add_memory_driver_managed(int nid, u64 start, u64
> size,
> goto out_unlock;
> }
>
> - rc = add_memory_resource(nid, res, mhp_flags);
> + rc = __add_memory_resource(nid, res, mhp_flags, online_type);
> if (rc < 0)
> release_memory_resource(res);
>
> @@ -1677,6 +1680,40 @@ int add_memory_driver_managed(int nid, u64 start, u64
> size,
> unlock_device_hotplug();
> return rc;
> }
> +EXPORT_SYMBOL_FOR_MODULES(__add_memory_driver_managed, "kmem");
> +
> +/*
> + * Add special, driver-managed memory to the system as system RAM. Such
> + * memory is not exposed via the raw firmware-provided memmap as system
> + * RAM, instead, it is detected and added by a driver - during cold boot,
> + * after a reboot, and after kexec.
> + *
> + * Reasons why this memory should not be used for the initial memmap of a
> + * kexec kernel or for placing kexec images:
> + * - The booting kernel is in charge of determining how this memory will be
> + * used (e.g., use persistent memory as system RAM)
> + * - Coordination with a hypervisor is required before this memory
> + * can be used (e.g., inaccessible parts).
> + *
> + * For this memory, no entries in /sys/firmware/memmap ("raw
> firmware-provided
> + * memory map") are created. Also, the created memory resource is flagged
> + * with IORESOURCE_SYSRAM_DRIVER_MANAGED, so in-kernel users can special-case
> + * this memory as well (esp., not place kexec images onto it).
> + *
> + * The resource_name (visible via /proc/iomem) has to have the format
> + * "System RAM ($DRIVER)".
> + *
> + * Memory will be onlined using the system default online type.
> + *
> + * Returns 0 on success, negative error code on failure.
> + */
> +int add_memory_driver_managed(int nid, u64 start, u64 size,
> + const char *resource_name, mhp_t mhp_flags)
> +{
> + return __add_memory_driver_managed(nid, start, size, resource_name,
> + mhp_flags,
> + mhp_get_default_online_type());
> +}
> EXPORT_SYMBOL_GPL(add_memory_driver_managed);
>
> /*
