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