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.
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 <david@kernel.org>
Cc: Oscar Salvador <osalvador@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Gregory Price <gourry@gourry.net>
---
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);
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)
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
+ * @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)
*
- * 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)
+ 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);
/*
--
2.52.0On Thu, 29 Jan 2026 16:04:35 -0500
Gregory Price <gourry@gourry.net> 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 <david@kernel.org>
> Cc: Oscar Salvador <osalvador@suse.de>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---
> 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);
>
> /*
On Mon, Feb 02, 2026 at 05:25:24PM +0000, Jonathan Cameron wrote: > On Thu, 29 Jan 2026 16:04:35 -0500 > Gregory Price <gourry@gourry.net> 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... > Less about why they want the default, more about maintaining backward compatibility. In the cxl driver, Ben pointed out something that made me realize we can change `region/bind()` to actually use the new `sysram/bind` path by just adding a one line `sysram_regionN->online_type = default()` I can add this detail to the changelog. > > 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? > I can add a cleanup-patch prior to use the enum, but i don't think this actually enables the compiler to do anything new at the moment? An enum just resolves to an int, and setting `enum thing val = -1` when the enum definition doesn't include -1 doesn't actually fire any errors (at least IIRC - maybe i'm just wrong). Same with function(enum) -> function(-1) wouldn't fire a compilation error It might actually be worth adding `MMOP_NOT_CONFIGURED = -1` so that the cxl-sysram driver can set this explicitly rather than just setting -1 as an implicit version of this - but then why would memory_hotplug.c ever want to expose a NOT_CONFIGURED option lol. So, yeah, the enum looks nicer, but not sure how much it buys us beyond that. > 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...) > ack. Can add some more cleanups early in the series. > > + 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. > I think you still have to sanity check this, but maybe the code looks cleaner, so will do. ~Gregory
On Mon, 2 Feb 2026 13:02:10 -0500 Gregory Price <gourry@gourry.net> wrote: > On Mon, Feb 02, 2026 at 05:25:24PM +0000, Jonathan Cameron wrote: > > On Thu, 29 Jan 2026 16:04:35 -0500 > > Gregory Price <gourry@gourry.net> 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... > > > > Less about why they want the default, more about maintaining backward > compatibility. > > In the cxl driver, Ben pointed out something that made me realize we can > change `region/bind()` to actually use the new `sysram/bind` path by > just adding a one line `sysram_regionN->online_type = default()` > > I can add this detail to the changelog. > > > > > 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? > > > > I can add a cleanup-patch prior to use the enum, but i don't think this > actually enables the compiler to do anything new at the moment? Good point. More coffee needed (or sleep) It lets sparse do some checking, but sadly only for wrong enum assignment. (Gcc has -Wenum-conversion as well which I think is effectively the same) I.e. you can't assign a value from a different enum without casting. It can't do anything if people just pass in an out of range int. > > An enum just resolves to an int, and setting `enum thing val = -1` when > the enum definition doesn't include -1 doesn't actually fire any errors > (at least IIRC - maybe i'm just wrong). Same with > > function(enum) -> function(-1) wouldn't fire a compilation error > > It might actually be worth adding `MMOP_NOT_CONFIGURED = -1` so that the > cxl-sysram driver can set this explicitly rather than just setting -1 > as an implicit version of this - but then why would memory_hotplug.c > ever want to expose a NOT_CONFIGURED option lol. > > So, yeah, the enum looks nicer, but not sure how much it buys us beyond > that. > > > 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...) > > > > ack. Can add some more cleanups early in the series. > > > > + 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. > > > > I think you still have to sanity check this, but maybe the code looks > cleaner, so will do. I'm in two minds about this. If it's an enum and someone writes an int I take take the view it's not our problem that they shot themselves in the foot. Maybe we should be paranoid... J > > ~Gregory >
On Mon, Feb 02, 2026 at 06:46:09PM +0000, Jonathan Cameron wrote:
> >
> > I can add a cleanup-patch prior to use the enum, but i don't think this
> > actually enables the compiler to do anything new at the moment?
>
> Good point. More coffee needed (or sleep)
>
> It lets sparse do some checking, but sadly only for wrong enum assignment.
> (Gcc has -Wenum-conversion as well which I think is effectively the same)
> I.e. you can't assign a value from a different enum without casting.
>
> It can't do anything if people just pass in an out of range int.
>
Which, after looking a bit... mm/memory_hotplug.c does this quite a bit
internally - except it uses a uint8_t
Example:
static int try_offline_memory_block(struct memory_block *mem, void *arg)
{
uint8_t online_type = MMOP_ONLINE_KERNEL;
uint8_t **online_types = arg;
... snip ...
}
int offline_and_remove_memory(u64 start, u64 size)
{
uint8_t *online_types, *tmp;
... snip ...
online_types = kmalloc_array(mb_count, sizeof(*online_types),
GFP_KERNEL);
}
So that's fun.
I'm not sure it's worth the churn here, but happy to do it if there are
strong opinions.
---
David do you have thoughts here?
~Gregory
On 2/2/26 22:37, Gregory Price wrote:
> On Mon, Feb 02, 2026 at 06:46:09PM +0000, Jonathan Cameron wrote:
>>>
>>> I can add a cleanup-patch prior to use the enum, but i don't think this
>>> actually enables the compiler to do anything new at the moment?
>>
>> Good point. More coffee needed (or sleep)
>>
>> It lets sparse do some checking, but sadly only for wrong enum assignment.
>> (Gcc has -Wenum-conversion as well which I think is effectively the same)
>> I.e. you can't assign a value from a different enum without casting.
>>
>> It can't do anything if people just pass in an out of range int.
>>
>
> Which, after looking a bit... mm/memory_hotplug.c does this quite a bit
> internally - except it uses a uint8_t
>
> Example:
>
> static int try_offline_memory_block(struct memory_block *mem, void *arg)
> {
> uint8_t online_type = MMOP_ONLINE_KERNEL;
> uint8_t **online_types = arg;
> ... snip ...
> }
>
> int offline_and_remove_memory(u64 start, u64 size)
> {
> uint8_t *online_types, *tmp;
> ... snip ...
> online_types = kmalloc_array(mb_count, sizeof(*online_types),
> GFP_KERNEL);
> }
>
> So that's fun.
>
> I'm not sure it's worth the churn here, but happy to do it if there are
> strong opinions.
>
> ---
>
> David do you have thoughts here?
I guess we should clean that all up where easily possible, but I don't
expect you to do that.
For online_types I used it, obviously, to save memory. So I'd expect it
to stay at least there, but cast it to the proper type once we take it
out the array.
--
Cheers,
David
On Wed, Feb 04, 2026 at 10:08:45PM +0100, David Hildenbrand (arm) wrote: > > > > David do you have thoughts here? > > I guess we should clean that all up where easily possible, but I don't > expect you to do that. > > For online_types I used it, obviously, to save memory. So I'd expect it to > stay at least there, but cast it to the proper type once we take it out the > array. > I can do it pretty easily and pull it out ahead. ~Gregory
© 2016 - 2026 Red Hat, Inc.