From: Dan Williams Date: Thu, 26 Aug 2021 23:32:13 +0000 (-0700) Subject: cxl/core: Replace devm_cxl_add_decoder() with non-devm version X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=e13d206924f51efbf7fce355c97fdacaad18ec04;p=users%2Fjedix%2Flinux-maple.git 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 Reported-by: Nathan Chancellor Reported-by: Dan Carpenter Link: https://lore.kernel.org/r/163002073312.1700305.17017280228713298614.stgit@dwillia2-desk3.amr.corp.intel.com Signed-off-by: Dan Williams --- diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index 0b7c6268bba45..7130beffc9293 100644 --- 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 1320a996220ae..3991ac231c3e3 100644 --- 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 c85b7fbad02dd..e0c9aacc4e9c9 100644 --- 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 68046b6a22b52..f9a260f54f2bf 100644 --- 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 3705b2454b666..708bfe92b5967 100644 --- 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;