| Message ID | 51A94C63.8060104@inktank.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Reviewed-by: Josh Durgin <josh.durgin@inktank.com> Alex Elder <elder@inktank.com> wrote: >When an rbd device is first getting mapped, its device registration >is protected the control mutex. There is no need to do that though, >because the device has already been assigned an id that's guaranteed >to be unique. > >An unmap of an rbd device won't proceed if the device has a non-zero >open count or is already being unmapped. So there's no need to hold >the control mutex in that case either. > >Finally, an rbd device can't be opened if it is being removed, and >it won't go away if there is a non-zero open count. So here too >there's no need to hold the control mutex while getting or putting a >reference to an rbd device's Linux device structure. > >Drop the mutex calls in these cases. > >Signed-off-by: Alex Elder <elder@inktank.com> >--- > drivers/block/rbd.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > >diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c >index 380940d..9c81a5c 100644 >--- a/drivers/block/rbd.c >+++ b/drivers/block/rbd.c >@@ -489,10 +489,8 @@ static int rbd_open(struct block_device *bdev, >fmode_t mode) > if (removing) > return -ENOENT; > >- mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); > (void) get_device(&rbd_dev->dev); > set_device_ro(bdev, rbd_dev->mapping.read_only); >- mutex_unlock(&ctl_mutex); > > return 0; > } >@@ -507,9 +505,7 @@ static int rbd_release(struct gendisk *disk, >fmode_t >mode) > spin_unlock_irq(&rbd_dev->lock); > rbd_assert(open_count_before > 0); > >- mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); > put_device(&rbd_dev->dev); >- mutex_unlock(&ctl_mutex); > > return 0; > } >@@ -4348,8 +4344,6 @@ static int rbd_bus_add_dev(struct rbd_device >*rbd_dev) > struct device *dev; > int ret; > >- mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); >- > dev = &rbd_dev->dev; > dev->bus = &rbd_bus_type; > dev->type = &rbd_device_type; >@@ -4358,8 +4352,6 @@ static int rbd_bus_add_dev(struct rbd_device >*rbd_dev) > dev_set_name(dev, "%d", rbd_dev->dev_id); > ret = device_register(dev); > >- mutex_unlock(&ctl_mutex); >- > return ret; > } > >@@ -5165,8 +5157,6 @@ static ssize_t rbd_remove(struct bus_type *bus, > if (dev_id != ul) > return -EINVAL; > >- mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); >- > ret = -ENOENT; > spin_lock(&rbd_dev_list_lock); > list_for_each(tmp, &rbd_dev_list) { >@@ -5187,7 +5177,7 @@ static ssize_t rbd_remove(struct bus_type *bus, > } > spin_unlock(&rbd_dev_list_lock); > if (ret < 0 || already) >- goto done; >+ return ret; > > rbd_bus_del_dev(rbd_dev); > ret = rbd_dev_header_watch_sync(rbd_dev, false); >@@ -5195,11 +5185,8 @@ static ssize_t rbd_remove(struct bus_type *bus, > rbd_warn(rbd_dev, "failed to cancel watch event (%d)\n", ret); > rbd_dev_image_release(rbd_dev); > module_put(THIS_MODULE); >- ret = count; >-done: >- mutex_unlock(&ctl_mutex); > >- return ret; >+ return count; > } > > /* -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 380940d..9c81a5c 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -489,10 +489,8 @@ static int rbd_open(struct block_device *bdev, fmode_t mode) if (removing) return -ENOENT; - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); (void) get_device(&rbd_dev->dev); set_device_ro(bdev, rbd_dev->mapping.read_only); - mutex_unlock(&ctl_mutex); return 0;
When an rbd device is first getting mapped, its device registration is protected the control mutex. There is no need to do that though, because the device has already been assigned an id that's guaranteed to be unique. An unmap of an rbd device won't proceed if the device has a non-zero open count or is already being unmapped. So there's no need to hold the control mutex in that case either. Finally, an rbd device can't be opened if it is being removed, and it won't go away if there is a non-zero open count. So here too there's no need to hold the control mutex while getting or putting a reference to an rbd device's Linux device structure. Drop the mutex calls in these cases. Signed-off-by: Alex Elder <elder@inktank.com> --- drivers/block/rbd.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) } @@ -507,9 +505,7 @@ static int rbd_release(struct gendisk *disk, fmode_t mode) spin_unlock_irq(&rbd_dev->lock); rbd_assert(open_count_before > 0); - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); put_device(&rbd_dev->dev); - mutex_unlock(&ctl_mutex); return 0; } @@ -4348,8 +4344,6 @@ static int rbd_bus_add_dev(struct rbd_device *rbd_dev) struct device *dev; int ret; - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); - dev = &rbd_dev->dev; dev->bus = &rbd_bus_type; dev->type = &rbd_device_type; @@ -4358,8 +4352,6 @@ static int rbd_bus_add_dev(struct rbd_device *rbd_dev) dev_set_name(dev, "%d", rbd_dev->dev_id); ret = device_register(dev); - mutex_unlock(&ctl_mutex); - return ret; } @@ -5165,8 +5157,6 @@ static ssize_t rbd_remove(struct bus_type *bus, if (dev_id != ul) return -EINVAL; - mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); - ret = -ENOENT; spin_lock(&rbd_dev_list_lock); list_for_each(tmp, &rbd_dev_list) { @@ -5187,7 +5177,7 @@ static ssize_t rbd_remove(struct bus_type *bus, } spin_unlock(&rbd_dev_list_lock); if (ret < 0 || already) - goto done; + return ret; rbd_bus_del_dev(rbd_dev); ret = rbd_dev_header_watch_sync(rbd_dev, false); @@ -5195,11 +5185,8 @@ static ssize_t rbd_remove(struct bus_type *bus, rbd_warn(rbd_dev, "failed to cancel watch event (%d)\n", ret); rbd_dev_image_release(rbd_dev); module_put(THIS_MODULE); - ret = count; -done: - mutex_unlock(&ctl_mutex); - return ret; + return count; } /*