rbd: set removing flag while holding list lock

When unmapping a device, its id is supplied, and that is used to
look up which rbd device should be unmapped.  Looking up the
device involves searching the rbd device list while holding
a spinlock that protects access to that list.

Currently all of this is done under protection of the control lock,
but that protection is going away soon.  To ensure the rbd_dev is
still valid (still on the list) while setting its REMOVING flag, do
so while still holding the list lock.  To do so, get rid of
__rbd_get_dev(), and open code what it did in the one place it
was used.

Signed-off-by: Alex Elder <elder@inktank.com>
Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c
index 478ab6164f5ad7da020f93f982d5cf62d7035a38..bfdd8f5fa340b4faf519132aa9afc4d510c4771d 100644 (file)
--- a/drivers/block/rbd.c
+++ b/drivers/block/rbd.c
@@ -5110,23 +5110,6 @@ err_out_module:
        return (ssize_t)rc;
 }
 
-static struct rbd_device *__rbd_get_dev(unsigned long dev_id)
-{
-       struct list_head *tmp;
-       struct rbd_device *rbd_dev;
-
-       spin_lock(&rbd_dev_list_lock);
-       list_for_each(tmp, &rbd_dev_list) {
-               rbd_dev = list_entry(tmp, struct rbd_device, node);
-               if (rbd_dev->dev_id == dev_id) {
-                       spin_unlock(&rbd_dev_list_lock);
-                       return rbd_dev;
-               }
-       }
-       spin_unlock(&rbd_dev_list_lock);
-       return NULL;
-}
-
 static void rbd_dev_device_release(struct device *dev)
 {
        struct rbd_device *rbd_dev = dev_to_rbd_dev(dev);
@@ -5171,7 +5154,8 @@ static ssize_t rbd_remove(struct bus_type *bus,
                          size_t count)
 {
        struct rbd_device *rbd_dev = NULL;
-       int target_id;
+       struct list_head *tmp;
+       int dev_id;
        unsigned long ul;
        int ret;
 
@@ -5180,26 +5164,33 @@ static ssize_t rbd_remove(struct bus_type *bus,
                return ret;
 
        /* convert to int; abort if we lost anything in the conversion */
-       target_id = (int) ul;
-       if (target_id != ul)
+       dev_id = (int)ul;
+       if (dev_id != ul)
                return -EINVAL;
 
        mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING);
 
-       rbd_dev = __rbd_get_dev(target_id);
-       if (!rbd_dev) {
-               ret = -ENOENT;
-               goto done;
+       ret = -ENOENT;
+       spin_lock(&rbd_dev_list_lock);
+       list_for_each(tmp, &rbd_dev_list) {
+               rbd_dev = list_entry(tmp, struct rbd_device, node);
+               if (rbd_dev->dev_id == dev_id) {
+                       ret = 0;
+                       break;
+               }
        }
-
-       spin_lock_irq(&rbd_dev->lock);
-       if (rbd_dev->open_count)
-               ret = -EBUSY;
-       else
-               set_bit(RBD_DEV_FLAG_REMOVING, &rbd_dev->flags);
-       spin_unlock_irq(&rbd_dev->lock);
+       if (!ret) {
+               spin_lock_irq(&rbd_dev->lock);
+               if (rbd_dev->open_count)
+                       ret = -EBUSY;
+               else
+                       set_bit(RBD_DEV_FLAG_REMOVING, &rbd_dev->flags);
+               spin_unlock_irq(&rbd_dev->lock);
+       }
+       spin_unlock(&rbd_dev_list_lock);
        if (ret < 0)
                goto done;
+
        rbd_bus_del_dev(rbd_dev);
        ret = rbd_dev_header_watch_sync(rbd_dev, false);
        if (ret)