diff options
author | NeilBrown <neilb@suse.de> | 2009-07-01 12:27:21 +1000 |
---|---|---|
committer | NeilBrown <neilb@suse.de> | 2009-07-01 12:27:21 +1000 |
commit | 0909dc448c98ed5021c87ffdfc09fb473aa464ab (patch) | |
tree | 17c48d146c0f9eb7367a0d0feafea29838eae8eb /drivers/md/md.c | |
parent | 1ec22eb2b4a2e1a763106bce36b11c02eaa84e61 (diff) | |
download | linux-0909dc448c98ed5021c87ffdfc09fb473aa464ab.tar.gz linux-0909dc448c98ed5021c87ffdfc09fb473aa464ab.tar.bz2 linux-0909dc448c98ed5021c87ffdfc09fb473aa464ab.zip |
md: tidy up error paths in md_alloc
As the recent bug in md_alloc showed, having a single exit path for
unlocking and putting is a good idea. So restructure md_alloc to have
a single mutex_unlock and mddev_put, and use gotos where necessary.
Found-by: Jiri Slaby <jirislaby@gmail.com>
Signed-off-by: NeilBrown <neilb@suse.de>
Diffstat (limited to 'drivers/md/md.c')
-rw-r--r-- | drivers/md/md.c | 38 |
1 files changed, 18 insertions, 20 deletions
diff --git a/drivers/md/md.c b/drivers/md/md.c index 58bee2366ea8..65fe35b5e34a 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -3846,11 +3846,9 @@ static int md_alloc(dev_t dev, char *name) flush_scheduled_work(); mutex_lock(&disks_mutex); - if (mddev->gendisk) { - mutex_unlock(&disks_mutex); - mddev_put(mddev); - return -EEXIST; - } + error = -EEXIST; + if (mddev->gendisk) + goto abort; if (name) { /* Need to ensure that 'name' is not a duplicate. @@ -3862,19 +3860,15 @@ static int md_alloc(dev_t dev, char *name) if (mddev2->gendisk && strcmp(mddev2->gendisk->disk_name, name) == 0) { spin_unlock(&all_mddevs_lock); - mutex_unlock(&disks_mutex); - mddev_put(mddev); - return -EEXIST; + goto abort; } spin_unlock(&all_mddevs_lock); } + error = -ENOMEM; mddev->queue = blk_alloc_queue(GFP_KERNEL); - if (!mddev->queue) { - mutex_unlock(&disks_mutex); - mddev_put(mddev); - return -ENOMEM; - } + if (!mddev->queue) + goto abort; mddev->queue->queuedata = mddev; /* Can be unlocked because the queue is new: no concurrency */ @@ -3884,11 +3878,9 @@ static int md_alloc(dev_t dev, char *name) disk = alloc_disk(1 << shift); if (!disk) { - mutex_unlock(&disks_mutex); blk_cleanup_queue(mddev->queue); mddev->queue = NULL; - mddev_put(mddev); - return -ENOMEM; + goto abort; } disk->major = MAJOR(mddev->unit); disk->first_minor = unit << shift; @@ -3910,16 +3902,22 @@ static int md_alloc(dev_t dev, char *name) mddev->gendisk = disk; error = kobject_init_and_add(&mddev->kobj, &md_ktype, &disk_to_dev(disk)->kobj, "%s", "md"); - mutex_unlock(&disks_mutex); - if (error) + if (error) { + /* This isn't possible, but as kobject_init_and_add is marked + * __must_check, we must do something with the result + */ printk(KERN_WARNING "md: cannot register %s/md - name in use\n", disk->disk_name); - else { + error = 0; + } + abort: + mutex_unlock(&disks_mutex); + if (!error) { kobject_uevent(&mddev->kobj, KOBJ_ADD); mddev->sysfs_state = sysfs_get_dirent(mddev->kobj.sd, "array_state"); } mddev_put(mddev); - return 0; + return error; } static struct kobject *md_probe(dev_t dev, int *part, void *data) |