[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c7e02935-ef77-4673-ab1c-6d07768bf02b@amd.com>
Date: Tue, 13 Jan 2026 16:34:59 -0600
From: "Cheatham, Benjamin" <benjamin.cheatham@....com>
To: Gregory Price <gourry@...rry.net>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<kernel-team@...a.com>, <dave@...olabs.net>, <jonathan.cameron@...wei.com>,
<dave.jiang@...el.com>, <alison.schofield@...el.com>,
<vishal.l.verma@...el.com>, <ira.weiny@...el.com>,
<dan.j.williams@...el.com>, David Hildenbrand <david@...nel.org>
Subject: Re: [PATCH 2/6] cxl: add sysram_region memory controller
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.
Powered by blists - more mailing lists