diff options
author | NeilBrown <neilb@suse.de> | 2009-06-30 22:27:21 -0400 |
---|---|---|
committer | NeilBrown <neilb@suse.de> | 2009-06-30 22:27:21 -0400 |
commit | 0909dc448c98ed5021c87ffdfc09fb473aa464ab (patch) | |
tree | 17c48d146c0f9eb7367a0d0feafea29838eae8eb /drivers/md/md.c | |
parent | 1ec22eb2b4a2e1a763106bce36b11c02eaa84e61 (diff) |
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) | |||
3846 | flush_scheduled_work(); | 3846 | flush_scheduled_work(); |
3847 | 3847 | ||
3848 | mutex_lock(&disks_mutex); | 3848 | mutex_lock(&disks_mutex); |
3849 | if (mddev->gendisk) { | 3849 | error = -EEXIST; |
3850 | mutex_unlock(&disks_mutex); | 3850 | if (mddev->gendisk) |
3851 | mddev_put(mddev); | 3851 | goto abort; |
3852 | return -EEXIST; | ||
3853 | } | ||
3854 | 3852 | ||
3855 | if (name) { | 3853 | if (name) { |
3856 | /* Need to ensure that 'name' is not a duplicate. | 3854 | /* Need to ensure that 'name' is not a duplicate. |
@@ -3862,19 +3860,15 @@ static int md_alloc(dev_t dev, char *name) | |||
3862 | if (mddev2->gendisk && | 3860 | if (mddev2->gendisk && |
3863 | strcmp(mddev2->gendisk->disk_name, name) == 0) { | 3861 | strcmp(mddev2->gendisk->disk_name, name) == 0) { |
3864 | spin_unlock(&all_mddevs_lock); | 3862 | spin_unlock(&all_mddevs_lock); |
3865 | mutex_unlock(&disks_mutex); | 3863 | goto abort; |
3866 | mddev_put(mddev); | ||
3867 | return -EEXIST; | ||
3868 | } | 3864 | } |
3869 | spin_unlock(&all_mddevs_lock); | 3865 | spin_unlock(&all_mddevs_lock); |
3870 | } | 3866 | } |
3871 | 3867 | ||
3868 | error = -ENOMEM; | ||
3872 | mddev->queue = blk_alloc_queue(GFP_KERNEL); | 3869 | mddev->queue = blk_alloc_queue(GFP_KERNEL); |
3873 | if (!mddev->queue) { | 3870 | if (!mddev->queue) |
3874 | mutex_unlock(&disks_mutex); | 3871 | goto abort; |
3875 | mddev_put(mddev); | ||
3876 | return -ENOMEM; | ||
3877 | } | ||
3878 | mddev->queue->queuedata = mddev; | 3872 | mddev->queue->queuedata = mddev; |
3879 | 3873 | ||
3880 | /* Can be unlocked because the queue is new: no concurrency */ | 3874 | /* Can be unlocked because the queue is new: no concurrency */ |
@@ -3884,11 +3878,9 @@ static int md_alloc(dev_t dev, char *name) | |||
3884 | 3878 | ||
3885 | disk = alloc_disk(1 << shift); | 3879 | disk = alloc_disk(1 << shift); |
3886 | if (!disk) { | 3880 | if (!disk) { |
3887 | mutex_unlock(&disks_mutex); | ||
3888 | blk_cleanup_queue(mddev->queue); | 3881 | blk_cleanup_queue(mddev->queue); |
3889 | mddev->queue = NULL; | 3882 | mddev->queue = NULL; |
3890 | mddev_put(mddev); | 3883 | goto abort; |
3891 | return -ENOMEM; | ||
3892 | } | 3884 | } |
3893 | disk->major = MAJOR(mddev->unit); | 3885 | disk->major = MAJOR(mddev->unit); |
3894 | disk->first_minor = unit << shift; | 3886 | disk->first_minor = unit << shift; |
@@ -3910,16 +3902,22 @@ static int md_alloc(dev_t dev, char *name) | |||
3910 | mddev->gendisk = disk; | 3902 | mddev->gendisk = disk; |
3911 | error = kobject_init_and_add(&mddev->kobj, &md_ktype, | 3903 | error = kobject_init_and_add(&mddev->kobj, &md_ktype, |
3912 | &disk_to_dev(disk)->kobj, "%s", "md"); | 3904 | &disk_to_dev(disk)->kobj, "%s", "md"); |
3913 | mutex_unlock(&disks_mutex); | 3905 | if (error) { |
3914 | if (error) | 3906 | /* This isn't possible, but as kobject_init_and_add is marked |
3907 | * __must_check, we must do something with the result | ||
3908 | */ | ||
3915 | printk(KERN_WARNING "md: cannot register %s/md - name in use\n", | 3909 | printk(KERN_WARNING "md: cannot register %s/md - name in use\n", |
3916 | disk->disk_name); | 3910 | disk->disk_name); |
3917 | else { | 3911 | error = 0; |
3912 | } | ||
3913 | abort: | ||
3914 | mutex_unlock(&disks_mutex); | ||
3915 | if (!error) { | ||
3918 | kobject_uevent(&mddev->kobj, KOBJ_ADD); | 3916 | kobject_uevent(&mddev->kobj, KOBJ_ADD); |
3919 | mddev->sysfs_state = sysfs_get_dirent(mddev->kobj.sd, "array_state"); | 3917 | mddev->sysfs_state = sysfs_get_dirent(mddev->kobj.sd, "array_state"); |
3920 | } | 3918 | } |
3921 | mddev_put(mddev); | 3919 | mddev_put(mddev); |
3922 | return 0; | 3920 | return error; |
3923 | } | 3921 | } |
3924 | 3922 | ||
3925 | static struct kobject *md_probe(dev_t dev, int *part, void *data) | 3923 | static struct kobject *md_probe(dev_t dev, int *part, void *data) |