Linux CXL
 help / color / mirror / Atom feed
From: "Cheatham, Benjamin" <benjamin.cheatham@amd.com>
To: Gregory Price <gourry@gourry.net>
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>, <dan.j.williams@intel.com>,
	David Hildenbrand <david@kernel.org>
Subject: Re: [PATCH 2/6] cxl: add sysram_region memory controller
Date: Tue, 13 Jan 2026 16:34:59 -0600	[thread overview]
Message-ID: <c7e02935-ef77-4673-ab1c-6d07768bf02b@amd.com> (raw)
In-Reply-To: <aWV78x2NZd0-iNSv@gourry-fedora-PF4VCD3F>

On 1/12/2026 4:55 PM, Gregory Price wrote:
> On Mon, Jan 12, 2026 at 03:10:41PM -0600, Cheatham, Benjamin wrote:
>> On 1/12/2026 10:35 AM, Gregory Price wrote:
>>> Add a sysram memctrl that directly hotplugs memory without needing to
>>> route through DAX.  This simplifies the sysram usecase considerably.
>>>
>>> The sysram memctl adds new sysfs controls when registered:
>>> 	region/memctrl/[hotplug, hotunplug, state]
>>>
>>> hotplug:   controller attempts to hotplug the memory region
>>> hotunplug: controller attempts to offline and hotunplug the memory region
>>
>> Nit: Would it be better to use hotadd/hotremove here instead of hotplug/hotunplug? The terms
>> are basically synonymous, but I think hotadd and hotremove are more descriptive.
> 
> I will defer to David on this.  I think keeping the terminology
> consistent is better, but also hotplug is overloaded between physical
> and logical.  It ultimately means the same thing to be honest.

I agree, I'm fine with either here.

> 
>>> state:     [online,online_normal,offline]
>>>    online       : controller onlines blocks in ZONE_MOVABLE
>>>    online_normal: controller onlines blocks in ZONE_NORMAL
>>
>> The naming for online states could be improved imo. I understand and agree with the motivation
>> behind the names, but I could see the use of the word "normal" being confusing to less savvy users.
>> You could change it to include the zone for both (online_movable/online_normal), but I think it may
>> be easier to mark which one has drawbacks, i.e. change "online_normal" to something like "online_nonremovable".
>> That way, anyone who doesn't want to go find the documentation for these can understand the user-visible
>> impact.
>>
>> In any case, all of these attributes need ABI documentation as well.
>>
> 
> This is what i was getting at originally, I will consider the other
> feedback and spin a v2 with this simplified a bit.
> 
> I'm leaning towards agreeing with Dan and David that probably we just
> keep online/online_movable since it's consistent with base/memory.c, but
> we can continue to have this argument.
> 
> I don't think we can reasonable get away from users of this interface
> understanding the implications of ZONEs, since whatever they choose to
> do dictates what zone the memory gets added to.

That sounds reasonable. I was going under the assumption that someone may come
along who doesn't know much about zones, which probably isn't very likely. So
if we want to ditch that assumption it's fine by me.


  reply	other threads:[~2026-01-13 22:35 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
2026-01-12 21:10     ` Cheatham, Benjamin
2026-01-12 22:55       ` Gregory Price
2026-01-13 22:34         ` Cheatham, Benjamin [this message]
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=c7e02935-ef77-4673-ab1c-6d07768bf02b@amd.com \
    --to=benjamin.cheatham@amd.com \
    --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=gourry@gourry.net \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox