cxl/core: Replace devm_cxl_add_decoder() with non-devm version

The split of devm_cxl_add_decoder() to factor out cxl_decoder_alloc()
introduced a couple bugs. An initialization order bug, and a layering
violation around assumptions about who is responsible for put_device()
when device_add() for the decoder fails. Fix this by making the caller
responsible for registering a devm callback to trigger
device_unregister() for the decoder device.

Fixes: b7ca54b62551 ("cxl/core: Split decoder setup into alloc + add")
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Nathan Chancellor <nathan@kernel.org>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/163002073312.1700305.17017280228713298614.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 0b7c6268bba452a33566ba70ebcb8164bd9fe66f..7130beffc92930d7e1d6ff5a328fef6853bc5c51 100644 (file)
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -133,7 +133,11 @@ static void cxl_add_cfmws_decoders(struct device *dev,
                cxld->interleave_granularity =
                        CFMWS_INTERLEAVE_GRANULARITY(cfmws);
 
-               rc = devm_cxl_add_decoder(dev, cxld, target_map);
+               rc = cxl_decoder_add(dev, cxld, target_map);
+               if (rc)
+                       put_device(&cxld->dev);
+               else
+                       rc = cxl_decoder_autoremove(dev, cxld);
                if (rc) {
                        dev_err(dev, "Failed to add decoder for %#llx-%#llx\n",
                                cfmws->base_hpa, cfmws->base_hpa +
@@ -340,10 +344,14 @@ static int add_host_bridge_uport(struct device *match, void *arg)
 
        single_port_map[0] = dport->port_id;
 
-       rc = devm_cxl_add_decoder(host, cxld, single_port_map);
+       rc = cxl_decoder_add(host, cxld, single_port_map);
+       if (rc)
+               put_device(&cxld->dev);
+       else
+               rc = cxl_decoder_autoremove(host, cxld);
+
        if (rc == 0)
                dev_dbg(host, "add: %s\n", dev_name(&cxld->dev));
-
        return rc;
 }
 

diff --git a/drivers/cxl/core/bus.c b/drivers/cxl/core/bus.c
index 1320a996220aeeb3440e3a84f1102a4bfc4647ec..3991ac231c3e33f5e07e6d79738b8e554c253723 100644 (file)
--- a/drivers/cxl/core/bus.c
+++ b/drivers/cxl/core/bus.c
@@ -491,10 +491,10 @@ err:
 }
 EXPORT_SYMBOL_GPL(cxl_decoder_alloc);
 
-int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
-                        int *target_map)
+int cxl_decoder_add(struct device *host, struct cxl_decoder *cxld,
+                   int *target_map)
 {
-       struct cxl_port *port = to_cxl_port(cxld->dev.parent);
+       struct cxl_port *port;
        struct device *dev;
        int rc = 0, i;
 
@@ -504,44 +504,51 @@ int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
        if (IS_ERR(cxld))
                return PTR_ERR(cxld);
 
-       if (cxld->interleave_ways < 1) {
-               rc = -EINVAL;
-               goto err;
-       }
+       if (cxld->interleave_ways < 1)
+               return -EINVAL;
 
+       port = to_cxl_port(cxld->dev.parent);
        device_lock(&port->dev);
-       if (list_empty(&port->dports))
+       if (list_empty(&port->dports)) {
                rc = -EINVAL;
+               goto out_unlock;
+       }
 
        for (i = 0; rc == 0 && target_map && i < cxld->nr_targets; i++) {
                struct cxl_dport *dport = find_dport(port, target_map[i]);
 
                if (!dport) {
                        rc = -ENXIO;
-                       break;
+                       goto out_unlock;
                }
                dev_dbg(host, "%s: target: %d\n", dev_name(dport->dport), i);
                cxld->target[i] = dport;
        }
        device_unlock(&port->dev);
-       if (rc)
-               goto err;
 
        dev = &cxld->dev;
        rc = dev_set_name(dev, "decoder%d.%d", port->id, cxld->id);
        if (rc)
-               goto err;
+               return rc;
 
-       rc = device_add(dev);
-       if (rc)
-               goto err;
+       return device_add(dev);
 
-       return devm_add_action_or_reset(host, unregister_cxl_dev, dev);
-err:
-       put_device(dev);
+out_unlock:
+       device_unlock(&port->dev);
        return rc;
 }
-EXPORT_SYMBOL_GPL(devm_cxl_add_decoder);
+EXPORT_SYMBOL_GPL(cxl_decoder_add);
+
+static void cxld_unregister(void *dev)
+{
+       device_unregister(dev);
+}
+
+int cxl_decoder_autoremove(struct device *host, struct cxl_decoder *cxld)
+{
+       return devm_add_action_or_reset(host, cxld_unregister, &cxld->dev);
+}
+EXPORT_SYMBOL_GPL(cxl_decoder_autoremove);
 
 /**
  * __cxl_driver_register - register a driver for the cxl bus

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index c85b7fbad02dd50018b84634cd03de11b5c8743d..e0c9aacc4e9c9813c6002b9d26ef49a3f8cbdd9c 100644 (file)
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -9,11 +9,6 @@ extern const struct device_type cxl_nvdimm_type;
 
 extern struct attribute_group cxl_base_attribute_group;
 
-static inline void unregister_cxl_dev(void *dev)
-{
-       device_unregister(dev);
-}
-
 struct cxl_send_command;
 struct cxl_mem_query_commands;
 int cxl_query_cmd(struct cxl_memdev *cxlmd,

diff --git a/drivers/cxl/core/pmem.c b/drivers/cxl/core/pmem.c
index 68046b6a22b52bf4d3aafdf15551ec6153dc7fd1..f9a260f54f2bf2669570c132c7e44654c88934e4 100644 (file)
--- a/drivers/cxl/core/pmem.c
+++ b/drivers/cxl/core/pmem.c
@@ -203,6 +203,11 @@ static struct cxl_nvdimm *cxl_nvdimm_alloc(struct cxl_memdev *cxlmd)
        return cxl_nvd;
 }
 
+static void cxl_nvd_unregister(void *dev)
+{
+       device_unregister(dev);
+}
+
 int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd)
 {
        struct cxl_nvdimm *cxl_nvd;
@@ -225,7 +230,7 @@ int devm_cxl_add_nvdimm(struct device *host, struct cxl_memdev *cxlmd)
        dev_dbg(host, "%s: register %s\n", dev_name(dev->parent),
                dev_name(dev));
 
-       return devm_add_action_or_reset(host, unregister_cxl_dev, dev);
+       return devm_add_action_or_reset(host, cxl_nvd_unregister, dev);
 
 err:
        put_device(dev);

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 3705b2454b666e7253a583f6b5c52a8a0cc98972..708bfe92b59677874fc92757521be5a2e039df7a 100644 (file)
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -289,8 +289,9 @@ int cxl_add_dport(struct cxl_port *port, struct device *dport, int port_id,
 struct cxl_decoder *to_cxl_decoder(struct device *dev);
 bool is_root_decoder(struct device *dev);
 struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port, int nr_targets);
-int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
-                        int *target_map);
+int cxl_decoder_add(struct device *host, struct cxl_decoder *cxld,
+                   int *target_map);
+int cxl_decoder_autoremove(struct device *host, struct cxl_decoder *cxld);
 
 extern struct bus_type cxl_bus_type;