Skip to contentSkip to conversation

rbd: fix module sysfs setup/teardown code

PATCH In review Alex Elder 3 participants 4 messages Feb 29, 2012 – Mar 2, 2012

Subsystems (2)
Block layer RADOS block device (RBD)
lore
Share & tools
lore

Alex Elder

author
#0 lore raw

PATCH 1/1 rbd: fix module sysfs setup/teardown code

Once rbd_bus_type is registered, it allows an "add" operation via the /sys/bus/rbd/add bus attribute, and adding a new rbd device that way establishes a connection between the device and rbd_root_dev. But rbd_root_dev is not registered until after the rbd_bus_type registration is complete. This could (in principle anyway) result in an invalid state.

Since rbd_root_dev has no tie to rbd_bus_type we can reorder these two initializations and never be faced with this scenario.

In addition, unregister the device in the event the bus registration fails at module init time.

Signed-off-by Alex Elder <[email protected]>
---
 drivers/block/rbd.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/block/rbd.c +5 -3
index bc539cd..b7c9af5 100644
@@ -2560,19 +2560,21 @@ static int rbd_sysfs_init(void)
 {
 	int ret;
 
-	ret = bus_register(&rbd_bus_type);
+	ret = device_register(&rbd_root_dev);
 	if (ret < 0)
 		return ret;
 
-	ret = device_register(&rbd_root_dev);
+	ret = bus_register(&rbd_bus_type);
+	if (ret)
+		device_unregister(&rbd_root_dev);
 
 	return ret;
 }
 
 static void rbd_sysfs_cleanup(void)
 {
-	device_unregister(&rbd_root_dev);
 	bus_unregister(&rbd_bus_type);
+	device_unregister(&rbd_root_dev);
 }
 
 int __init rbd_init(void)
Signature · 1 line
1.7.5.4
#1 lore raw
Quoted 25 lines · Alex Elder

Once rbd_bus_type is registered, it allows an "add" operation via the /sys/bus/rbd/add bus attribute, and adding a new rbd device that way establishes a connection between the device and rbd_root_dev. But rbd_root_dev is not registered until after the rbd_bus_type registration is complete.  This could (in principle anyway) result in an invalid state.

Since rbd_root_dev has no tie to rbd_bus_type we can reorder these two initializations and never be faced with this scenario.

In addition, unregister the device in the event the bus registration fails at module init time.

Signed-off-by Alex Elder <[email protected]>

---
 drivers/block/rbd.c |    8 +++++---

 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/block/rbd.c +0 -0
index bc539cd..b7c9af5 100644
@@ -2560,19 +2560,21 @@ static int rbd_sysfs_init(void)
  {

drivers/block/rbd.c static int rbd_sysfs_init(void) · line 2568

Quoted 10 lines
drivers/block/rbd.c +3 -2
        int ret;
 
-       ret = bus_register(&rbd_bus_type);
+       ret = device_register(&rbd_root_dev);
        if (ret < 0)
                return ret;
 
-       ret = device_register(&rbd_root_dev);
+       ret = bus_register(&rbd_bus_type);
+       if (ret)

For consistency if (ret < 0) or change the previous one.

Quoted 16 lines
drivers/block/rbd.c +2 -2
+               device_unregister(&rbd_root_dev);
 
        return ret;
  }
 
  static void rbd_sysfs_cleanup(void)
  {
-       device_unregister(&rbd_root_dev);
        bus_unregister(&rbd_bus_type);
+       device_unregister(&rbd_root_dev);
  }
 
  int __init rbd_init(void)
--

1.7.5.4

Quoted 4 lines

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#2 lore raw
Quoted 1 line · Yehuda Sadeh Weinraub

For consistency if (ret< 0) or change the previous one.

OK. I will update. -Alex

Reviewed-by on v1 1/1
#3 lore raw
In reply to Alex Elder
Reviewed-by Sage Weil <[email protected]>
Quoted 56 lines · Alex Elder

Once rbd_bus_type is registered, it allows an "add" operation via the /sys/bus/rbd/add bus attribute, and adding a new rbd device that way establishes a connection between the device and rbd_root_dev. But rbd_root_dev is not registered until after the rbd_bus_type registration is complete. This could (in principle anyway) result in an invalid state.

Since rbd_root_dev has no tie to rbd_bus_type we can reorder these two initializations and never be faced with this scenario.

In addition, unregister the device in the event the bus registration fails at module init time.

Signed-off-by Alex Elder <[email protected]>
---
 drivers/block/rbd.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/block/rbd.c +5 -3
index bc539cd..b7c9af5 100644
@@ -2560,19 +2560,21 @@ static int rbd_sysfs_init(void)
 {
 	int ret;
 
-	ret = bus_register(&rbd_bus_type);
+	ret = device_register(&rbd_root_dev);
 	if (ret < 0)
 		return ret;
 
-	ret = device_register(&rbd_root_dev);
+	ret = bus_register(&rbd_bus_type);
+	if (ret)
+		device_unregister(&rbd_root_dev);
 
 	return ret;
 }
 
 static void rbd_sysfs_cleanup(void)
 {
-	device_unregister(&rbd_root_dev);
 	bus_unregister(&rbd_bus_type);
+	device_unregister(&rbd_root_dev);
 }
 
 int __init rbd_init(void)
Signature · 6 lines
1.7.5.4

--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html