2021-09-27 22:04:00

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v2 00/10] block: second batch of add_disk() error handling conversions

This is the second series of driver conversions for add_disk()
error handling. You can find this set and the rest of the 7th set of
driver conversions on my 20210927-for-axboe-add-disk-error-handling
branch [0].

Changes on this v2 since the last first version of this
patch series:

- rebased onto linux-next tag 20210927
- nvme-multipath: used test_and_set_bit() as suggested by Keith Busch,
and justified this in the code with a comment as this race was not
obvious
- Added reviewed-by / Acked-by tags where one was provided

[0] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20210927-for-axboe-add-disk-error-handling

Luis Chamberlain (10):
block/brd: add error handling support for add_disk()
bcache: add error handling support for add_disk()
nvme-multipath: add error handling support for add_disk()
nvdimm/btt: do not call del_gendisk() if not needed
nvdimm/btt: use goto error labels on btt_blk_init()
nvdimm/btt: add error handling support for add_disk()
nvdimm/blk: avoid calling del_gendisk() on early failures
nvdimm/blk: add error handling support for add_disk()
xen-blkfront: add error handling support for add_disk()
zram: add error handling support for add_disk()

drivers/block/brd.c | 10 ++++++++--
drivers/block/xen-blkfront.c | 8 +++++++-
drivers/block/zram/zram_drv.c | 6 +++++-
drivers/md/bcache/super.c | 17 ++++++++++++-----
drivers/nvdimm/blk.c | 21 +++++++++++++++------
drivers/nvdimm/btt.c | 24 +++++++++++++++---------
drivers/nvme/host/multipath.c | 13 +++++++++++--
7 files changed, 73 insertions(+), 26 deletions(-)

--
2.30.2


2021-09-27 22:04:16

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v2 01/10] block/brd: add error handling support for add_disk()

We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.

Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/block/brd.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 58ec167aa018..c2bf4946f4e3 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -372,6 +372,7 @@ static int brd_alloc(int i)
struct brd_device *brd;
struct gendisk *disk;
char buf[DISK_NAME_LEN];
+ int err = -ENOMEM;

brd = kzalloc(sizeof(*brd), GFP_KERNEL);
if (!brd)
@@ -410,14 +411,19 @@ static int brd_alloc(int i)
/* Tell the block layer that this is not a rotational device */
blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
- add_disk(disk);
+ err = add_disk(disk);
+ if (err)
+ goto out_cleanup_disk;
+
list_add_tail(&brd->brd_list, &brd_devices);

return 0;

+out_cleanup_disk:
+ blk_cleanup_disk(disk);
out_free_dev:
kfree(brd);
- return -ENOMEM;
+ return err;
}

static void brd_probe(dev_t dev)
--
2.30.2

2021-09-27 22:04:33

by Luis Chamberlain

[permalink] [raw]
Subject: [PATCH v2 09/10] xen-blkfront: add error handling support for add_disk()

We never checked for errors on device_add_disk() as this function
returned void. Now that this is fixed, use the shiny new error
handling. The function xlvbd_alloc_gendisk() typically does the
unwinding on error on allocating the disk and creating the tag,
but since all that error handling was stuffed inside
xlvbd_alloc_gendisk() we must repeat the tag free'ing as well.

We set the info->rq to NULL to ensure blkif_free() doesn't crash
on blk_mq_stop_hw_queues() on device_add_disk() error as the queue
will be long gone by then.

Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/block/xen-blkfront.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 72902104f111..86440b051766 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2385,7 +2385,13 @@ static void blkfront_connect(struct blkfront_info *info)
for_each_rinfo(info, rinfo, i)
kick_pending_request_queues(rinfo);

- device_add_disk(&info->xbdev->dev, info->gd, NULL);
+ err = device_add_disk(&info->xbdev->dev, info->gd, NULL);
+ if (err) {
+ blk_cleanup_disk(info->gd);
+ blk_mq_free_tag_set(&info->tag_set);
+ info->rq = NULL;
+ goto fail;
+ }

info->is_ready = 1;
return;
--
2.30.2

2021-09-27 22:35:59

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] block: second batch of add_disk() error handling conversions

On 9/27/21 4:00 PM, Luis Chamberlain wrote:
> This is the second series of driver conversions for add_disk()
> error handling. You can find this set and the rest of the 7th set of
> driver conversions on my 20210927-for-axboe-add-disk-error-handling
> branch [0].

Applied 1, thanks.

--
Jens Axboe

2021-09-30 13:30:41

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH v2 09/10] xen-blkfront: add error handling support for add_disk()

On 28.09.21 00:00, Luis Chamberlain wrote:
> We never checked for errors on device_add_disk() as this function
> returned void. Now that this is fixed, use the shiny new error
> handling. The function xlvbd_alloc_gendisk() typically does the
> unwinding on error on allocating the disk and creating the tag,
> but since all that error handling was stuffed inside
> xlvbd_alloc_gendisk() we must repeat the tag free'ing as well.
>
> We set the info->rq to NULL to ensure blkif_free() doesn't crash
> on blk_mq_stop_hw_queues() on device_add_disk() error as the queue
> will be long gone by then.
>
> Signed-off-by: Luis Chamberlain <[email protected]>

Reviewed-by: Juergen Gross <[email protected]>


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.06 kB)
OpenPGP public key
OpenPGP_signature (505.00 B)
OpenPGP digital signature
Download all attachments