From 7323fb22f05ff1d20498d267828870a5fbbaebd6 Mon Sep 17 00:00:00 2001 From: Shiyang Ruan Date: Tue, 26 Jan 2021 10:13:31 +0800 Subject: [PATCH 1/8] device-dax: Fix default return code of range_parse() The return value of range_parse() indicates the size when it is positive. The error code should be negative. Signed-off-by: Shiyang Ruan Reviewed-by: Joao Martins Link: https://lore.kernel.org/r/20210126021331.1059933-1-ruansy.fnst@cn.fujitsu.com Reported-by: Zhang Qilong Fixes: 8490e2e25b5a ("device-dax: add a range mapping allocation attribute") Signed-off-by: Dan Williams --- drivers/dax/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 737b207c9e30..3003558c1a8b 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -1038,7 +1038,7 @@ static ssize_t range_parse(const char *opt, size_t len, struct range *range) { unsigned long long addr = 0; char *start, *end, *str; - ssize_t rc = EINVAL; + ssize_t rc = -EINVAL; str = kstrdup(opt, GFP_KERNEL); if (!str) From 8409f942a1f8c28296910b7b1d60f27dd3e03162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 12 Feb 2021 18:10:42 +0100 Subject: [PATCH 2/8] libnvdimm/dimm: Simplify nvdimm_remove() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nvdimm_remove is only ever called after nvdimm_probe() returned successfully. In this case driver data is always set to a non-NULL value so the check for driver data being NULL can go away as it's always false. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210212171043.2136580-1-u.kleine-koenig@pengutronix.de Signed-off-by: Dan Williams --- drivers/nvdimm/dimm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c index 7d4ddc4d9322..94be3ae1d29f 100644 --- a/drivers/nvdimm/dimm.c +++ b/drivers/nvdimm/dimm.c @@ -117,9 +117,6 @@ static int nvdimm_remove(struct device *dev) { struct nvdimm_drvdata *ndd = dev_get_drvdata(dev); - if (!ndd) - return 0; - nvdimm_bus_lock(dev); dev_set_drvdata(dev, NULL); nvdimm_bus_unlock(dev); From 1f975074634a63f014e2b7e76852ee6d6005a91d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 12 Feb 2021 18:10:43 +0100 Subject: [PATCH 3/8] libnvdimm: Make remove callback return void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All drivers return 0 in their remove callback and the driver core ignores the return value of nvdimm_bus_remove() anyhow. So simplify by changing the driver remove callback to return void and return 0 unconditionally to the upper layer. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210212171043.2136580-2-u.kleine-koenig@pengutronix.de Signed-off-by: Dan Williams --- drivers/dax/pmem/compat.c | 3 +-- drivers/nvdimm/blk.c | 3 +-- drivers/nvdimm/bus.c | 13 +++++-------- drivers/nvdimm/dimm.c | 4 +--- drivers/nvdimm/pmem.c | 4 +--- drivers/nvdimm/region.c | 4 +--- include/linux/nd.h | 2 +- 7 files changed, 11 insertions(+), 22 deletions(-) diff --git a/drivers/dax/pmem/compat.c b/drivers/dax/pmem/compat.c index 863c114fd88c..d81dc35fd65d 100644 --- a/drivers/dax/pmem/compat.c +++ b/drivers/dax/pmem/compat.c @@ -41,10 +41,9 @@ static int dax_pmem_compat_release(struct device *dev, void *data) return 0; } -static int dax_pmem_compat_remove(struct device *dev) +static void dax_pmem_compat_remove(struct device *dev) { device_for_each_child(dev, NULL, dax_pmem_compat_release); - return 0; } static struct nd_device_driver dax_pmem_compat_driver = { diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c index 22e5617b2cea..8a53728e13e6 100644 --- a/drivers/nvdimm/blk.c +++ b/drivers/nvdimm/blk.c @@ -310,11 +310,10 @@ static int nd_blk_probe(struct device *dev) return nsblk_attach_disk(nsblk); } -static int nd_blk_remove(struct device *dev) +static void nd_blk_remove(struct device *dev) { if (is_nd_btt(dev)) nvdimm_namespace_detach_btt(to_nd_btt(dev)); - return 0; } static struct nd_device_driver nd_blk_driver = { diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index 2304c6183822..48f0985ca8a0 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -113,18 +113,17 @@ static int nvdimm_bus_remove(struct device *dev) struct nd_device_driver *nd_drv = to_nd_device_driver(dev->driver); struct module *provider = to_bus_provider(dev); struct nvdimm_bus *nvdimm_bus = walk_to_nvdimm_bus(dev); - int rc = 0; if (nd_drv->remove) { debug_nvdimm_lock(dev); - rc = nd_drv->remove(dev); + nd_drv->remove(dev); debug_nvdimm_unlock(dev); } - dev_dbg(&nvdimm_bus->dev, "%s.remove(%s) = %d\n", dev->driver->name, - dev_name(dev), rc); + dev_dbg(&nvdimm_bus->dev, "%s.remove(%s)\n", dev->driver->name, + dev_name(dev)); module_put(provider); - return rc; + return 0; } static void nvdimm_bus_shutdown(struct device *dev) @@ -427,7 +426,7 @@ static void free_badrange_list(struct list_head *badrange_list) list_del_init(badrange_list); } -static int nd_bus_remove(struct device *dev) +static void nd_bus_remove(struct device *dev) { struct nvdimm_bus *nvdimm_bus = to_nvdimm_bus(dev); @@ -446,8 +445,6 @@ static int nd_bus_remove(struct device *dev) spin_unlock(&nvdimm_bus->badrange.lock); nvdimm_bus_destroy_ndctl(nvdimm_bus); - - return 0; } static int nd_bus_probe(struct device *dev) diff --git a/drivers/nvdimm/dimm.c b/drivers/nvdimm/dimm.c index 94be3ae1d29f..91d9163ee303 100644 --- a/drivers/nvdimm/dimm.c +++ b/drivers/nvdimm/dimm.c @@ -113,7 +113,7 @@ static int nvdimm_probe(struct device *dev) return rc; } -static int nvdimm_remove(struct device *dev) +static void nvdimm_remove(struct device *dev) { struct nvdimm_drvdata *ndd = dev_get_drvdata(dev); @@ -121,8 +121,6 @@ static int nvdimm_remove(struct device *dev) dev_set_drvdata(dev, NULL); nvdimm_bus_unlock(dev); put_ndd(ndd); - - return 0; } static struct nd_device_driver nvdimm_driver = { diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c index 875076b0ea6c..062f0f22bac9 100644 --- a/drivers/nvdimm/pmem.c +++ b/drivers/nvdimm/pmem.c @@ -564,7 +564,7 @@ static int nd_pmem_probe(struct device *dev) return pmem_attach_disk(dev, ndns); } -static int nd_pmem_remove(struct device *dev) +static void nd_pmem_remove(struct device *dev) { struct pmem_device *pmem = dev_get_drvdata(dev); @@ -579,8 +579,6 @@ static int nd_pmem_remove(struct device *dev) pmem->bb_state = NULL; } nvdimm_flush(to_nd_region(dev->parent), NULL); - - return 0; } static void nd_pmem_shutdown(struct device *dev) diff --git a/drivers/nvdimm/region.c b/drivers/nvdimm/region.c index bfce87ed72ab..e0c34120df37 100644 --- a/drivers/nvdimm/region.c +++ b/drivers/nvdimm/region.c @@ -87,7 +87,7 @@ static int child_unregister(struct device *dev, void *data) return 0; } -static int nd_region_remove(struct device *dev) +static void nd_region_remove(struct device *dev) { struct nd_region *nd_region = to_nd_region(dev); @@ -108,8 +108,6 @@ static int nd_region_remove(struct device *dev) */ sysfs_put(nd_region->bb_state); nd_region->bb_state = NULL; - - return 0; } static int child_notify(struct device *dev, void *data) diff --git a/include/linux/nd.h b/include/linux/nd.h index 55c735997805..cec526c8043d 100644 --- a/include/linux/nd.h +++ b/include/linux/nd.h @@ -26,7 +26,7 @@ struct nd_device_driver { struct device_driver drv; unsigned long type; int (*probe)(struct device *dev); - int (*remove)(struct device *dev); + void (*remove)(struct device *dev); void (*shutdown)(struct device *dev); void (*notify)(struct device *dev, enum nvdimm_event event); }; From 5b8e64f1ada37574b9ab124e1414af2adf688a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 5 Feb 2021 23:28:38 +0100 Subject: [PATCH 4/8] device-dax: Prevent registering drivers without probe callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bus probe function dax_bus_probe() calls the probe callback without checking it to be non-NULL. Prevent a NULL pointer exception if a driver without a probe function is registered by refusing to register this driver. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210205222842.34896-2-uwe@kleine-koenig.org Signed-off-by: Dan Williams --- drivers/dax/bus.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 3003558c1a8b..2b29728ec2fd 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -1392,6 +1392,13 @@ int __dax_driver_register(struct dax_device_driver *dax_drv, struct device_driver *drv = &dax_drv->drv; int rc = 0; + /* + * dax_bus_probe() calls dax_drv->probe() unconditionally. + * So better be safe than sorry and ensure it is provided. + */ + if (!dax_drv->probe) + return -EINVAL; + INIT_LIST_HEAD(&dax_drv->ids); drv->owner = module; drv->name = mod_name; From 8029968e2ae02361f376751459dc644b45970b40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 5 Feb 2021 23:28:39 +0100 Subject: [PATCH 5/8] device-dax: Properly handle drivers without remove callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If all resources are allocated in .probe() using devm_ functions it might make sense to not provide a .remove() callback. Then the right thing is to just return success. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210205222842.34896-3-uwe@kleine-koenig.org Signed-off-by: Dan Williams --- drivers/dax/bus.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 2b29728ec2fd..ef53da441c5d 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -178,8 +178,12 @@ static int dax_bus_remove(struct device *dev) { struct dax_device_driver *dax_drv = to_dax_drv(dev->driver); struct dev_dax *dev_dax = to_dev_dax(dev); + int ret = 0; - return dax_drv->remove(dev_dax); + if (dax_drv->remove) + ret = dax_drv->remove(dev_dax); + + return ret; } static struct bus_type dax_bus_type = { From e307bf11c5198dbc0c9de0694c3e85c681648df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 5 Feb 2021 23:28:40 +0100 Subject: [PATCH 6/8] device-dax: Fix error path in dax_driver_register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The static variable match_always_count is supposed to track if there is a driver registered that has .match_always set. If driver_register() fails, the previous increment must be undone. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210205222842.34896-4-uwe@kleine-koenig.org Signed-off-by: Dan Williams --- drivers/dax/bus.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index ef53da441c5d..d2419e88073a 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -1420,7 +1420,15 @@ int __dax_driver_register(struct dax_device_driver *dax_drv, mutex_unlock(&dax_bus_lock); if (rc) return rc; - return driver_register(drv); + + rc = driver_register(drv); + if (rc && dax_drv->match_always) { + mutex_lock(&dax_bus_lock); + match_always_count -= dax_drv->match_always; + mutex_unlock(&dax_bus_lock); + } + + return rc; } EXPORT_SYMBOL_GPL(__dax_driver_register); From c80b53204d6ee8f70e5f4e846bc0e62dda64aee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 5 Feb 2021 23:28:41 +0100 Subject: [PATCH 7/8] device-dax: Drop an empty .remove callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dax core properly handles a dax driver not having a remove callback. So drop it without changing the effective behaviour. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210205222842.34896-5-uwe@kleine-koenig.org Signed-off-by: Dan Williams --- drivers/dax/device.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/dax/device.c b/drivers/dax/device.c index 5da2980bb16b..db92573c94e8 100644 --- a/drivers/dax/device.c +++ b/drivers/dax/device.c @@ -452,15 +452,9 @@ int dev_dax_probe(struct dev_dax *dev_dax) } EXPORT_SYMBOL_GPL(dev_dax_probe); -static int dev_dax_remove(struct dev_dax *dev_dax) -{ - /* all probe actions are unwound by devm */ - return 0; -} - static struct dax_device_driver device_dax_driver = { .probe = dev_dax_probe, - .remove = dev_dax_remove, + /* all probe actions are unwound by devm, so .remove isn't necessary */ .match_always = 1, }; From 0d519e0d52ee7c532d4018b90cd0b042d374c06d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 5 Feb 2021 23:28:42 +0100 Subject: [PATCH 8/8] dax-device: Make remove callback return void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The driver core ignores the return value of struct bus_type::remove() because there is only little that can be done. To simplify the quest to make this function return void, let struct dax_device_driver::remove() return void, too. All users already unconditionally return 0, this commit makes it obvious that returning an error code isn't intended. Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/20210205222842.34896-6-uwe@kleine-koenig.org Signed-off-by: Dan Williams --- drivers/dax/bus.c | 5 ++--- drivers/dax/bus.h | 2 +- drivers/dax/kmem.c | 7 ++----- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index d2419e88073a..452e85ae87a8 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -178,12 +178,11 @@ static int dax_bus_remove(struct device *dev) { struct dax_device_driver *dax_drv = to_dax_drv(dev->driver); struct dev_dax *dev_dax = to_dev_dax(dev); - int ret = 0; if (dax_drv->remove) - ret = dax_drv->remove(dev_dax); + dax_drv->remove(dev_dax); - return ret; + return 0; } static struct bus_type dax_bus_type = { diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h index 72b92f95509f..1e946ad7780a 100644 --- a/drivers/dax/bus.h +++ b/drivers/dax/bus.h @@ -39,7 +39,7 @@ struct dax_device_driver { struct list_head ids; int match_always; int (*probe)(struct dev_dax *dev); - int (*remove)(struct dev_dax *dev); + void (*remove)(struct dev_dax *dev); }; int __dax_driver_register(struct dax_device_driver *dax_drv, diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c index 403ec42472d1..ac231cc36359 100644 --- a/drivers/dax/kmem.c +++ b/drivers/dax/kmem.c @@ -136,7 +136,7 @@ err_res_name: } #ifdef CONFIG_MEMORY_HOTREMOVE -static int dev_dax_kmem_remove(struct dev_dax *dev_dax) +static void dev_dax_kmem_remove(struct dev_dax *dev_dax) { int i, success = 0; struct device *dev = &dev_dax->dev; @@ -176,11 +176,9 @@ static int dev_dax_kmem_remove(struct dev_dax *dev_dax) kfree(data); dev_set_drvdata(dev, NULL); } - - return 0; } #else -static int dev_dax_kmem_remove(struct dev_dax *dev_dax) +static void dev_dax_kmem_remove(struct dev_dax *dev_dax) { /* * Without hotremove purposely leak the request_mem_region() for the @@ -190,7 +188,6 @@ static int dev_dax_kmem_remove(struct dev_dax *dev_dax) * request_mem_region(). */ any_hotremove_failed = true; - return 0; } #endif /* CONFIG_MEMORY_HOTREMOVE */