Re: [PATCH 2/6] cxl: add sysram_region memory controller
From: Gregory Price
Date: Mon Jan 12 2026 - 17:48:20 EST
On Mon, Jan 12, 2026 at 01:10:15PM -0800, dan.j.williams@xxxxxxxxx wrote:
> Gregory Price wrote:
> >
> > This can result in future management functions failing (such as adding a
> > new region). This is why "online_normal" is explicit, and the default
> > online zone is ZONE_MOVABLE.
>
> David's early feedback aligns with my own with respect to not creating
> new "online_*" ABI terms, but I want to go a step further.
>
> Part of the proposal here solves a fundamental problem with the way
> dax_kmem operates in terms of fixing the complication of dax_kmem
> depending on fine grained / multi-step online control via memblock
> sysfs.
>
> If we are going to introduce a new omnibus way to online entire regions
> at a time then that goodness should first come to dax_kmem and then
> potentially be refactored into a library that CXL can use to skip the
> device_dax indirection.
>
I think that probably just looks like sinking some of this into
memory_hotplug.c as bulk-commands and then exposing a similar
dax0.0/hotplug function that shows up if you're bound to dax_kmem.
That should be trivial to sink, can do.
> I.e. the end result would be this "hotplug" mechanism that fixes a long
> standing dax_kmem problem and then go further to drop the indirection
> through device_dax and have a "hotplug" mechanism directly at the
> cxl_region level.
>
The only catch may be auto-online behavior in dax, we may not want to
encode that there and instead improve the hotplug interface to entice
users to use it directly instead.
> > +int devm_cxl_add_sysram_region(struct cxl_region *cxlr)
> > +{
> [..]
> > +err_add_group:
> > + dev_set_drvdata(dev, NULL);
> > + /* if this fails, memory cannot be removed from the system until reboot */
> > + remove_memory(range.start, range_len(&range));
> > +err_add_memory:
> > + remove_resource(res);
> > + kfree(res);
> > +err_request_mem:
> > + memory_group_unregister(data->mgid);
> > +err_reg_mgid:
> > + kfree(data->res_name);
> > +err_res_name:
> > + kfree(data);
> > +err_data:
> > + clear_node_memory_type(numa_node, mtype);
> > + return rc;
>
> ...btw, this feels like too many new gotos in the age of
> scope-based-cleanup. It also feels like a bunch of duplicated code that
> CXL and fixed up dax_kmem can share.
Yeah i cribbed a bunch of this from dax and hotplug, i expect this to
get significantly cleaner in a version or two.
~Gregory