[PATCH v2 5/6] block: add initial error handling for *add_disk()* and friends

From: Luis Chamberlain
Date: Thu Jul 15 2021 - 00:55:51 EST

This adds error handling to the *add_disk*() callers and the functions
it depends on. This is initial work as drivers are not converted. That
is separate work.

Signed-off-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
---
block/blk-integrity.c | 12 +++--
block/blk-sysfs.c | 5 +-
block/blk.h | 7 +--
block/disk-events.c | 8 +--
block/genhd.c | 118 ++++++++++++++++++++++++++++++------------
include/linux/genhd.h | 14 ++---
6 files changed, 112 insertions(+), 52 deletions(-)

diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index 410da060d1f5..e46f47f2dec9 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -431,13 +431,17 @@ void blk_integrity_unregister(struct gendisk *disk)
}
EXPORT_SYMBOL(blk_integrity_unregister);

-void blk_integrity_add(struct gendisk *disk)
+int blk_integrity_add(struct gendisk *disk)
{
- if (kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
- &disk_to_dev(disk)->kobj, "%s", "integrity"))
- return;
+ int ret;
+
+ ret = kobject_init_and_add(&disk->integrity_kobj, &integrity_ktype,
+ &disk_to_dev(disk)->kobj, "%s", "integrity");
+ if (ret)
+ return ret;

kobject_uevent(&disk->integrity_kobj, KOBJ_ADD);
+ return 0;
}

void blk_integrity_del(struct gendisk *disk)
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 370d83c18057..7fd53bf3f8ad 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -862,9 +862,10 @@ int blk_register_queue(struct gendisk *disk)
if (WARN_ON(!q))
return -ENXIO;

- WARN_ONCE(blk_queue_registered(q),
+ if (WARN_ONCE(blk_queue_registered(q),
"%s is registering an already registered queue\n",
- kobject_name(&dev->kobj));
+ kobject_name(&dev->kobj)))
+ return -ENXIO;

blk_queue_update_readahead(q);

diff --git a/block/blk.h b/block/blk.h
index 4b885c0f6708..e24273ebc4bd 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -132,7 +132,7 @@ static inline bool integrity_req_gap_front_merge(struct request *req,
bip_next->bip_vec[0].bv_offset);
}

-void blk_integrity_add(struct gendisk *);
+int blk_integrity_add(struct gendisk *);
void blk_integrity_del(struct gendisk *);
#else /* CONFIG_BLK_DEV_INTEGRITY */
static inline bool blk_integrity_merge_rq(struct request_queue *rq,
@@ -166,8 +166,9 @@ static inline bool bio_integrity_endio(struct bio *bio)
static inline void bio_integrity_free(struct bio *bio)
{
}
-static inline void blk_integrity_add(struct gendisk *disk)
+static inline int blk_integrity_add(struct gendisk *disk)
{
+ return 0;
}
static inline void blk_integrity_del(struct gendisk *disk)
{
@@ -360,7 +361,7 @@ int bio_add_hw_page(struct request_queue *q, struct bio *bio,

struct request_queue *blk_alloc_queue(int node_id);

-void disk_alloc_events(struct gendisk *disk);
+int disk_alloc_events(struct gendisk *disk);
void disk_add_events(struct gendisk *disk);
void disk_del_events(struct gendisk *disk);
void disk_release_events(struct gendisk *disk);
diff --git a/block/disk-events.c b/block/disk-events.c
index a75931ff5da4..0262784be34c 100644
--- a/block/disk-events.c
+++ b/block/disk-events.c
@@ -410,17 +410,17 @@ module_param_cb(events_dfl_poll_msecs, &disk_events_dfl_poll_msecs_param_ops,
/*
* disk_{alloc|add|del|release}_events - initialize and destroy disk_events.
*/
-void disk_alloc_events(struct gendisk *disk)
+int disk_alloc_events(struct gendisk *disk)
{
struct disk_events *ev;

if (!disk->fops->check_events || !disk->events)
- return;
+ return 0;

ev = kzalloc(sizeof(*ev), GFP_KERNEL);
if (!ev) {
pr_warn("%s: failed to initialize events\n", disk->disk_name);
- return;
+ return -ENOMEM;
}

INIT_LIST_HEAD(&ev->node);
@@ -432,6 +432,8 @@ void disk_alloc_events(struct gendisk *disk)
INIT_DELAYED_WORK(&ev->dwork, disk_events_workfn);

disk->ev = ev;
+
+ return 0;
}

void disk_add_events(struct gendisk *disk)
diff --git a/block/genhd.c b/block/genhd.c
index b84ba22eed39..c6c9c196ff27 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -449,8 +449,9 @@ static void unregister_disk(struct gendisk *disk)
disk_invalidate(disk);
}

-static void register_disk(struct device *parent, struct gendisk *disk,
- const struct attribute_group **groups)
+static int __must_check register_disk(struct device *parent,
+ struct gendisk *disk,
+ const struct attribute_group **groups)
{
struct device *ddev = disk_to_dev(disk);
int err;
@@ -466,15 +467,21 @@ static void register_disk(struct device *parent, struct gendisk *disk,
WARN_ON(ddev->groups);
ddev->groups = groups;
}
- if (device_add(ddev))
- return;
+ err = device_add(ddev);
+ if (err) {
+ /*
+ * We don't put_device(ddev), the driver is responsible to
+ * issue the last put_device(ddev), however it does this
+ * inderectly with put_disk(), or typically more often with
+ * just blk_cleanup_disk().
+ */
+ return err;
+ }
if (!sysfs_deprecated) {
err = sysfs_create_link(block_depr, &ddev->kobj,
kobject_name(&ddev->kobj));
- if (err) {
- device_del(ddev);
- return;
- }
+ if (err)
+ goto exit_del_device;
}

/*
@@ -489,7 +496,7 @@ static void register_disk(struct device *parent, struct gendisk *disk,
disk->slave_dir = kobject_create_and_add("slaves", &ddev->kobj);

if (disk->flags & GENHD_FL_HIDDEN)
- return;
+ return 0;

disk_scan_partitions(disk);
disk_announce(disk);
@@ -498,8 +505,19 @@ static void register_disk(struct device *parent, struct gendisk *disk,
err = sysfs_create_link(&ddev->kobj,
&disk->queue->backing_dev_info->dev->kobj,
"bdi");
- WARN_ON(err);
+ if (WARN_ON(err))
+ goto exit_del_block_depr;
}
+ return 0;
+
+exit_del_block_depr:
+ unregister_disk_partitions(disk);
+ if (!sysfs_deprecated)
+ sysfs_remove_link(block_depr, dev_name(disk_to_dev(disk)));
+exit_del_device:
+ device_del(ddev);
+
+ return err;
}

/**
@@ -512,22 +530,24 @@ static void register_disk(struct device *parent, struct gendisk *disk,
* This function registers the partitioning information in @disk
* with the kernel.
*
- * FIXME: error handling
*/
-static void __device_add_disk(struct device *parent, struct gendisk *disk,
- const struct attribute_group **groups,
- bool register_queue)
+static int __device_add_disk(struct device *parent, struct gendisk *disk,
+ const struct attribute_group **groups,
+ bool register_queue)
{
int ret;

/*
* Take an extra ref on queue which will be put on disk_release()
- * so that it sticks around as long as @disk is there.
+ * so that it sticks around as long as @disk is there. The driver
+ * must call blk_cleanup_disk() on error from this function.
*/
- if (blk_get_queue(disk->queue))
- set_bit(GD_QUEUE_REF, &disk->state);
- else
- WARN_ON_ONCE(1);
+ if (WARN_ON_ONCE(!blk_get_queue(disk->queue))) {
+ disk->queue = NULL;
+ return -ESHUTDOWN;
+ }
+
+ set_bit(GD_QUEUE_REF, &disk->state);

/*
* The disk queue should now be all set with enough information about
@@ -546,7 +566,8 @@ static 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",
@@ -554,12 +575,13 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
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;
+ return ret;
}
disk->major = BLOCK_EXT_MAJOR;
disk->first_minor = MINOR(ret);
@@ -568,7 +590,9 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,

disk->flags |= GENHD_FL_UP;

- disk_alloc_events(disk);
+ ret = disk_alloc_events(disk);
+ if (ret)
+ goto exit_free_ext_minor;

if (disk->flags & GENHD_FL_HIDDEN) {
/*
@@ -585,29 +609,57 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
dev->devt = MKDEV(disk->major, disk->first_minor);
ret = bdi_register(bdi, "%u:%u",
disk->major, disk->first_minor);
- WARN_ON(ret);
+ if (WARN_ON(ret))
+ goto exit_disk_release_events;
bdi_set_owner(bdi, dev);
bdev_add(disk->part0, dev->devt);
}
- register_disk(parent, disk, groups);
- if (register_queue)
- blk_register_queue(disk);
+ ret = register_disk(parent, disk, groups);
+ if (ret)
+ goto exit_unregister_bdi;
+
+ if (register_queue) {
+ ret = blk_register_queue(disk);
+ if (ret)
+ goto exit_unregister_disk;
+ }

disk_add_events(disk);
- blk_integrity_add(disk);
+
+ ret = blk_integrity_add(disk);
+ if (ret)
+ goto exit_del_events;
+
+ return 0;
+exit_del_events:
+ disk_del_events(disk);
+exit_unregister_disk:
+ unregister_disk(disk);
+ return ret;
+
+exit_unregister_bdi:
+ if (disk->queue && !(disk->flags & GENHD_FL_HIDDEN))
+ bdi_unregister(disk->queue->backing_dev_info);
+exit_disk_release_events:
+ disk_release_events(disk);
+exit_free_ext_minor:
+ if (disk->major == BLOCK_EXT_MAJOR)
+ blk_free_ext_minor(disk->first_minor);
+
+ return ret;
}

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

{
- __device_add_disk(parent, disk, groups, true);
+ return __device_add_disk(parent, disk, groups, true);
}
EXPORT_SYMBOL(device_add_disk);

-void device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk)
+int device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk)
{
- __device_add_disk(parent, disk, NULL, false);
+ return __device_add_disk(parent, disk, NULL, false);
}
EXPORT_SYMBOL(device_add_disk_no_queue_reg);

diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 13b34177cc85..51f27b9b38b5 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -210,16 +210,16 @@ 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)
+extern 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 device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk);
-static inline void add_disk_no_queue_reg(struct gendisk *disk)
+extern int device_add_disk_no_queue_reg(struct device *parent, struct gendisk *disk);
+static inline int add_disk_no_queue_reg(struct gendisk *disk)
{
- device_add_disk_no_queue_reg(NULL, disk);
+ return device_add_disk_no_queue_reg(NULL, disk);
}

extern void del_gendisk(struct gendisk *gp);
--
2.27.0