* [PATCH v2 00/10] block: second batch of add_disk() error handling conversions
@ 2021-09-27 22:00 Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 01/10] block/brd: add error handling support for add_disk() Luis Chamberlain
` (10 more replies)
0 siblings, 11 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 01/10] block/brd: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 02/10] bcache: " Luis Chamberlain
` (9 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
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 <mcgrof@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 02/10] bcache: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 01/10] block/brd: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 03/10] nvme-multipath: " Luis Chamberlain
` (8 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.
This driver doesn't do any unwinding with blk_cleanup_disk()
even on errors after add_disk() and so we follow that
tradition.
Acked-by: Coly Li <colyli@suse.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/md/bcache/super.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index f2874c77ff79..f0c32cdd6594 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1082,7 +1082,9 @@ int bch_cached_dev_run(struct cached_dev *dc)
closure_sync(&cl);
}
- add_disk(d->disk);
+ ret = add_disk(d->disk);
+ if (ret)
+ goto out;
bd_link_disk_holder(dc->bdev, dc->disk.disk);
/*
* won't show up in the uevent file, use udevadm monitor -e instead
@@ -1534,10 +1536,11 @@ static void flash_dev_flush(struct closure *cl)
static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
{
+ int err = -ENOMEM;
struct bcache_device *d = kzalloc(sizeof(struct bcache_device),
GFP_KERNEL);
if (!d)
- return -ENOMEM;
+ goto err_ret;
closure_init(&d->cl, NULL);
set_closure_fn(&d->cl, flash_dev_flush, system_wq);
@@ -1551,9 +1554,12 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
bcache_device_attach(d, c, u - c->uuids);
bch_sectors_dirty_init(d);
bch_flash_dev_request_init(d);
- add_disk(d->disk);
+ err = add_disk(d->disk);
+ if (err)
+ goto err;
- if (kobject_add(&d->kobj, &disk_to_dev(d->disk)->kobj, "bcache"))
+ err = kobject_add(&d->kobj, &disk_to_dev(d->disk)->kobj, "bcache");
+ if (err)
goto err;
bcache_device_link(d, c, "volume");
@@ -1567,7 +1573,8 @@ static int flash_dev_run(struct cache_set *c, struct uuid_entry *u)
return 0;
err:
kobject_put(&d->kobj);
- return -ENOMEM;
+err_ret:
+ return err;
}
static int flash_devs_run(struct cache_set *c)
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 03/10] nvme-multipath: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 01/10] block/brd: add error handling support for add_disk() Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 02/10] bcache: " Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:13 ` Keith Busch
2021-09-28 5:39 ` Hannes Reinecke
2021-09-27 22:00 ` [PATCH v2 04/10] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
` (7 subsequent siblings)
10 siblings, 2 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.
Since we now can tell for sure when a disk was added, move
setting the bit NVME_NSHEAD_DISK_LIVE only when we did
add the disk successfully.
Nothing to do here as the cleanup is done elsewhere. We take
care and use test_and_set_bit() because it is protects against
two nvme paths simultaneously calling device_add_disk() on the
same namespace head.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/nvme/host/multipath.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index e8ccdd398f78..35cace4f3f5f 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -496,13 +496,22 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
static void nvme_mpath_set_live(struct nvme_ns *ns)
{
struct nvme_ns_head *head = ns->head;
+ int rc;
if (!head->disk)
return;
+ /*
+ * test_and_set_bit() is used because it is protecting against two nvme
+ * paths simultaneously calling device_add_disk() on the same namespace
+ * head.
+ */
if (!test_and_set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
- device_add_disk(&head->subsys->dev, head->disk,
- nvme_ns_id_attr_groups);
+ rc = device_add_disk(&head->subsys->dev, head->disk,
+ nvme_ns_id_attr_groups);
+ if (rc)
+ return;
+ set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags);
nvme_add_ns_head_cdev(head);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 04/10] nvdimm/btt: do not call del_gendisk() if not needed
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (2 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 03/10] nvme-multipath: " Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 05/10] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
` (6 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
We know we don't need del_gendisk() if we haven't added
the disk, so just skip it. This should fix a bug on older
kernels, as del_gendisk() became able to deal with
disks not added only recently, after the patch titled
"block: add flag for add_disk() completion notation".
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/nvdimm/btt.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 52de60b7adee..29cc7325e890 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1538,7 +1538,6 @@ static int btt_blk_init(struct btt *btt)
int rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt));
if (rc) {
- del_gendisk(btt->btt_disk);
blk_cleanup_disk(btt->btt_disk);
return rc;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 05/10] nvdimm/btt: use goto error labels on btt_blk_init()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (3 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 04/10] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 06/10] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
` (5 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
This will make it easier to share common error paths.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/nvdimm/btt.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 29cc7325e890..23ee8c005db5 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1520,10 +1520,11 @@ static int btt_blk_init(struct btt *btt)
{
struct nd_btt *nd_btt = btt->nd_btt;
struct nd_namespace_common *ndns = nd_btt->ndns;
+ int rc = -ENOMEM;
btt->btt_disk = blk_alloc_disk(NUMA_NO_NODE);
if (!btt->btt_disk)
- return -ENOMEM;
+ goto out;
nvdimm_namespace_disk_name(ndns, btt->btt_disk->disk_name);
btt->btt_disk->first_minor = 0;
@@ -1535,19 +1536,23 @@ static int btt_blk_init(struct btt *btt)
blk_queue_flag_set(QUEUE_FLAG_NONROT, btt->btt_disk->queue);
if (btt_meta_size(btt)) {
- int rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt));
-
- if (rc) {
- blk_cleanup_disk(btt->btt_disk);
- return rc;
- }
+ rc = nd_integrity_init(btt->btt_disk, btt_meta_size(btt));
+ if (rc)
+ goto out_cleanup_disk;
}
+
set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9);
device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL);
+
btt->nd_btt->size = btt->nlba * (u64)btt->sector_size;
nvdimm_check_and_set_ro(btt->btt_disk);
return 0;
+
+out_cleanup_disk:
+ blk_cleanup_disk(btt->btt_disk);
+out:
+ return rc;
}
static void btt_blk_cleanup(struct btt *btt)
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 06/10] nvdimm/btt: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (4 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 05/10] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 07/10] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
` (4 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
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 <mcgrof@kernel.org>
---
drivers/nvdimm/btt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 23ee8c005db5..57b921c5fbb5 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1542,7 +1542,9 @@ static int btt_blk_init(struct btt *btt)
}
set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9);
- device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL);
+ rc = device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL);
+ if (rc)
+ goto out_cleanup_disk;
btt->nd_btt->size = btt->nlba * (u64)btt->sector_size;
nvdimm_check_and_set_ro(btt->btt_disk);
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 07/10] nvdimm/blk: avoid calling del_gendisk() on early failures
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (5 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 06/10] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 08/10] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
` (3 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
If nd_integrity_init() fails we'd get del_gendisk() called,
but that's not correct as we should only call that if we're
done with device_add_disk(). Fix this by providing unwinding
prior to the devm call being registered and moving the devm
registration to the very end.
This should fix calling del_gendisk() if nd_integrity_init()
fails. I only spotted this issue through code inspection. It
does not fix any real world bug.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/nvdimm/blk.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index 088d3dd6f6fa..591fa1f86f1e 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -240,6 +240,7 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk)
resource_size_t available_disk_size;
struct gendisk *disk;
u64 internal_nlba;
+ int rc;
internal_nlba = div_u64(nsblk->size, nsblk_internal_lbasize(nsblk));
available_disk_size = internal_nlba * nsblk_sector_size(nsblk);
@@ -256,20 +257,26 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk)
blk_queue_logical_block_size(disk->queue, nsblk_sector_size(nsblk));
blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
- if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk))
- return -ENOMEM;
-
if (nsblk_meta_size(nsblk)) {
- int rc = nd_integrity_init(disk, nsblk_meta_size(nsblk));
+ rc = nd_integrity_init(disk, nsblk_meta_size(nsblk));
if (rc)
- return rc;
+ goto out_before_devm_err;
}
set_capacity(disk, available_disk_size >> SECTOR_SHIFT);
device_add_disk(dev, disk, NULL);
+
+ /* nd_blk_release_disk() is called if this fails */
+ if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk))
+ return -ENOMEM;
+
nvdimm_check_and_set_ro(disk);
return 0;
+
+out_before_devm_err:
+ blk_cleanup_disk(disk);
+ return rc;
}
static int nd_blk_probe(struct device *dev)
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 08/10] nvdimm/blk: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (6 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 07/10] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 09/10] xen-blkfront: " Luis Chamberlain
` (2 subsequent siblings)
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.
Since nvdimm/blk uses devm we just need to move the devm
registration towards the end. And in hindsight, that seems
to also provide a fix given del_gendisk() should not be
called unless the disk was already added via add_disk().
The probably of that issue happening is low though, like
OOM while calling devm_add_action(), so the fix is minor.
We manually unwind in case of add_disk() failure prior
to the devm registration.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/nvdimm/blk.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvdimm/blk.c b/drivers/nvdimm/blk.c
index 591fa1f86f1e..9f1eb41404ac 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -265,7 +265,9 @@ static int nsblk_attach_disk(struct nd_namespace_blk *nsblk)
}
set_capacity(disk, available_disk_size >> SECTOR_SHIFT);
- device_add_disk(dev, disk, NULL);
+ rc = device_add_disk(dev, disk, NULL);
+ if (rc)
+ goto out_before_devm_err;
/* nd_blk_release_disk() is called if this fails */
if (devm_add_action_or_reset(dev, nd_blk_release_disk, disk))
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 09/10] xen-blkfront: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (7 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 08/10] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-30 13:07 ` Juergen Gross
2021-09-27 22:00 ` [PATCH v2 10/10] zram: " Luis Chamberlain
2021-09-27 22:33 ` [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Jens Axboe
10 siblings, 1 reply; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
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 <mcgrof@kernel.org>
---
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 10/10] zram: add error handling support for add_disk()
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (8 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 09/10] xen-blkfront: " Luis Chamberlain
@ 2021-09-27 22:00 ` Luis Chamberlain
2021-09-27 22:33 ` [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Jens Axboe
10 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-27 22:00 UTC (permalink / raw)
To: axboe, colyli, kent.overstreet, kbusch, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block,
linux-kernel, Luis Chamberlain
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 <mcgrof@kernel.org>
---
drivers/block/zram/zram_drv.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f61910c65f0f..59086e178fbd 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1953,7 +1953,9 @@ static int zram_add(void)
blk_queue_max_write_zeroes_sectors(zram->disk->queue, UINT_MAX);
blk_queue_flag_set(QUEUE_FLAG_STABLE_WRITES, zram->disk->queue);
- device_add_disk(NULL, zram->disk, zram_disk_attr_groups);
+ ret = device_add_disk(NULL, zram->disk, zram_disk_attr_groups);
+ if (ret)
+ goto out_cleanup_disk;
strlcpy(zram->compressor, default_compressor, sizeof(zram->compressor));
@@ -1961,6 +1963,8 @@ static int zram_add(void)
pr_info("Added device: %s\n", zram->disk->disk_name);
return device_id;
+out_cleanup_disk:
+ blk_cleanup_disk(zram->disk);
out_free_idr:
idr_remove(&zram_index_idr, device_id);
out_free_dev:
--
2.30.2
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 03/10] nvme-multipath: add error handling support for add_disk()
2021-09-27 22:00 ` [PATCH v2 03/10] nvme-multipath: " Luis Chamberlain
@ 2021-09-27 22:13 ` Keith Busch
2021-09-28 5:39 ` Hannes Reinecke
1 sibling, 0 replies; 15+ messages in thread
From: Keith Busch @ 2021-09-27 22:13 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, colyli, kent.overstreet, sagi, vishal.l.verma,
dan.j.williams, dave.jiang, ira.weiny, konrad.wilk, roger.pau,
boris.ostrovsky, jgross, sstabellini, minchan, ngupta,
senozhatsky, xen-devel, nvdimm, linux-nvme, linux-bcache,
linux-block, linux-kernel
On Mon, Sep 27, 2021 at 03:00:32PM -0700, Luis Chamberlain wrote:
> + /*
> + * test_and_set_bit() is used because it is protecting against two nvme
> + * paths simultaneously calling device_add_disk() on the same namespace
> + * head.
> + */
> if (!test_and_set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
> - device_add_disk(&head->subsys->dev, head->disk,
> - nvme_ns_id_attr_groups);
> + rc = device_add_disk(&head->subsys->dev, head->disk,
> + nvme_ns_id_attr_groups);
> + if (rc)
> + return;
> + set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags);
No need to set_bit() here since the test_and_set_bit() already took care
of that.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 00/10] block: second batch of add_disk() error handling conversions
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
` (9 preceding siblings ...)
2021-09-27 22:00 ` [PATCH v2 10/10] zram: " Luis Chamberlain
@ 2021-09-27 22:33 ` Jens Axboe
10 siblings, 0 replies; 15+ messages in thread
From: Jens Axboe @ 2021-09-27 22:33 UTC (permalink / raw)
To: Luis Chamberlain, colyli, kent.overstreet, kbusch, sagi,
vishal.l.verma, dan.j.williams, dave.jiang, ira.weiny,
konrad.wilk, roger.pau, boris.ostrovsky, jgross, sstabellini,
minchan, ngupta, senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 03/10] nvme-multipath: add error handling support for add_disk()
2021-09-27 22:00 ` [PATCH v2 03/10] nvme-multipath: " Luis Chamberlain
2021-09-27 22:13 ` Keith Busch
@ 2021-09-28 5:39 ` Hannes Reinecke
1 sibling, 0 replies; 15+ messages in thread
From: Hannes Reinecke @ 2021-09-28 5:39 UTC (permalink / raw)
To: Luis Chamberlain, axboe, colyli, kent.overstreet, kbusch, sagi,
vishal.l.verma, dan.j.williams, dave.jiang, ira.weiny,
konrad.wilk, roger.pau, boris.ostrovsky, jgross, sstabellini,
minchan, ngupta, senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block, linux-kernel
On 9/28/21 12:00 AM, Luis Chamberlain wrote:
> We never checked for errors on add_disk() as this function
> returned void. Now that this is fixed, use the shiny new
> error handling.
>
> Since we now can tell for sure when a disk was added, move
> setting the bit NVME_NSHEAD_DISK_LIVE only when we did
> add the disk successfully.
>
> Nothing to do here as the cleanup is done elsewhere. We take
> care and use test_and_set_bit() because it is protects against
> two nvme paths simultaneously calling device_add_disk() on the
> same namespace head.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
> drivers/nvme/host/multipath.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
> index e8ccdd398f78..35cace4f3f5f 100644
> --- a/drivers/nvme/host/multipath.c
> +++ b/drivers/nvme/host/multipath.c
> @@ -496,13 +496,22 @@ int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl, struct nvme_ns_head *head)
> static void nvme_mpath_set_live(struct nvme_ns *ns)
> {
> struct nvme_ns_head *head = ns->head;
> + int rc;
>
> if (!head->disk)
> return;
>
> + /*
> + * test_and_set_bit() is used because it is protecting against two nvme
> + * paths simultaneously calling device_add_disk() on the same namespace
> + * head.
> + */
> if (!test_and_set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags)) {
> - device_add_disk(&head->subsys->dev, head->disk,
> - nvme_ns_id_attr_groups);
> + rc = device_add_disk(&head->subsys->dev, head->disk,
> + nvme_ns_id_attr_groups);
> + if (rc)
> + return;
> + set_bit(NVME_NSHEAD_DISK_LIVE, &head->flags);
> nvme_add_ns_head_cdev(head);
> }
>
> Errm.
Setting the same bit twice?
And shouldn't you unset the bit if 'device_add_disk()' fails?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 09/10] xen-blkfront: add error handling support for add_disk()
2021-09-27 22:00 ` [PATCH v2 09/10] xen-blkfront: " Luis Chamberlain
@ 2021-09-30 13:07 ` Juergen Gross
0 siblings, 0 replies; 15+ messages in thread
From: Juergen Gross @ 2021-09-30 13:07 UTC (permalink / raw)
To: Luis Chamberlain, axboe, colyli, kent.overstreet, kbusch, sagi,
vishal.l.verma, dan.j.williams, dave.jiang, ira.weiny,
konrad.wilk, roger.pau, boris.ostrovsky, sstabellini, minchan,
ngupta, senozhatsky
Cc: xen-devel, nvdimm, linux-nvme, linux-bcache, linux-block, linux-kernel
[-- Attachment #1.1.1: Type: text/plain, Size: 729 bytes --]
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 <mcgrof@kernel.org>
Reviewed-by: Juergen Gross <jgross@suse.com>
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-09-30 13:08 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 22:00 [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 01/10] block/brd: add error handling support for add_disk() Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 02/10] bcache: " Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 03/10] nvme-multipath: " Luis Chamberlain
2021-09-27 22:13 ` Keith Busch
2021-09-28 5:39 ` Hannes Reinecke
2021-09-27 22:00 ` [PATCH v2 04/10] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 05/10] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 06/10] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 07/10] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 08/10] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
2021-09-27 22:00 ` [PATCH v2 09/10] xen-blkfront: " Luis Chamberlain
2021-09-30 13:07 ` Juergen Gross
2021-09-27 22:00 ` [PATCH v2 10/10] zram: " Luis Chamberlain
2021-09-27 22:33 ` [PATCH v2 00/10] block: second batch of add_disk() error handling conversions Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).