Both detach_target() and cxld_unregister() want to tear down a cxl_region
when an endpoint decoder is either detached or destroyed.
When a region is to be destroyed cxl_decoder_detach() releases
cxl_region_rwsem unbinds the cxl_region driver and re-acquires the rwsem.
This "reverse" locking pattern is difficult to reason about, not amenable
to scope-based cleanup, and the minor differences in the calling convention
of cxl_decoder_detach() currently results in the cxl_decoder_kill_region()
wrapper.
Introduce CLASS(cxl_decoder_detach...) which creates an object that moves
the post-detach cleanup work to a destructor, and consolidates minor
preamble differences in the constructor.
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Jonathan Cameron <jonathan.cameron@huawei.com>
Cc: Dave Jiang <dave.jiang@intel.com>
Cc: Alison Schofield <alison.schofield@intel.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/cxl/core/core.h | 43 ++++++++++++++++++-
drivers/cxl/core/port.c | 6 +--
drivers/cxl/core/region.c | 88 ++++++++++++++++++---------------------
3 files changed, 83 insertions(+), 54 deletions(-)
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 17b692eb3257..44b09552f44e 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -12,6 +12,11 @@ extern const struct device_type cxl_pmu_type;
extern struct attribute_group cxl_base_attribute_group;
+enum cxl_detach_mode {
+ DETACH_ONLY,
+ DETACH_INVALIDATE,
+};
+
#ifdef CONFIG_CXL_REGION
extern struct device_attribute dev_attr_create_pmem_region;
extern struct device_attribute dev_attr_create_ram_region;
@@ -20,7 +25,11 @@ extern struct device_attribute dev_attr_region;
extern const struct device_type cxl_pmem_region_type;
extern const struct device_type cxl_dax_region_type;
extern const struct device_type cxl_region_type;
-void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled);
+
+struct cxl_region *cxl_decoder_detach(struct cxl_region *cxlr,
+ struct cxl_endpoint_decoder *cxled,
+ int pos, enum cxl_detach_mode mode);
+
#define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)
#define CXL_REGION_TYPE(x) (&cxl_region_type)
#define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
@@ -48,7 +57,9 @@ static inline int cxl_get_poison_by_endpoint(struct cxl_port *port)
{
return 0;
}
-static inline void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
+static inline struct cxl_region *
+cxl_decoder_detach(struct cxl_region *cxlr, struct cxl_endpoint_decoder *cxled,
+ int pos, enum cxl_detach_mode mode)
{
}
static inline int cxl_region_init(void)
@@ -99,6 +110,34 @@ u16 cxl_rcrb_to_aer(struct device *dev, resource_size_t rcrb);
extern struct rw_semaphore cxl_dpa_rwsem;
extern struct rw_semaphore cxl_region_rwsem;
+DEFINE_CLASS(
+ cxl_decoder_detach, struct cxl_region *,
+ if (!IS_ERR_OR_NULL(_T)) {
+ device_release_driver(&_T->dev);
+ put_device(&_T->dev);
+ },
+ ({
+ int rc = 0;
+
+ /* when the decoder is being destroyed lock unconditionally */
+ if (mode == DETACH_INVALIDATE)
+ down_write(&cxl_region_rwsem);
+ else
+ rc = down_write_killable(&cxl_region_rwsem);
+
+ if (rc)
+ cxlr = ERR_PTR(rc);
+ else {
+ cxlr = cxl_decoder_detach(cxlr, cxled, pos, mode);
+ get_device(&cxlr->dev);
+ }
+ up_write(&cxl_region_rwsem);
+
+ cxlr;
+ }),
+ struct cxl_region *cxlr, struct cxl_endpoint_decoder *cxled, int pos,
+ enum cxl_detach_mode mode)
+
int cxl_memdev_init(void);
void cxl_memdev_exit(void);
void cxl_mbox_init(void);
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 726bd4a7de27..20b65f13bded 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -2008,11 +2008,9 @@ EXPORT_SYMBOL_NS_GPL(cxl_decoder_add, "CXL");
static void cxld_unregister(void *dev)
{
- struct cxl_endpoint_decoder *cxled;
-
if (is_endpoint_decoder(dev)) {
- cxled = to_cxl_endpoint_decoder(dev);
- cxl_decoder_kill_region(cxled);
+ CLASS(cxl_decoder_detach, cxlr)
+ (NULL, to_cxl_endpoint_decoder(dev), -1, DETACH_INVALIDATE);
}
device_unregister(dev);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 11448824ddd4..17e69f6cc772 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2122,27 +2122,52 @@ static int cxl_region_attach(struct cxl_region *cxlr,
return 0;
}
-static int cxl_region_detach(struct cxl_endpoint_decoder *cxled)
+/*
+ * Cleanup a decoder's interest in a region. There are 2 cases to
+ * handle, removing an unknown @cxled from a known position in a region
+ * (detach_target()) or removing a known @cxled from an unknown @cxlr
+ * (cxld_unregister())
+ *
+ * When the detachment finds a region, the caller is responsible for
+ * releasing the region driver.
+ */
+struct cxl_region *cxl_decoder_detach(struct cxl_region *cxlr,
+ struct cxl_endpoint_decoder *cxled,
+ int pos, enum cxl_detach_mode mode)
{
- struct cxl_port *iter, *ep_port = cxled_to_port(cxled);
- struct cxl_region *cxlr = cxled->cxld.region;
struct cxl_region_params *p;
- int rc = 0;
lockdep_assert_held_write(&cxl_region_rwsem);
- if (!cxlr)
- return 0;
+ if (!cxled) {
+ p = &cxlr->params;
+
+ if (pos >= p->interleave_ways) {
+ dev_dbg(&cxlr->dev, "position %d out of range %d\n",
+ pos, p->interleave_ways);
+ return ERR_PTR(-ENXIO);
+ }
+
+ if (!p->targets[pos])
+ return NULL;
+ cxled = p->targets[pos];
+ } else {
+ cxlr = cxled->cxld.region;
+ if (!cxlr)
+ return NULL;
+ p = &cxlr->params;
+ }
- p = &cxlr->params;
- get_device(&cxlr->dev);
+
+ if (mode == DETACH_INVALIDATE)
+ cxled->part = -1;
if (p->state > CXL_CONFIG_ACTIVE) {
cxl_region_decode_reset(cxlr, p->interleave_ways);
p->state = CXL_CONFIG_ACTIVE;
}
- for (iter = ep_port; !is_cxl_root(iter);
+ for (struct cxl_port *iter = cxled_to_port(cxled); !is_cxl_root(iter);
iter = to_cxl_port(iter->dev.parent))
cxl_port_detach_region(iter, cxlr, cxled);
@@ -2153,7 +2178,7 @@ static int cxl_region_detach(struct cxl_endpoint_decoder *cxled)
dev_WARN_ONCE(&cxlr->dev, 1, "expected %s:%s at position %d\n",
dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
cxled->pos);
- goto out;
+ return NULL;
}
if (p->state == CXL_CONFIG_ACTIVE) {
@@ -2167,21 +2192,7 @@ static int cxl_region_detach(struct cxl_endpoint_decoder *cxled)
.end = -1,
};
- /* notify the region driver that one of its targets has departed */
- up_write(&cxl_region_rwsem);
- device_release_driver(&cxlr->dev);
- down_write(&cxl_region_rwsem);
-out:
- put_device(&cxlr->dev);
- return rc;
-}
-
-void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled)
-{
- down_write(&cxl_region_rwsem);
- cxled->part = -1;
- cxl_region_detach(cxled);
- up_write(&cxl_region_rwsem);
+ return cxlr;
}
static int attach_target(struct cxl_region *cxlr,
@@ -2206,29 +2217,10 @@ static int attach_target(struct cxl_region *cxlr,
static int detach_target(struct cxl_region *cxlr, int pos)
{
- struct cxl_region_params *p = &cxlr->params;
- int rc;
-
- rc = down_write_killable(&cxl_region_rwsem);
- if (rc)
- return rc;
-
- if (pos >= p->interleave_ways) {
- dev_dbg(&cxlr->dev, "position %d out of range %d\n", pos,
- p->interleave_ways);
- rc = -ENXIO;
- goto out;
- }
-
- if (!p->targets[pos]) {
- rc = 0;
- goto out;
- }
-
- rc = cxl_region_detach(p->targets[pos]);
-out:
- up_write(&cxl_region_rwsem);
- return rc;
+ CLASS(cxl_decoder_detach, ret)(cxlr, NULL, pos, DETACH_ONLY);
+ if (IS_ERR(ret))
+ return PTR_ERR(ret);
+ return 0;
}
static size_t store_targetN(struct cxl_region *cxlr, const char *buf, int pos,
--
2.49.0Hi Dan,
kernel test robot noticed the following build warnings:
[auto build test WARNING on b4432656b36e5cc1d50a1f2dc15357543add530e]
url: https://github.com/intel-lab-lkp/linux/commits/Dan-Williams/cleanup-Introduce-DEFINE_ACQUIRE-a-CLASS-for-conditional-locking/20250507-152728
base: b4432656b36e5cc1d50a1f2dc15357543add530e
patch link: https://lore.kernel.org/r/20250507072145.3614298-7-dan.j.williams%40intel.com
patch subject: [PATCH 6/7] cxl/region: Introduce CLASS(cxl_decoder_detach...) consolidate multiple paths
config: um-allmodconfig (https://download.01.org/0day-ci/archive/20250508/202505081548.9HldF62Y-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505081548.9HldF62Y-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505081548.9HldF62Y-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/cxl/core/port.c:9:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:549:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
549 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:567:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
567 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/cxl/core/port.c:9:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:585:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/cxl/core/port.c:9:
In file included from include/linux/pci.h:38:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:12:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:601:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
601 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:616:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
616 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:631:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
631 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:724:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
724 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:737:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
737 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:750:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
750 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:764:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
764 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:778:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
778 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:792:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
792 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from drivers/cxl/core/port.c:17:
>> drivers/cxl/core/core.h:64:1: warning: non-void function does not return a value [-Wreturn-type]
64 | }
| ^
13 warnings generated.
vim +64 drivers/cxl/core/core.h
79ed8367834ee8 Dan Williams 2025-05-07 28
79ed8367834ee8 Dan Williams 2025-05-07 29 struct cxl_region *cxl_decoder_detach(struct cxl_region *cxlr,
79ed8367834ee8 Dan Williams 2025-05-07 30 struct cxl_endpoint_decoder *cxled,
79ed8367834ee8 Dan Williams 2025-05-07 31 int pos, enum cxl_detach_mode mode);
79ed8367834ee8 Dan Williams 2025-05-07 32
779dd20cfb56c5 Ben Widawsky 2021-06-08 33 #define CXL_REGION_ATTR(x) (&dev_attr_##x.attr)
8d48817df6ac20 Dan Williams 2021-06-15 34 #define CXL_REGION_TYPE(x) (&cxl_region_type)
779dd20cfb56c5 Ben Widawsky 2021-06-08 35 #define SET_CXL_REGION_ATTR(x) (&dev_attr_##x.attr),
04ad63f086d1a9 Dan Williams 2022-01-11 36 #define CXL_PMEM_REGION_TYPE(x) (&cxl_pmem_region_type)
09d09e04d2fcf8 Dan Williams 2023-02-10 37 #define CXL_DAX_REGION_TYPE(x) (&cxl_dax_region_type)
8d48817df6ac20 Dan Williams 2021-06-15 38 int cxl_region_init(void);
8d48817df6ac20 Dan Williams 2021-06-15 39 void cxl_region_exit(void);
f0832a58639691 Alison Schofield 2023-04-18 40 int cxl_get_poison_by_endpoint(struct cxl_port *port);
b98d042698a325 Alison Schofield 2024-04-30 41 struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
9aa5f6235e16ac Alison Schofield 2024-07-02 42 u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
86954ff5032d9d Alison Schofield 2024-04-30 43 u64 dpa);
b98d042698a325 Alison Schofield 2024-04-30 44
779dd20cfb56c5 Ben Widawsky 2021-06-08 45 #else
9aa5f6235e16ac Alison Schofield 2024-07-02 46 static inline u64 cxl_dpa_to_hpa(struct cxl_region *cxlr,
9aa5f6235e16ac Alison Schofield 2024-07-02 47 const struct cxl_memdev *cxlmd, u64 dpa)
86954ff5032d9d Alison Schofield 2024-04-30 48 {
86954ff5032d9d Alison Schofield 2024-04-30 49 return ULLONG_MAX;
86954ff5032d9d Alison Schofield 2024-04-30 50 }
b98d042698a325 Alison Schofield 2024-04-30 51 static inline
b98d042698a325 Alison Schofield 2024-04-30 52 struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
b98d042698a325 Alison Schofield 2024-04-30 53 {
b98d042698a325 Alison Schofield 2024-04-30 54 return NULL;
b98d042698a325 Alison Schofield 2024-04-30 55 }
f0832a58639691 Alison Schofield 2023-04-18 56 static inline int cxl_get_poison_by_endpoint(struct cxl_port *port)
f0832a58639691 Alison Schofield 2023-04-18 57 {
f0832a58639691 Alison Schofield 2023-04-18 58 return 0;
f0832a58639691 Alison Schofield 2023-04-18 59 }
79ed8367834ee8 Dan Williams 2025-05-07 60 static inline struct cxl_region *
79ed8367834ee8 Dan Williams 2025-05-07 61 cxl_decoder_detach(struct cxl_region *cxlr, struct cxl_endpoint_decoder *cxled,
79ed8367834ee8 Dan Williams 2025-05-07 62 int pos, enum cxl_detach_mode mode)
b9686e8c8e39d4 Dan Williams 2022-06-04 63 {
b9686e8c8e39d4 Dan Williams 2022-06-04 @64 }
8d48817df6ac20 Dan Williams 2021-06-15 65 static inline int cxl_region_init(void)
8d48817df6ac20 Dan Williams 2021-06-15 66 {
8d48817df6ac20 Dan Williams 2021-06-15 67 return 0;
8d48817df6ac20 Dan Williams 2021-06-15 68 }
8d48817df6ac20 Dan Williams 2021-06-15 69 static inline void cxl_region_exit(void)
8d48817df6ac20 Dan Williams 2021-06-15 70 {
8d48817df6ac20 Dan Williams 2021-06-15 71 }
779dd20cfb56c5 Ben Widawsky 2021-06-08 72 #define CXL_REGION_ATTR(x) NULL
8d48817df6ac20 Dan Williams 2021-06-15 73 #define CXL_REGION_TYPE(x) NULL
779dd20cfb56c5 Ben Widawsky 2021-06-08 74 #define SET_CXL_REGION_ATTR(x)
04ad63f086d1a9 Dan Williams 2022-01-11 75 #define CXL_PMEM_REGION_TYPE(x) NULL
09d09e04d2fcf8 Dan Williams 2023-02-10 76 #define CXL_DAX_REGION_TYPE(x) NULL
779dd20cfb56c5 Ben Widawsky 2021-06-08 77 #endif
779dd20cfb56c5 Ben Widawsky 2021-06-08 78
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.