iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

rbd: fix error handling around rbd_init_disk()

Browse Source 
add_disk() takes an extra reference on disk->queue, which is put in
put_disk() -> disk_release().  Avoiding blk_cleanup_queue() (which also
puts the queue) until add_disk() sets GENHD_FL_UP works for the queue
itself, but leaks various queue internals.  Conditioning tag_set freeing
on GENHD_FL_UP is wrong too: all error paths after rbd_init_disk() leak
the tag_set.

Move the final "announce" steps out of rbd_dev_device_setup() so that
it can be unwound like any other function.  Leave "announce" steps to
do_rbd_add/remove().

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Jason Dillaman <dillaman@redhat.com>
This commit is contained in:
Ilya Dryomov 2017-04-13 12:17:38 +02:00
parent fd22aef8b4
commit 5769ed0cb1
1 changed files with 44 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

87
drivers/block/rbd.c
View File

@ -4114,19 +4114,10 @@ static int rbd_queue_rq(struct blk_mq_hw_ctx *hctx,
static void rbd_free_disk(struct rbd_device *rbd_dev) static void rbd_free_disk(struct rbd_device *rbd_dev)
{ {
struct gendisk *disk = rbd_dev->disk; blk_cleanup_queue(rbd_dev->disk->queue);
if (!disk)
return;
rbd_dev->disk = NULL;
if (disk->flags & GENHD_FL_UP) {
del_gendisk(disk);
if (disk->queue)
blk_cleanup_queue(disk->queue);
blk_mq_free_tag_set(&rbd_dev->tag_set); blk_mq_free_tag_set(&rbd_dev->tag_set);
} put_disk(rbd_dev->disk);
put_disk(disk); rbd_dev->disk = NULL;
} }
static int rbd_obj_read_sync(struct rbd_device *rbd_dev, static int rbd_obj_read_sync(struct rbd_device *rbd_dev,
@ -4385,8 +4376,12 @@ static int rbd_init_disk(struct rbd_device *rbd_dev)
if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC)) if (!ceph_test_opt(rbd_dev->rbd_client->client, NOCRC))
q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES; q->backing_dev_info->capabilities |= BDI_CAP_STABLE_WRITES;
/*
* disk_release() expects a queue ref from add_disk() and will
* put it. Hold an extra ref until add_disk() is called.
*/
WARN_ON(!blk_get_queue(q));
disk->queue = q; disk->queue = q;
q->queuedata = rbd_dev; q->queuedata = rbd_dev;
rbd_dev->disk = disk; rbd_dev->disk = disk;
@ -5875,6 +5870,15 @@ out_err:
return ret; return ret;
} }
static void rbd_dev_device_release(struct rbd_device *rbd_dev)
{
clear_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
rbd_dev_mapping_clear(rbd_dev);
rbd_free_disk(rbd_dev);
if (!single_major)
unregister_blkdev(rbd_dev->major, rbd_dev->name);
}
/* /*
* rbd_dev->header_rwsem must be locked for write and will be unlocked * rbd_dev->header_rwsem must be locked for write and will be unlocked
* upon return. * upon return.
@ -5910,26 +5914,13 @@ static int rbd_dev_device_setup(struct rbd_device *rbd_dev)
set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE); set_capacity(rbd_dev->disk, rbd_dev->mapping.size / SECTOR_SIZE);
set_disk_ro(rbd_dev->disk, rbd_dev->mapping.read_only); set_disk_ro(rbd_dev->disk, rbd_dev->mapping.read_only);
dev_set_name(&rbd_dev->dev, "%d", rbd_dev->dev_id); ret = dev_set_name(&rbd_dev->dev, "%d", rbd_dev->dev_id);
ret = device_add(&rbd_dev->dev);
if (ret) if (ret)
goto err_out_mapping; goto err_out_mapping;
/* Everything's ready. Announce the disk to the world. */
set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags); set_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
up_write(&rbd_dev->header_rwsem); up_write(&rbd_dev->header_rwsem);
return 0;
spin_lock(&rbd_dev_list_lock);
list_add_tail(&rbd_dev->node, &rbd_dev_list);
spin_unlock(&rbd_dev_list_lock);
add_disk(rbd_dev->disk);
pr_info("%s: capacity %llu features 0x%llx\n", rbd_dev->disk->disk_name,
(unsigned long long)get_capacity(rbd_dev->disk) << SECTOR_SHIFT,
rbd_dev->header.features);
return ret;
err_out_mapping: err_out_mapping:
rbd_dev_mapping_clear(rbd_dev); rbd_dev_mapping_clear(rbd_dev);
@ -6131,11 +6122,30 @@ static ssize_t do_rbd_add(struct bus_type *bus,
if (rc) if (rc)
goto err_out_image_probe; goto err_out_image_probe;
/* Everything's ready. Announce the disk to the world. */
rc = device_add(&rbd_dev->dev);
if (rc)
goto err_out_device_setup;
add_disk(rbd_dev->disk);
/* see rbd_init_disk() */
blk_put_queue(rbd_dev->disk->queue);
spin_lock(&rbd_dev_list_lock);
list_add_tail(&rbd_dev->node, &rbd_dev_list);
spin_unlock(&rbd_dev_list_lock);
pr_info("%s: capacity %llu features 0x%llx\n", rbd_dev->disk->disk_name,
(unsigned long long)get_capacity(rbd_dev->disk) << SECTOR_SHIFT,
rbd_dev->header.features);
rc = count; rc = count;
out: out:
module_put(THIS_MODULE); module_put(THIS_MODULE);
return rc; return rc;
err_out_device_setup:
rbd_dev_device_release(rbd_dev);
err_out_image_probe: err_out_image_probe:
rbd_dev_image_release(rbd_dev); rbd_dev_image_release(rbd_dev);
err_out_rbd_dev: err_out_rbd_dev:
@ -6165,21 +6175,6 @@ static ssize_t rbd_add_single_major(struct bus_type *bus,
return do_rbd_add(bus, buf, count); return do_rbd_add(bus, buf, count);
} }
static void rbd_dev_device_release(struct rbd_device *rbd_dev)
{
rbd_free_disk(rbd_dev);
spin_lock(&rbd_dev_list_lock);
list_del_init(&rbd_dev->node);
spin_unlock(&rbd_dev_list_lock);
clear_bit(RBD_DEV_FLAG_EXISTS, &rbd_dev->flags);
device_del(&rbd_dev->dev);
rbd_dev_mapping_clear(rbd_dev);
if (!single_major)
unregister_blkdev(rbd_dev->major, rbd_dev->name);
}
static void rbd_dev_remove_parent(struct rbd_device *rbd_dev) static void rbd_dev_remove_parent(struct rbd_device *rbd_dev)
{ {
while (rbd_dev->parent) { while (rbd_dev->parent) {
@ -6266,6 +6261,12 @@ static ssize_t do_rbd_remove(struct bus_type *bus,
blk_set_queue_dying(rbd_dev->disk->queue); blk_set_queue_dying(rbd_dev->disk->queue);
} }
del_gendisk(rbd_dev->disk);
spin_lock(&rbd_dev_list_lock);
list_del_init(&rbd_dev->node);
spin_unlock(&rbd_dev_list_lock);
device_del(&rbd_dev->dev);
down_write(&rbd_dev->lock_rwsem); down_write(&rbd_dev->lock_rwsem);
if (__rbd_is_lock_owner(rbd_dev)) if (__rbd_is_lock_owner(rbd_dev))
rbd_unlock(rbd_dev); rbd_unlock(rbd_dev);