All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <djbw@kernel.org>
To: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: linux-cxl@vger.kernel.org,  linux-kernel@vger.kernel.org,
	 kernel-team@meta.com,  dave@stgolabs.net,  dave.jiang@intel.com,
	 alison.schofield@intel.com,  vishal.l.verma@intel.com,
	 ira.weiny@intel.com
Subject: Re: [PATCH v4 1/3] cxl/core/region: move pmem region driver logic into region_pmem.c
Date: Sat, 11 Apr 2026 13:34:00 -0700	[thread overview]
Message-ID: <69dab038caf54_46de10051@djbw-dev.notmuch> (raw)
In-Reply-To: <20260323150847.00002e6a@huawei.com>

Jonathan Cameron wrote:
> On Sun, 22 Mar 2026 09:16:36 -0400
> Gregory Price <gourry@gourry.net> wrote:
> 
> > core/region.c is overloaded with per-region control logic (pmem, dax,
> > sysram, etc). Move the pmem region driver logic from region.c into
> > region_pmem.c make it clear that this code only applies to pmem regions.
> > 
> > No functional changes.
> > 
> > Signed-off-by: Gregory Price <gourry@gourry.net>
[..] 
> > +static struct lock_class_key cxl_pmem_region_key;
> > +
> 
> > +
> > +/**
> > + * devm_cxl_add_pmem_region() - add a cxl_region-to-nd_region bridge
> > + * @cxlr: parent CXL region for this pmem region bridge device
> > + *
> > + * Return: 0 on success negative error code on failure.
> 
> FWIW the lifetime management in here feels way to complex to me.  Not
> a problem for this patch and I'm not immediately sure what we can do about it.
> 
[..]
> > +	scoped_guard(device, &cxl_nvb->dev) {
> > +		if (cxl_nvb->dev.driver)
> > +			rc = devm_add_action_or_reset(&cxl_nvb->dev,
> > +					cxlr_pmem_unregister,
> > +					cxlr_pmem);
> > +		else
> > +			rc = -ENXIO;
> As an example. If we happen to take this path... Where is the device_add() undone?

Yes, but thankfully this a leftover from "cb9cfff82f6a cxl/acpi:
Simplify cxl_nvdimm_bridge probing" where the "!cxl_nvb->dev.driver"
case was obviated. It should not be the case that this can end up with a
disabled "nvdimm-bridge".

Still more complicated than I would like. I believe that this should be
able to rely on the same base unwind logic as other regions and memdevs.
I.e. region comes down on either CXL-root removal or any member memdev
removal. Just not clear that this reduces complexity vs just refactors
it out of devm_cxl_add_pmem_region().

  parent reply	other threads:[~2026-04-11 20:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-22 13:16 [PATCH v4 0/3] pull region-specific logic into new files Gregory Price
2026-03-22 13:16 ` [PATCH v4 1/3] cxl/core/region: move pmem region driver logic into region_pmem.c Gregory Price
2026-03-23  2:37   ` Alison Schofield
2026-03-23  5:37     ` Gregory Price
2026-03-24  3:49       ` Alison Schofield
2026-03-24 15:11         ` Gregory Price
2026-03-23 15:08   ` Jonathan Cameron
2026-03-23 15:47     ` Gregory Price
2026-03-23 17:58       ` Jonathan Cameron
2026-04-11 20:34     ` Dan Williams [this message]
2026-03-25  1:31   ` Alison Schofield
2026-03-22 13:16 ` [PATCH v4 2/3] cxl/core/region: move dax region device logic into region_dax.c Gregory Price
2026-03-23 15:11   ` Jonathan Cameron
2026-03-25  1:31   ` Alison Schofield
2026-03-22 13:16 ` [PATCH v4 3/3] cxl/core: use cleanup.h for devm_cxl_add_dax_region Gregory Price
2026-03-23 12:17   ` Jonathan Cameron
2026-03-25  1:33   ` Alison Schofield
2026-03-26 16:28 ` [PATCH v4 0/3] pull region-specific logic into new files Ira Weiny
2026-03-26 16:34   ` 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=69dab038caf54_46de10051@djbw-dev.notmuch \
    --to=djbw@kernel.org \
    --cc=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.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 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.