Skip to contentSkip to conversation

MIPS: ip22-gio: fix leaks and replace static root

PATCH In review Johan Hovold 2 participants 7 messages Apr 24 – May 27

Subsystems (2)
Stable branch MIPS
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover MIPS: ip22-gio: fix leaks and replace static root

This series fixes some issues found through inspection when looking into replacing the statically allocated gio bus root device.

Johan

Johan Hovold (5):
MIPS: ip22-gio: fix kfree() of static object
MIPS: ip22-gio: fix gio device memory leak
MIPS: ip22-gio: fix device reference leak in probe
MIPS: ip22-gio: switch to dynamic root device
MIPS: ip22-gio: do not export device release function

 arch/mips/include/asm/gio_device.h |  6 ------
 arch/mips/sgi-ip22/ip22-gio.c      | 34 ++++++++++--------------------
 2 files changed, 11 insertions(+), 29 deletions(-)
Signature · 1 line
2.53.0
author
#1 lore raw

PATCH 1/5 MIPS: ip22-gio: fix kfree() of static object

The gio bus root device is a statically allocated object which must not be freed by kfree() on failure to register the device or bus.

Fixes 82242d28ff8b ("MIPS: IP22: Add missing put_device call")
Cc [email protected] # 3.17
Cc Levente Kurusa <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/mips/sgi-ip22/ip22-gio.c | 1 -
 1 file changed, 1 deletion(-)
arch/mips/sgi-ip22/ip22-gio.c +0 -1
index 9eec8842ffb7..a574441fa44b 100644
@@ -30,7 +30,6 @@ static struct {
 
 static void gio_bus_release(struct device *dev)
 {
-	kfree(dev);
 }
 
 static struct device gio_bus = {
Signature · 1 line
2.53.0
author
#2 lore raw

PATCH 2/5 MIPS: ip22-gio: fix gio device memory leak

In reply to Johan Hovold

The gio device release callback was never wired up so gio devices are not freed when the last reference is dropped.

Fixes e84de0c61905 ("MIPS: GIO bus support for SGI IP22/28")
Cc [email protected] # 3.3
Cc Thomas Bogendoerfer <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/mips/sgi-ip22/ip22-gio.c | 2 ++
 1 file changed, 2 insertions(+)
arch/mips/sgi-ip22/ip22-gio.c +2 -0
index a574441fa44b..2f5c6c6f8254 100644
@@ -100,6 +100,8 @@ int gio_device_register(struct gio_device *giodev)
 {
 	giodev->dev.bus = &gio_bus_type;
 	giodev->dev.parent = &gio_bus;
+	giodev->dev.release = gio_release_dev;
+
 	return device_register(&giodev->dev);
 }
 EXPORT_SYMBOL_GPL(gio_device_register);
Signature · 1 line
2.53.0
author
#3 lore raw

PATCH 3/5 MIPS: ip22-gio: fix device reference leak in probe

In reply to Johan Hovold

The gio probe function needlessly takes a device reference which is never released and therefore prevents unbound gio devices from being freed.

Fixes e84de0c61905 ("MIPS: GIO bus support for SGI IP22/28")
Cc [email protected] # 3.3
Cc Thomas Bogendoerfer <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 arch/mips/sgi-ip22/ip22-gio.c | 4 ----
 1 file changed, 4 deletions(-)
arch/mips/sgi-ip22/ip22-gio.c +0 -4
index 2f5c6c6f8254..7b7572d11250 100644
@@ -133,13 +133,9 @@ static int gio_device_probe(struct device *dev)
 	if (!drv->probe)
 		return error;
 
-	gio_dev_get(gio_dev);
-
 	match = gio_match_device(drv->id_table, gio_dev);
 	if (match)
 		error = drv->probe(gio_dev, match);
-	if (error)
-		gio_dev_put(gio_dev);
 
 	return error;
 }
Signature · 1 line
2.53.0
author
#4 lore raw

PATCH 4/5 MIPS: ip22-gio: switch to dynamic root device

In reply to Johan Hovold

Driver core expects devices to be dynamically allocated and will, for example, complain loudly when no release function has been provided.

Use root_device_register() to allocate and register the root device instead of open coding using a static device.

Signed-off-by Johan Hovold <[email protected]>
---
 arch/mips/sgi-ip22/ip22-gio.c | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)
arch/mips/sgi-ip22/ip22-gio.c +8 -16
index 7b7572d11250..d29067430b44 100644
@@ -28,14 +28,7 @@ static struct {
 	{ .name = "SGI GR2/GR3", .id = 0x7f },
 };
 
-static void gio_bus_release(struct device *dev)
-{
-}
-
-static struct device gio_bus = {
-	.init_name = "gio",
-	.release = &gio_bus_release,
-};
+static struct device *gio_bus;
 
 /**
  * gio_match_device - Tell if an of_device structure has a matching

@@ -99,7 +92,7 @@ EXPORT_SYMBOL_GPL(gio_release_dev);
 int gio_device_register(struct gio_device *giodev)
 {
 	giodev->dev.bus = &gio_bus_type;
-	giodev->dev.parent = &gio_bus;
+	giodev->dev.parent = gio_bus;
 	giodev->dev.release = gio_release_dev;
 
 	return device_register(&giodev->dev);

@@ -397,11 +390,9 @@ static int __init ip22_gio_init(void)
 	unsigned int pbdma __maybe_unused;
 	int ret;
 
-	ret = device_register(&gio_bus);
-	if (ret) {
-		put_device(&gio_bus);
-		return ret;
-	}
+	gio_bus = root_device_register("gio");
+	if (IS_ERR(gio_bus))
+		return PTR_ERR(gio_bus);
 
 	ret = bus_register(&gio_bus_type);
 	if (!ret) {

@@ -420,8 +411,9 @@ static int __init ip22_gio_init(void)
 			ip22_check_gio(1, GIO_SLOT_EXP0_BASE, SGI_GIOEXP0_IRQ);
 			ip22_check_gio(2, GIO_SLOT_EXP1_BASE, SGI_GIOEXP1_IRQ);
 		}
-	} else
-		device_unregister(&gio_bus);
+	} else {
+		root_device_unregister(gio_bus);
+	}
 
 	return ret;
 }
Signature · 1 line
2.53.0
author
#5 lore raw

PATCH 5/5 MIPS: ip22-gio: do not export device release function

In reply to Johan Hovold

There is no need to export the gio device release function as the devices are reference counted.

Drop the export along with the unused inline wrapper.

Signed-off-by Johan Hovold <[email protected]>
---
 arch/mips/include/asm/gio_device.h | 6 ------
 arch/mips/sgi-ip22/ip22-gio.c      | 3 +--
 2 files changed, 1 insertion(+), 8 deletions(-)
arch/mips/include/asm/gio_device.h +0 -6
index 159087f5386e..f9c102f038d6 100644
@@ -37,12 +37,6 @@ extern void gio_dev_put(struct gio_device *);
 
 extern int gio_device_register(struct gio_device *);
 extern void gio_device_unregister(struct gio_device *);
-extern void gio_release_dev(struct device *);
-
-static inline void gio_device_free(struct gio_device *dev)
-{
-	gio_release_dev(&dev->dev);
-}
 
 extern int gio_register_driver(struct gio_driver *);
 extern void gio_unregister_driver(struct gio_driver *);
arch/mips/sgi-ip22/ip22-gio.c +1 -2
index d29067430b44..54e17c8693e2 100644
@@ -80,14 +80,13 @@ EXPORT_SYMBOL_GPL(gio_dev_put);
  * Will be called only by the device core when all users of this gio device are
  * done.
  */
-void gio_release_dev(struct device *dev)
+static void gio_release_dev(struct device *dev)
 {
 	struct gio_device *giodev;
 
 	giodev = to_gio_device(dev);
 	kfree(giodev);
 }
-EXPORT_SYMBOL_GPL(gio_release_dev);
 
 int gio_device_register(struct gio_device *giodev)
 {
Signature · 1 line
2.53.0
maintainer
#6 lore raw
In reply to Johan Hovold
Quoted 13 lines · Johan Hovold

This series fixes some issues found through inspection when looking into replacing the statically allocated gio bus root device.

Johan

Johan Hovold (5):
MIPS: ip22-gio: fix kfree() of static object
MIPS: ip22-gio: fix gio device memory leak
MIPS: ip22-gio: fix device reference leak in probe
MIPS: ip22-gio: switch to dynamic root device
MIPS: ip22-gio: do not export device release function

Quoted 3 lines
 arch/mips/include/asm/gio_device.h |  6 ------
 arch/mips/sgi-ip22/ip22-gio.c      | 34 ++++++++++--------------------
 2 files changed, 11 insertions(+), 29 deletions(-)

series applied to mips-next

Thomas.

Signature · 2 lines
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]