Re: [PATCH 2/9] mm/memory_hotplug: add __add_memory_driver_managed() with online_type arg
From: Gregory Price
Date: Mon Feb 02 2026 - 13:03:17 EST
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@xxxxxxxxxx> 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