| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <69320503ebad3_1e02100e9@dwillia2-mobl4.notmuch>
Date: Thu, 4 Dec 2025 14:02:43 -0800
From: <dan.j.williams@...el.com>
To: <dan.j.williams@...el.com>, "Cheatham, Benjamin"
<benjamin.cheatham@....com>, Dan Williams <dan.j.williams@...el.com>,
<dave.jiang@...el.com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<Smita.KoralahalliChannabasappa@....com>, <alison.schofield@...el.com>,
<terry.bowman@....com>, <alejandro.lucero-palau@....com>,
<linux-pci@...r.kernel.org>, <Jonathan.Cameron@...wei.com>, Alejandro Lucero
<alucerop@....com>
Subject: Re: [PATCH 6/6] cxl/mem: Introduce a memdev creation ->probe()
operation
dan.j.williams@ wrote:
[..]
> That is functionally equivalent to a new devm_cxl_add_memdev() flag, and
> is something that can come later when such a driver arrives.
Here are the fixups collected for a v2, I added some documentation of
the expectations around @ops:
1: aa399f4e13c5 = 1: 1368388728fd cxl/mem: Fix devm_cxl_memdev_edac_release() confusion
2: 120ac819e8cb = 2: f56b7c7ec1cb cxl/mem: Arrange for always-synchronous memdev attach
3: bcb0109994a3 = 3: 1fe83e925d59 cxl/port: Arrange for always synchronous endpoint attach
4: 74c426d1dd5d ! 4: 9bd03230d6f3 cxl/mem: Convert devm_cxl_add_memdev() to scope-based-cleanup
@@ drivers/cxl/core/memdev.c: static const struct file_operations cxl_memdev_fops =
+
+DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
+ if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
++
++static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
++{
++ int rc;
++
++ rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
++ cxlmd);
++ if (rc)
++ return ERR_PTR(rc);
++
++ return cxlmd;
++}
+
/*
* Core helper for devm_cxl_add_memdev() that wants to both create a device and
@@ drivers/cxl/core/memdev.c: static const struct file_operations cxl_memdev_fops =
- */
- cxlmd->cxlds = cxlds;
- cxlds->cxlmd = cxlmd;
-+ return ERR_PTR(rc);
-
+-
- cdev = &cxlmd->cdev;
- rc = cdev_device_add(cdev, dev);
-+ rc = cxlmd_add(cxlmd, cxlds);
- if (rc)
+- if (rc)
- goto err;
+ return ERR_PTR(rc);
- rc = devm_add_action_or_reset(host, cxl_memdev_unregister, cxlmd);
-+ rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
-+ no_free_ptr(cxlmd));
++ rc = cxlmd_add(cxlmd, cxlds);
if (rc)
return ERR_PTR(rc);
- return cxlmd;
--
+- return cxlmd;
+
-err:
- /*
- * The cdev was briefly live, shutdown any ioctl operations that
@@ drivers/cxl/core/memdev.c: static const struct file_operations cxl_memdev_fops =
- cxl_memdev_shutdown(dev);
- put_device(dev);
- return ERR_PTR(rc);
++ return cxl_memdev_autoremove(no_free_ptr(cxlmd));
}
EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
5: b48acca13cdb ! 5: e636831b1ff9 cxl/mem: Drop @host argument to devm_cxl_add_memdev()
@@ drivers/cxl/cxlmem.h: static inline bool is_cxl_endpoint(struct cxl_port *port)
struct cxl_memdev_state;
## drivers/cxl/core/memdev.c ##
-@@ drivers/cxl/core/memdev.c: DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
+@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
* Core helper for devm_cxl_add_memdev() that wants to both create a device and
* assert to the caller that upon return cxl_mem::probe() has been invoked.
*/
@@ drivers/cxl/core/memdev.c: DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
{
struct device *dev;
int rc;
-@@ drivers/cxl/core/memdev.c: struct cxl_memdev *__devm_cxl_add_memdev(struct device *host,
- if (rc)
- return ERR_PTR(rc);
-
-- rc = devm_add_action_or_reset(host, cxl_memdev_unregister,
-+ rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
- no_free_ptr(cxlmd));
- if (rc)
- return ERR_PTR(rc);
## drivers/cxl/mem.c ##
@@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
6: f7e58dea4878 ! 6: 13c07d702c92 cxl/mem: Introduce a memdev creation ->probe() operation
@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_alloc(struct cxl
dev = &cxlmd->dev;
device_initialize(dev);
-@@ drivers/cxl/core/memdev.c: static int cxlmd_add(struct cxl_memdev *cxlmd, struct cxl_dev_state *cxlds)
- DEFINE_FREE(put_cxlmd, struct cxl_memdev *,
- if (!IS_ERR_OR_NULL(_T)) put_device(&_T->dev);)
+@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
+ {
+ int rc;
-+static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
-+{
-+ struct cxl_memdev *ret = cxlmd;
-+ int rc;
-+
+ /*
-+ * If ops is provided fail if the driver is not attached upon
-+ * return. The ->endpoint ERR_PTR may have a more precise error
-+ * code to convey. Note that failure here could be the result of
-+ * a race to teardown the CXL port topology. I.e.
-+ * cxl_mem_probe() could have succeeded and then cxl_mem unbound
-+ * before the lock is acquired.
++ * If ops is provided fail if the driver is not attached upon return.
++ * Note that failure here could be the result of a race to teardown the
++ * CXL port topology. I.e. cxl_mem_probe() could have succeeded and then
++ * cxl_mem unbound before the lock is acquired.
+ */
+ guard(device)(&cxlmd->dev);
+ if (cxlmd->ops && !cxlmd->dev.driver) {
-+ ret = ERR_PTR(-ENXIO);
-+ if (IS_ERR(cxlmd->endpoint))
-+ ret = ERR_CAST(cxlmd->endpoint);
+ cxl_memdev_unregister(cxlmd);
-+ return ret;
++ return ERR_PTR(-ENXIO);
+ }
+
-+ rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
-+ cxlmd);
-+ if (rc)
-+ return ERR_PTR(rc);
-+
-+ return ret;
-+}
-+
- /*
+ rc = devm_add_action_or_reset(cxlmd->cxlds->dev, cxl_memdev_unregister,
+ cxlmd);
+ if (rc)
+@@ drivers/cxl/core/memdev.c: static struct cxl_memdev *cxl_memdev_autoremove(struct cxl_memdev *cxlmd)
* Core helper for devm_cxl_add_memdev() that wants to both create a device and
* assert to the caller that upon return cxl_mem::probe() has been invoked.
*/
@@ drivers/cxl/core/memdev.c: static int cxlmd_add(struct cxl_memdev *cxlmd, struct
if (IS_ERR(cxlmd))
return cxlmd;
-@@ drivers/cxl/core/memdev.c: struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds)
- if (rc)
- return ERR_PTR(rc);
-
-- rc = devm_add_action_or_reset(cxlds->dev, cxl_memdev_unregister,
-- no_free_ptr(cxlmd));
-- if (rc)
-- return ERR_PTR(rc);
-- return cxlmd;
-+ return cxl_memdev_autoremove(no_free_ptr(cxlmd));
- }
- EXPORT_SYMBOL_FOR_MODULES(__devm_cxl_add_memdev, "cxl_mem");
-
## drivers/cxl/mem.c ##
@@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
@@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
/**
* devm_cxl_add_memdev - Add a CXL memory device
* @cxlds: CXL device state to associate with the memdev
-+ * @ops: optional operations to run in cxl_mem::{probe,remove}() context
++ * @ops: optional operations to run in cxl_mem_probe() context
*
* Upon return the device will have had a chance to attach to the
- * cxl_mem driver, but may fail if the CXL topology is not ready
-@@ drivers/cxl/mem.c: static int cxl_mem_probe(struct device *dev)
+- * cxl_mem driver, but may fail if the CXL topology is not ready
+- * (hardware CXL link down, or software platform CXL root not attached)
++ * cxl_mem driver, but may fail to attach if the CXL topology is not ready
++ * (hardware CXL link down, or software platform CXL root not attached).
++ *
++ * When @ops is NULL it indicates the caller wants the memdev to remain
++ * registered even if it does not immediately attach to the CXL hierarchy. When
++ * @ops is provided a cxl_mem_probe() failure leads to failure of this routine.
+ *
* The parent of the resulting device and the devm context for allocations is
* @cxlds->dev.
*/
Powered by blists - more mailing lists