@@ -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;
}
@@ -491,10 +491,10 @@ struct cxl_decoder *cxl_decoder_alloc(struct cxl_port *port, int nr_targets)
}
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
@@ -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,
@@ -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);
@@ -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;
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> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/cxl/acpi.c | 14 +++++++++++--- drivers/cxl/core/bus.c | 45 ++++++++++++++++++++++++++------------------- drivers/cxl/core/core.h | 5 ----- drivers/cxl/core/pmem.c | 7 ++++++- drivers/cxl/cxl.h | 5 +++-- 5 files changed, 46 insertions(+), 30 deletions(-)