From: Gregory Price <gourry@gourry.net>
To: dan.j.williams@intel.com
Cc: linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@meta.com, dave@stgolabs.net,
jonathan.cameron@huawei.com, dave.jiang@intel.com,
alison.schofield@intel.com, vishal.l.verma@intel.com,
ira.weiny@intel.com, David Hildenbrand <david@kernel.org>
Subject: Re: [PATCH 2/6] cxl: add sysram_region memory controller
Date: Mon, 12 Jan 2026 17:47:45 -0500 [thread overview]
Message-ID: <aWV6ETg1Km_AVFsG@gourry-fedora-PF4VCD3F> (raw)
In-Reply-To: <69656337337f9_2071810063@dwillia2-mobl4.notmuch>
On Mon, Jan 12, 2026 at 01:10:15PM -0800, dan.j.williams@intel.com 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
next prev parent reply other threads:[~2026-01-12 22:48 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <CGME20260113093758epcas5p10cc9749a657b8e4d32db75b8b973b67d@epcas5p1.samsung.com>
2026-01-12 16:35 ` [PATCH 0/6] CXL: Introduce memory controller abstraction and sysram controller Gregory Price
2026-01-12 16:35 ` [PATCH 1/6] drivers/cxl: add cxl_memctrl_mode and region->memctrl Gregory Price
2026-01-12 20:59 ` dan.j.williams
2026-01-12 22:25 ` Gregory Price
2026-01-13 18:00 ` Dave Jiang
2026-01-13 20:07 ` Gregory Price
2026-01-14 16:36 ` dan.j.williams
2026-01-12 21:10 ` Cheatham, Benjamin
2026-01-12 22:34 ` Gregory Price
2026-01-14 17:18 ` Jonathan Cameron
2026-01-14 18:25 ` Gregory Price
2026-01-14 18:36 ` Jonathan Cameron
2026-01-12 16:35 ` [PATCH 2/6] cxl: add sysram_region memory controller Gregory Price
2026-01-12 20:00 ` David Hildenbrand (Red Hat)
2026-01-12 22:43 ` Gregory Price
2026-01-12 21:10 ` dan.j.williams
2026-01-12 22:47 ` Gregory Price [this message]
2026-01-12 21:10 ` Cheatham, Benjamin
2026-01-12 22:55 ` Gregory Price
2026-01-13 22:34 ` Cheatham, Benjamin
2026-01-12 16:35 ` [PATCH 3/6] cxl/core/region: move pmem memctrl logic into memctrl/pmem_region Gregory Price
2026-01-12 21:10 ` Cheatham, Benjamin
2026-01-12 22:58 ` Gregory Price
2026-01-13 9:12 ` Neeraj Kumar
2026-01-12 16:35 ` [PATCH 4/6] cxl: add CONFIG_CXL_REGION_CTRL_AUTO_* build config options Gregory Price
2026-01-12 21:10 ` Cheatham, Benjamin
2026-01-12 23:05 ` Gregory Price
2026-01-13 4:31 ` dan.j.williams
2026-01-13 13:55 ` Gregory Price
2026-01-12 16:35 ` [PATCH 5/6] cxl: add CXL_REGION_SYSRAM_DEFAULT_* build options Gregory Price
2026-01-12 21:11 ` Cheatham, Benjamin
2026-01-12 23:07 ` Gregory Price
2026-01-12 16:35 ` [PATCH 6/6] cxl/sysram: disallow onlining in ZONE_NORMAL if state is movable only Gregory Price
2026-01-12 21:11 ` Cheatham, Benjamin
2026-01-12 23:14 ` Gregory Price
2026-01-13 22:35 ` Cheatham, Benjamin
2026-01-13 9:37 ` [PATCH 0/6] CXL: Introduce memory controller abstraction and sysram controller Neeraj Kumar
2026-01-13 13:33 ` Gregory Price
2026-01-15 18:43 ` Alejandro Lucero Palau
2026-01-15 18:56 ` Gregory Price
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aWV6ETg1Km_AVFsG@gourry-fedora-PF4VCD3F \
--to=gourry@gourry.net \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=david@kernel.org \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=kernel-team@meta.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.