Thread (30 messages) 30 messages, 4 authors, 2026-02-05

Re: [PATCH 2/9] mm/memory_hotplug: add __add_memory_driver_managed() with online_type arg

From: Gregory Price <gourry@gourry.net>
Date: 2026-02-02 18:02:14
Also in: linux-cxl, linux-fsdevel, linux-mm, lkml, nvdimm

On Mon, Feb 02, 2026 at 05:25:24PM +0000, Jonathan Cameron wrote:
On Thu, 29 Jan 2026 16:04:35 -0500
Gregory Price [off-list ref] wrote:
quoted
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.
quoted
+	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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help