@@ -1399,14 +1399,14 @@ static void delete_endpoint(void *data)
struct cxl_port *endpoint = cxlmd->endpoint;
struct device *host = endpoint_host(endpoint);
- scoped_guard(device, host) {
- if (host->driver && !endpoint->dead) {
- devm_release_action(host, cxl_unlink_parent_dport, endpoint);
- devm_release_action(host, cxl_unlink_uport, endpoint);
- devm_release_action(host, unregister_port, endpoint);
- }
- cxlmd->endpoint = NULL;
+ device_lock(host);
+ if (host->driver && !endpoint->dead) {
+ devm_release_action(host, cxl_unlink_parent_dport, endpoint);
+ devm_release_action(host, cxl_unlink_uport, endpoint);
+ devm_release_action(host, unregister_port, endpoint);
}
+ cxlmd->endpoint = NULL;
+ device_unlock(host);
put_device(&endpoint->dev);
put_device(host);
}
@@ -1571,38 +1571,40 @@ static int add_port_attach_ep(struct cxl_memdev *cxlmd,
* dereferencing the device of the port before the parent_port releasing.
*/
struct cxl_port *port __free(put_cxl_port) = NULL;
- scoped_guard(device, &parent_port->dev) {
- if (!parent_port->dev.driver) {
- dev_warn(&cxlmd->dev,
- "port %s:%s disabled, failed to enumerate CXL.mem\n",
- dev_name(&parent_port->dev), dev_name(uport_dev));
- return -ENXIO;
- }
-
- port = find_cxl_port_at(parent_port, dport_dev, &dport);
- if (!port) {
- component_reg_phys = find_component_registers(uport_dev);
- port = devm_cxl_add_port(&parent_port->dev, uport_dev,
- component_reg_phys, parent_dport);
- if (IS_ERR(port))
- return PTR_ERR(port);
+ device_lock(&parent_port->dev);
+ if (!parent_port->dev.driver) {
+ dev_warn(&cxlmd->dev,
+ "port %s:%s disabled, failed to enumerate CXL.mem\n",
+ dev_name(&parent_port->dev), dev_name(uport_dev));
+ port = ERR_PTR(-ENXIO);
+ goto out;
+ }
- /* retry find to pick up the new dport information */
+ port = find_cxl_port_at(parent_port, dport_dev, &dport);
+ if (!port) {
+ component_reg_phys = find_component_registers(uport_dev);
+ port = devm_cxl_add_port(&parent_port->dev, uport_dev,
+ component_reg_phys, parent_dport);
+ /* retry find to pick up the new dport information */
+ if (!IS_ERR(port))
port = find_cxl_port_at(parent_port, dport_dev, &dport);
- if (!port)
- return -ENXIO;
- }
}
+out:
+ device_unlock(&parent_port->dev);
- dev_dbg(&cxlmd->dev, "add to new port %s:%s\n",
- dev_name(&port->dev), dev_name(port->uport_dev));
- rc = cxl_add_ep(dport, &cxlmd->dev);
- if (rc == -EBUSY) {
- /*
- * "can't" happen, but this error code means
- * something to the caller, so translate it.
- */
- rc = -ENXIO;
+ if (IS_ERR(port))
+ rc = PTR_ERR(port);
+ else {
+ dev_dbg(&cxlmd->dev, "add to new port %s:%s\n",
+ dev_name(&port->dev), dev_name(port->uport_dev));
+ rc = cxl_add_ep(dport, &cxlmd->dev);
+ if (rc == -EBUSY) {
+ /*
+ * "can't" happen, but this error code means
+ * something to the caller, so translate it.
+ */
+ rc = -ENXIO;
+ }
}
return rc;
@@ -3083,11 +3083,11 @@ static void cxlr_release_nvdimm(void *_cxlr)
struct cxl_region *cxlr = _cxlr;
struct cxl_nvdimm_bridge *cxl_nvb = cxlr->cxl_nvb;
- scoped_guard(device, &cxl_nvb->dev) {
- if (cxlr->cxlr_pmem)
- devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
- cxlr->cxlr_pmem);
- }
+ device_lock(&cxl_nvb->dev);
+ if (cxlr->cxlr_pmem)
+ devm_release_action(&cxl_nvb->dev, cxlr_pmem_unregister,
+ cxlr->cxlr_pmem);
+ device_unlock(&cxl_nvb->dev);
cxlr->cxl_nvb = NULL;
put_device(&cxl_nvb->dev);
}
@@ -3123,14 +3123,13 @@ static int devm_cxl_add_pmem_region(struct cxl_region *cxlr)
dev_dbg(&cxlr->dev, "%s: register %s\n", dev_name(dev->parent),
dev_name(dev));
- 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;
- }
+ device_lock(&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;
+ device_unlock(&cxl_nvb->dev);
if (rc)
goto err_bridge;
@@ -168,18 +168,20 @@ static int cxl_mem_probe(struct device *dev)
cxl_dport_init_ras_reporting(dport, dev);
- scoped_guard(device, endpoint_parent) {
- if (!endpoint_parent->driver) {
- dev_err(dev, "CXL port topology %s not enabled\n",
- dev_name(endpoint_parent));
- return -ENXIO;
- }
-
- rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
- if (rc)
- return rc;
+ device_lock(endpoint_parent);
+ if (!endpoint_parent->driver) {
+ dev_err(dev, "CXL port topology %s not enabled\n",
+ dev_name(endpoint_parent));
+ rc = -ENXIO;
+ goto unlock;
}
+ rc = devm_cxl_add_endpoint(endpoint_parent, cxlmd, dport);
+unlock:
+ device_unlock(endpoint_parent);
+ if (rc)
+ return rc;
+
/*
* The kernel may be operating out of CXL memory on this device,
* there is no spec defined way to determine whether this device
@@ -237,13 +237,15 @@ static int detach_nvdimm(struct device *dev, void *data)
if (!is_cxl_nvdimm(dev))
return 0;
- scoped_guard(device, dev) {
- if (dev->driver) {
- cxl_nvd = to_cxl_nvdimm(dev);
- if (cxl_nvd->cxlmd && cxl_nvd->cxlmd->cxl_nvb == data)
- release = true;
- }
- }
+ device_lock(dev);
+ if (!dev->driver)
+ goto out;
+
+ cxl_nvd = to_cxl_nvdimm(dev);
+ if (cxl_nvd->cxlmd && cxl_nvd->cxlmd->cxl_nvb == data)
+ release = true;
+out:
+ device_unlock(dev);
if (release)
device_release_driver(dev);
return 0;
Dan points out via smatch [1] that the conversion of add_port_attach_ep() to use cleanup helpers does not correctly handle a pointer reassignment case. drivers/cxl/core/port.c:1591 add_port_attach_ep() warn: re-assigning __cleanup__ ptr 'port' The conversion to scoped_guard() complicates rather than simplifies code readability, so just revert the scoped_guard() usage to make the next patch that fixes the reassignment of port consistent with the approach that either an entire function is converted at once, or none of it. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Link: http://lore.kernel.org/2a19289b-0bcf-42c4-82a9-268a922535f2@stanley.mountain [1] Cc: Li Ming <ming4.li@intel.com> Fixes: 7f569e917b78 ("cxl/port: Use scoped_guard()/guard() to drop device_lock() for cxl_port") Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/core/port.c | 72 +++++++++++++++++++++++---------------------- drivers/cxl/core/region.c | 25 ++++++++-------- drivers/cxl/mem.c | 22 ++++++++------ drivers/cxl/pmem.c | 16 ++++++---- 4 files changed, 70 insertions(+), 65 deletions(-)