* [RFC PATCH v2 1/8] workqueue: Add resource managed version of delayed work init
2021-03-08 8:14 [RFC PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
@ 2021-03-08 8:14 ` Matti Vaittinen
2021-03-08 8:14 ` [RFC PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers Matti Vaittinen
2021-03-08 8:43 ` [RFC PATCH v2 8/8] watchdog: retu_wdt: Clean-up by using managed work init Matti Vaittinen
2 siblings, 0 replies; 5+ messages in thread
From: Matti Vaittinen @ 2021-03-08 8:14 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
Jean Delvare, Guenter Roeck, Hans de Goede, Mark Gross,
Sebastian Reichel, Chen-Yu Tsai, Liam Girdwood, Mark Brown,
Wim Van Sebroeck, Matti Vaittinen, gregkh, linux-kernel,
linux-arm-msm, linux-hwmon, platform-driver-x86, linux-pm,
linux-watchdog
A few drivers which need a delayed work-queue must cancel work at driver
detach. Some of those implement remove() solely for this purpose. Help
drivers to avoid unnecessary remove and error-branch implementation by
adding managed verision of delayed work initialization. This will also
help drivers to avoid mixing manual and devm based unwinding when other
resources are handled by devm.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
include/linux/devm-helpers.h | 53 ++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 include/linux/devm-helpers.h
diff --git a/include/linux/devm-helpers.h b/include/linux/devm-helpers.h
new file mode 100644
index 000000000000..f64e0c9f3763
--- /dev/null
+++ b/include/linux/devm-helpers.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef __LINUX_DEVM_HELPERS_H
+#define __LINUX_DEVM_HELPERS_H
+
+/*
+ * Functions which do automatically cancel operations or release resources upon
+ * driver detach.
+ *
+ * These should be helpful to avoid mixing the manual and devm-based resource
+ * management which can be source of annoying, rarely occurring,
+ * hard-to-reproduce bugs.
+ *
+ * Please take into account that devm based cancellation may be performed some
+ * time after the remove() is ran.
+ *
+ * Thus mixing devm and manual resource management can easily cause problems
+ * when unwinding operations with dependencies. IRQ scheduling a work in a queue
+ * is typical example where IRQs are often devm-managed and WQs are manually
+ * cleaned at remove(). If IRQs are not manually freed at remove() (and this is
+ * often the case when we use devm for IRQs) we have a period of time after
+ * remove() - and before devm managed IRQs are freed - where new IRQ may fire
+ * and schedule a work item which won't be cancelled because remove() was
+ * already ran.
+ */
+
+#include <linux/device.h>
+#include <linux/workqueue.h>
+
+static inline void devm_delayed_work_drop(void *res)
+{
+ cancel_delayed_work_sync(res);
+}
+
+/**
+ * devm_delayed_work_autocancel - Resource-managed work allocation
+ * @dev: Device which lifetime work is bound to
+ * @pdata: work to be cancelled when driver is detached
+ *
+ * Initialize work which is automatically cancelled when driver is detached.
+ * A few drivers need delayed work which must be cancelled before driver
+ * is detached to avoid accessing removed resources.
+ * devm_delayed_work_autocancel() can be used to omit the explicit
+ * cancelleation when driver is detached.
+ */
+static inline int devm_delayed_work_autocancel(struct device *dev,
+ struct delayed_work *w,
+ work_func_t worker)
+{
+ INIT_DELAYED_WORK(w, worker);
+ return devm_add_action(dev, devm_delayed_work_drop, w);
+}
+
+#endif
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [RFC PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers
2021-03-08 8:14 [RFC PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
2021-03-08 8:14 ` [RFC PATCH v2 1/8] workqueue: Add resource " Matti Vaittinen
@ 2021-03-08 8:14 ` Matti Vaittinen
2021-03-08 9:58 ` Hans de Goede
2021-03-08 8:43 ` [RFC PATCH v2 8/8] watchdog: retu_wdt: Clean-up by using managed work init Matti Vaittinen
2 siblings, 1 reply; 5+ messages in thread
From: Matti Vaittinen @ 2021-03-08 8:14 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
Jean Delvare, Guenter Roeck, Hans de Goede, Mark Gross,
Sebastian Reichel, Chen-Yu Tsai, Liam Girdwood, Mark Brown,
Wim Van Sebroeck, Matti Vaittinen, gregkh, linux-kernel,
linux-arm-msm, linux-hwmon, platform-driver-x86, linux-pm,
linux-watchdog
Devm helper header containing small inline helpers was added.
Hans promised to maintain it.
Add Hans as maintainer and myself as designated reviewer.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..ffcb00006e14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5169,6 +5169,12 @@ M: Torben Mathiasen <device@lanana.org>
S: Maintained
W: http://lanana.org/docs/device-list/index.html
+DEVICE RESOURCE MANAGEMENT HELPERS
+M: Hans de Goede <hdegoede@redhat.com>
+R: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
+S: Maintained
+F: include/linux/devm-helpers.h
+
DEVICE-MAPPER (LVM)
M: Alasdair Kergon <agk@redhat.com>
M: Mike Snitzer <snitzer@redhat.com>
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers
2021-03-08 8:14 ` [RFC PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers Matti Vaittinen
@ 2021-03-08 9:58 ` Hans de Goede
0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-03-08 9:58 UTC (permalink / raw)
To: Matti Vaittinen, mazziesaccount
Cc: MyungJoo Ham, Chanwoo Choi, Andy Gross, Bjorn Andersson,
Jean Delvare, Guenter Roeck, Mark Gross, Sebastian Reichel,
Chen-Yu Tsai, Liam Girdwood, Mark Brown, Wim Van Sebroeck,
gregkh, linux-kernel, linux-arm-msm, linux-hwmon,
platform-driver-x86, linux-pm, linux-watchdog
Hi,
On 3/8/21 9:14 AM, Matti Vaittinen wrote:
> Devm helper header containing small inline helpers was added.
> Hans promised to maintain it.
Yes I did promise that, didn't I? FWIW going this route is still
fine by me, assuming that having someone else maintain this makes
this easier on / more acceptable to Greg.
This is still going to need an Ack from Greg though.
Regards,
Hans
>
> Add Hans as maintainer and myself as designated reviewer.
>
> Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> ---
> MAINTAINERS | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d92f85ca831d..ffcb00006e14 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5169,6 +5169,12 @@ M: Torben Mathiasen <device@lanana.org>
> S: Maintained
> W: http://lanana.org/docs/device-list/index.html
>
> +DEVICE RESOURCE MANAGEMENT HELPERS
> +M: Hans de Goede <hdegoede@redhat.com>
> +R: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> +S: Maintained
> +F: include/linux/devm-helpers.h
> +
> DEVICE-MAPPER (LVM)
> M: Alasdair Kergon <agk@redhat.com>
> M: Mike Snitzer <snitzer@redhat.com>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [RFC PATCH v2 8/8] watchdog: retu_wdt: Clean-up by using managed work init
2021-03-08 8:14 [RFC PATCH v2 0/8] Add managed version of delayed work init Matti Vaittinen
2021-03-08 8:14 ` [RFC PATCH v2 1/8] workqueue: Add resource " Matti Vaittinen
2021-03-08 8:14 ` [RFC PATCH v2 2/8] MAINTAINERS: Add entry for devm helpers Matti Vaittinen
@ 2021-03-08 8:43 ` Matti Vaittinen
2 siblings, 0 replies; 5+ messages in thread
From: Matti Vaittinen @ 2021-03-08 8:43 UTC (permalink / raw)
To: matti.vaittinen, mazziesaccount
Cc: Guenter Roeck, Hans de Goede, Wim Van Sebroeck, Matti Vaittinen,
gregkh, linux-kernel, linux-watchdog
Few drivers implement remove call-back only for ensuring a delayed
work gets cancelled prior driver removal. Clean-up these by switching
to use devm_delayed_work_autocancel() instead.
This change is compile-tested only. All testing is appreciated.
Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
---
drivers/watchdog/retu_wdt.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/watchdog/retu_wdt.c b/drivers/watchdog/retu_wdt.c
index 258dfcf9cbda..2b9017e1cd91 100644
--- a/drivers/watchdog/retu_wdt.c
+++ b/drivers/watchdog/retu_wdt.c
@@ -8,6 +8,7 @@
* Rewritten by Aaro Koskinen.
*/
+#include <linux/devm-helpers.h>
#include <linux/slab.h>
#include <linux/errno.h>
#include <linux/device.h>
@@ -127,9 +128,12 @@ static int retu_wdt_probe(struct platform_device *pdev)
wdev->rdev = rdev;
wdev->dev = &pdev->dev;
- INIT_DELAYED_WORK(&wdev->ping_work, retu_wdt_ping_work);
+ ret = devm_delayed_work_autocancel(&pdev->dev, &wdev->ping_work,
+ retu_wdt_ping_work);
+ if (ret)
+ return ret;
- ret = watchdog_register_device(retu_wdt);
+ ret = devm_watchdog_register_device(&pdev->dev, retu_wdt);
if (ret < 0)
return ret;
@@ -138,25 +142,11 @@ static int retu_wdt_probe(struct platform_device *pdev)
else
retu_wdt_ping_enable(wdev);
- platform_set_drvdata(pdev, retu_wdt);
-
- return 0;
-}
-
-static int retu_wdt_remove(struct platform_device *pdev)
-{
- struct watchdog_device *wdog = platform_get_drvdata(pdev);
- struct retu_wdt_dev *wdev = watchdog_get_drvdata(wdog);
-
- watchdog_unregister_device(wdog);
- cancel_delayed_work_sync(&wdev->ping_work);
-
return 0;
}
static struct platform_driver retu_wdt_driver = {
.probe = retu_wdt_probe,
- .remove = retu_wdt_remove,
.driver = {
.name = "retu-wdt",
},
--
2.25.4
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
^ permalink raw reply related [flat|nested] 5+ messages in thread