* [PATCH v2 0/5] dax-device: Some cleanups
@ 2021-02-05 22:28 Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 1/5] dax-device: Prevent registering drivers without probe callback Uwe Kleine-König
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-05 22:28 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Andrew Morton
Cc: linux-nvdimm, linux-kernel, Greg Kroah-Hartman
Hello,
I didn't get any feedback for the (implicit) v1 of this series that
started with Message-Id: 20210127230124.109522-1-uwe@kleine-koenig.org,
but I identified a few improvements myself:
- Use "dax-device" consistently as a prefix
- Instead of requiring a .remove callback, make it explicitly
optional. (Drop checking for .remove from former patch 1, introduce
new patch "Properly handle drivers without remove callback")
- The new patch about remove being optional allows to simplify one of
the two dax drivers which is implemented in patch 4
- Patch 5 got a bit smaller because we now have one driver less with a
remove callback.
- Added Andrew to To: as he merged dax drivers in the past.
Andrew: Assuming you consider these patches useful, would you please
care for merging them?
Best regards
Uwe
Uwe Kleine-König (5):
dax-device: Prevent registering drivers without probe callback
dax-device: Properly handle drivers without remove callback
dax-device: Fix error path in dax_driver_register
dax-device: Drop an empty .remove callback
dax-device: Make remove callback return void
drivers/dax/bus.c | 22 ++++++++++++++++++++--
drivers/dax/bus.h | 2 +-
drivers/dax/device.c | 8 +-------
drivers/dax/kmem.c | 7 ++-----
4 files changed, 24 insertions(+), 15 deletions(-)
base-commit: 5c8fe583cce542aa0b84adc939ce85293de36e5e
--
2.29.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/5] dax-device: Prevent registering drivers without probe callback
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
@ 2021-02-05 22:28 ` Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 2/5] dax-device: Properly handle drivers without remove callback Uwe Kleine-König
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-05 22:28 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Andrew Morton
Cc: linux-nvdimm, linux-kernel, Greg Kroah-Hartman
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 <uwe@kleine-koenig.org>
---
drivers/dax/bus.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 737b207c9e30..72fc4b9b9ae6 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;
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/5] dax-device: Properly handle drivers without remove callback
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 1/5] dax-device: Prevent registering drivers without probe callback Uwe Kleine-König
@ 2021-02-05 22:28 ` Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 3/5] dax-device: Fix error path in dax_driver_register Uwe Kleine-König
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-05 22:28 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Andrew Morton
Cc: linux-nvdimm, linux-kernel, Greg Kroah-Hartman
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 <uwe@kleine-koenig.org>
---
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 72fc4b9b9ae6..2c9e3f6f615f 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 = {
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/5] dax-device: Fix error path in dax_driver_register
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 1/5] dax-device: Prevent registering drivers without probe callback Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 2/5] dax-device: Properly handle drivers without remove callback Uwe Kleine-König
@ 2021-02-05 22:28 ` Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 4/5] dax-device: Drop an empty .remove callback Uwe Kleine-König
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-05 22:28 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Andrew Morton
Cc: linux-nvdimm, linux-kernel, Greg Kroah-Hartman
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 <uwe@kleine-koenig.org>
---
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 2c9e3f6f615f..bc425f1c52bd 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);
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 4/5] dax-device: Drop an empty .remove callback
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
` (2 preceding siblings ...)
2021-02-05 22:28 ` [PATCH v2 3/5] dax-device: Fix error path in dax_driver_register Uwe Kleine-König
@ 2021-02-05 22:28 ` Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 5/5] dax-device: Make remove callback return void Uwe Kleine-König
2021-02-17 3:48 ` [PATCH v2 0/5] dax-device: Some cleanups Dan Williams
5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-05 22:28 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Andrew Morton
Cc: linux-nvdimm, linux-kernel, Greg Kroah-Hartman
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 <uwe@kleine-koenig.org>
---
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,
};
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 5/5] dax-device: Make remove callback return void
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
` (3 preceding siblings ...)
2021-02-05 22:28 ` [PATCH v2 4/5] dax-device: Drop an empty .remove callback Uwe Kleine-König
@ 2021-02-05 22:28 ` Uwe Kleine-König
2021-02-17 3:48 ` [PATCH v2 0/5] dax-device: Some cleanups Dan Williams
5 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-05 22:28 UTC (permalink / raw)
To: Dan Williams, Vishal Verma, Dave Jiang, Andrew Morton
Cc: linux-nvdimm, linux-kernel, Greg Kroah-Hartman
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 <uwe@kleine-koenig.org>
---
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 bc425f1c52bd..0a939e28d048 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 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax)
}
#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 */
--
2.29.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] dax-device: Some cleanups
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
` (4 preceding siblings ...)
2021-02-05 22:28 ` [PATCH v2 5/5] dax-device: Make remove callback return void Uwe Kleine-König
@ 2021-02-17 3:48 ` Dan Williams
2021-02-17 3:55 ` Dan Williams
5 siblings, 1 reply; 9+ messages in thread
From: Dan Williams @ 2021-02-17 3:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Vishal Verma, Dave Jiang, Andrew Morton, linux-nvdimm,
Linux Kernel Mailing List, Greg Kroah-Hartman
On Fri, Feb 5, 2021 at 2:29 PM Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
>
> Hello,
>
> I didn't get any feedback for the (implicit) v1 of this series that
> started with Message-Id: 20210127230124.109522-1-uwe@kleine-koenig.org,
> but I identified a few improvements myself:
>
> - Use "dax-device" consistently as a prefix
> - Instead of requiring a .remove callback, make it explicitly
> optional. (Drop checking for .remove from former patch 1, introduce
> new patch "Properly handle drivers without remove callback")
> - The new patch about remove being optional allows to simplify one of
> the two dax drivers which is implemented in patch 4
> - Patch 5 got a bit smaller because we now have one driver less with a
> remove callback.
> - Added Andrew to To: as he merged dax drivers in the past.
>
> Andrew: Assuming you consider these patches useful, would you please
> care for merging them?
I've routed device-dax patches through Andrew when they had core-mm
entanglements, but a pure device-dax series like this I can take
through my tree.
One small comment on patch5, otherwise looks good.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] dax-device: Some cleanups
2021-02-17 3:48 ` [PATCH v2 0/5] dax-device: Some cleanups Dan Williams
@ 2021-02-17 3:55 ` Dan Williams
2021-02-17 6:39 ` Uwe Kleine-König
0 siblings, 1 reply; 9+ messages in thread
From: Dan Williams @ 2021-02-17 3:55 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Vishal Verma, Dave Jiang, Andrew Morton, linux-nvdimm,
Linux Kernel Mailing List, Greg Kroah-Hartman
On Tue, Feb 16, 2021 at 7:48 PM Dan Williams <dan.j.williams@intel.com> wrote:
>
> On Fri, Feb 5, 2021 at 2:29 PM Uwe Kleine-König <uwe@kleine-koenig.org> wrote:
> >
> > Hello,
> >
> > I didn't get any feedback for the (implicit) v1 of this series that
> > started with Message-Id: 20210127230124.109522-1-uwe@kleine-koenig.org,
> > but I identified a few improvements myself:
> >
> > - Use "dax-device" consistently as a prefix
> > - Instead of requiring a .remove callback, make it explicitly
> > optional. (Drop checking for .remove from former patch 1, introduce
> > new patch "Properly handle drivers without remove callback")
> > - The new patch about remove being optional allows to simplify one of
> > the two dax drivers which is implemented in patch 4
> > - Patch 5 got a bit smaller because we now have one driver less with a
> > remove callback.
> > - Added Andrew to To: as he merged dax drivers in the past.
> >
> > Andrew: Assuming you consider these patches useful, would you please
> > care for merging them?
>
> I've routed device-dax patches through Andrew when they had core-mm
> entanglements, but a pure device-dax series like this I can take
> through my tree.
>
> One small comment on patch5, otherwise looks good.
I take it back, patch5 looks good. I was going to ask about the return
value removal for dax_bus_remove(), but that would need struct
bus_type to change prototypes.
All merged to the nvdimm tree.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/5] dax-device: Some cleanups
2021-02-17 3:55 ` Dan Williams
@ 2021-02-17 6:39 ` Uwe Kleine-König
0 siblings, 0 replies; 9+ messages in thread
From: Uwe Kleine-König @ 2021-02-17 6:39 UTC (permalink / raw)
To: Dan Williams
Cc: Vishal Verma, Dave Jiang, Andrew Morton, linux-nvdimm,
Linux Kernel Mailing List, Greg Kroah-Hartman
[-- Attachment #1.1: Type: text/plain, Size: 619 bytes --]
Hello Dan,
On 2/17/21 4:55 AM, Dan Williams wrote:
>> One small comment on patch5, otherwise looks good.
>
> I take it back, patch5 looks good. I was going to ask about the return
> value removal for dax_bus_remove(), but that would need struct
> bus_type to change prototypes.
Changing struct bus_type::remove to return void is the eventual plan. To
make this a pretty and easily reviewable patch I currently go through
all buses and make sure that for the prototype change I only have to do
one s/int/void/ and drop a "return 0" per bus.
> All merged to the nvdimm tree.
Great, thanks
Uwe
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-17 6:40 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05 22:28 [PATCH v2 0/5] dax-device: Some cleanups Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 1/5] dax-device: Prevent registering drivers without probe callback Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 2/5] dax-device: Properly handle drivers without remove callback Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 3/5] dax-device: Fix error path in dax_driver_register Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 4/5] dax-device: Drop an empty .remove callback Uwe Kleine-König
2021-02-05 22:28 ` [PATCH v2 5/5] dax-device: Make remove callback return void Uwe Kleine-König
2021-02-17 3:48 ` [PATCH v2 0/5] dax-device: Some cleanups Dan Williams
2021-02-17 3:55 ` Dan Williams
2021-02-17 6:39 ` Uwe Kleine-König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).