Skip to content

Commit

Permalink
block: add error handling for device_add_disk / add_disk
Browse files Browse the repository at this point in the history
Properly unwind on errors in device_add_disk.  This is the initial work
as drivers are not converted yet, which will follow in separate patches.

Signed-off-by: Luis Chamberlain <[email protected]>
[hch: major rebase.  All bugs are probably mine]
Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Jens Axboe <[email protected]>
  • Loading branch information
mcgrof authored and axboe committed Aug 23, 2021
1 parent 92e7755 commit 83cbce9
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 38 deletions.
92 changes: 58 additions & 34 deletions block/genhd.c
Original file line number Diff line number Diff line change
Expand Up @@ -417,11 +417,8 @@ static void disk_scan_partitions(struct gendisk *disk)
*
* This function registers the partitioning information in @disk
* with the kernel.
*
* FIXME: error handling
*/

void device_add_disk(struct device *parent, struct gendisk *disk,
int device_add_disk(struct device *parent, struct gendisk *disk,
const struct attribute_group **groups)

{
Expand All @@ -444,27 +441,29 @@ void device_add_disk(struct device *parent, struct gendisk *disk,
* and all partitions from the extended dev_t space.
*/
if (disk->major) {
WARN_ON(!disk->minors);
if (WARN_ON(!disk->minors))
return -EINVAL;

if (disk->minors > DISK_MAX_PARTS) {
pr_err("block: can't allocate more than %d partitions\n",
DISK_MAX_PARTS);
disk->minors = DISK_MAX_PARTS;
}
} else {
WARN_ON(disk->minors);
if (WARN_ON(disk->minors))
return -EINVAL;

ret = blk_alloc_ext_minor();
if (ret < 0) {
WARN_ON(1);
return;
}
if (ret < 0)
return ret;
disk->major = BLOCK_EXT_MAJOR;
disk->first_minor = MINOR(ret);
disk->flags |= GENHD_FL_EXT_DEVT;
}

disk_alloc_events(disk);
ret = disk_alloc_events(disk);
if (ret)
goto out_free_ext_minor;

/* delay uevents, until we scanned partition table */
dev_set_uevent_suppress(ddev, 1);
Expand All @@ -474,15 +473,14 @@ void device_add_disk(struct device *parent, struct gendisk *disk,
dev_set_name(ddev, "%s", disk->disk_name);
if (!(disk->flags & GENHD_FL_HIDDEN))
ddev->devt = MKDEV(disk->major, disk->first_minor);
if (device_add(ddev))
return;
ret = device_add(ddev);
if (ret)
goto out_disk_release_events;
if (!sysfs_deprecated) {
ret = sysfs_create_link(block_depr, &ddev->kobj,
kobject_name(&ddev->kobj));
if (ret) {
device_del(ddev);
return;
}
if (ret)
goto out_device_del;
}

/*
Expand All @@ -492,23 +490,25 @@ void device_add_disk(struct device *parent, struct gendisk *disk,
*/
pm_runtime_set_memalloc_noio(ddev, true);

blk_integrity_add(disk);
ret = blk_integrity_add(disk);
if (ret)
goto out_del_block_link;

disk->part0->bd_holder_dir =
kobject_create_and_add("holders", &ddev->kobj);
if (!disk->part0->bd_holder_dir)
goto out_del_integrity;
disk->slave_dir = kobject_create_and_add("slaves", &ddev->kobj);
if (!disk->slave_dir)
goto out_put_holder_dir;

/*
* XXX: this is a mess, can't wait for real error handling in add_disk.
* Make sure ->slave_dir is NULL if we failed some of the registration
* so that the cleanup in bd_unlink_disk_holder works properly.
*/
if (bd_register_pending_holders(disk) < 0) {
kobject_put(disk->slave_dir);
disk->slave_dir = NULL;
}
ret = bd_register_pending_holders(disk);
if (ret < 0)
goto out_put_slave_dir;

blk_register_queue(disk);
ret = blk_register_queue(disk);
if (ret)
goto out_put_slave_dir;

if (disk->flags & GENHD_FL_HIDDEN) {
/*
Expand All @@ -520,13 +520,13 @@ void device_add_disk(struct device *parent, struct gendisk *disk,
} else {
ret = bdi_register(disk->bdi, "%u:%u",
disk->major, disk->first_minor);
WARN_ON(ret);
if (ret)
goto out_unregister_queue;
bdi_set_owner(disk->bdi, ddev);
if (disk->bdi->dev) {
ret = sysfs_create_link(&ddev->kobj,
&disk->bdi->dev->kobj, "bdi");
WARN_ON(ret);
}
ret = sysfs_create_link(&ddev->kobj,
&disk->bdi->dev->kobj, "bdi");
if (ret)
goto out_unregister_bdi;

bdev_add(disk->part0, ddev->devt);
disk_scan_partitions(disk);
Expand All @@ -541,6 +541,30 @@ void device_add_disk(struct device *parent, struct gendisk *disk,

disk_update_readahead(disk);
disk_add_events(disk);
return 0;

out_unregister_bdi:
if (!(disk->flags & GENHD_FL_HIDDEN))
bdi_unregister(disk->bdi);
out_unregister_queue:
blk_unregister_queue(disk);
out_put_slave_dir:
kobject_put(disk->slave_dir);
out_put_holder_dir:
kobject_put(disk->part0->bd_holder_dir);
out_del_integrity:
blk_integrity_del(disk);
out_del_block_link:
if (!sysfs_deprecated)
sysfs_remove_link(block_depr, dev_name(ddev));
out_device_del:
device_del(ddev);
out_disk_release_events:
disk_release_events(disk);
out_free_ext_minor:
if (disk->major == BLOCK_EXT_MAJOR)
blk_free_ext_minor(disk->first_minor);
return WARN_ON_ONCE(ret); /* keep until all callers handle errors */
}
EXPORT_SYMBOL(device_add_disk);

Expand Down
8 changes: 4 additions & 4 deletions include/linux/genhd.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,11 @@ static inline dev_t disk_devt(struct gendisk *disk)
void disk_uevent(struct gendisk *disk, enum kobject_action action);

/* block/genhd.c */
extern void device_add_disk(struct device *parent, struct gendisk *disk,
const struct attribute_group **groups);
static inline void add_disk(struct gendisk *disk)
int device_add_disk(struct device *parent, struct gendisk *disk,
const struct attribute_group **groups);
static inline int add_disk(struct gendisk *disk)
{
device_add_disk(NULL, disk, NULL);
return device_add_disk(NULL, disk, NULL);
}
extern void del_gendisk(struct gendisk *gp);

Expand Down

0 comments on commit 83cbce9

Please sign in to comment.