If the remove callback failed, it leaves some unfreed resources behind
that will never be cleared. I didn't manage to understand the driver
good enough to judge how critical that really is.
This patch is part of an effort to change the remove callbacks for
platform devices to return void in the hope this will prevent the wrong
assumption that returning an error code from .remove() is proper error
handling.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
drivers/acpi/apei/ghes.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
Hello,
on a side note: The remove callback calls (in some cases) free_irq() for
a shared interrupt. A requirement in this case is to disable the
device's interrupt beforehand. It's not obvious (to me that is) that
said irq is disabled here. This is another opportunity for ugly things
to happen.
Best regards
Uwe
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 307fbb97a116..78d2e4df74ee 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -1393,7 +1393,7 @@ static int ghes_probe(struct platform_device *ghes_dev)
return rc;
}
-static int ghes_remove(struct platform_device *ghes_dev)
+static int _ghes_remove(struct platform_device *ghes_dev)
{
int rc;
struct ghes *ghes;
@@ -1447,6 +1447,21 @@ static int ghes_remove(struct platform_device *ghes_dev)
return 0;
}
+static int ghes_remove(struct platform_device *ghes_dev)
+{
+ /*
+ * If _ghes_remove() returns an error, we're in trouble. Some of the
+ * cleanup was skipped then and this will never be catched up. So some
+ * resources will stay around, maybe even used although the platform
+ * device will be gone.
+ */
+ int err = _ghes_remove(ghes_dev);
+
+ WARN_ON(err);
+
+ return 0;
+}
+
static struct platform_driver ghes_platform_driver = {
.driver = {
.name = "GHES",
--
2.37.2
