nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] block: add_disk() error handling stragglers
@ 2021-10-15 23:52 Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 01/13] block/brd: add error handling support for add_disk() Luis Chamberlain
                   ` (15 more replies)
  0 siblings, 16 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	linux-kernel, Luis Chamberlain

This patch set consists of al the straggler drivers for which we have
have no patch reviews done for yet. I'd like to ask for folks to please
consider chiming in, specially if you're the maintainer for the driver.
Additionally if you can specify if you'll take the patch in yourself or
if you want Jens to take it, that'd be great too.

Luis Chamberlain (13):
  block/brd: 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()
  zram: add error handling support for add_disk()
  z2ram: add error handling support for add_disk()
  ps3disk: add error handling support for add_disk()
  ps3vram: add error handling support for add_disk()
  block/sunvdc: add error handling support for add_disk()
  mtd/ubi/block: add error handling support for add_disk()

 drivers/block/brd.c           |  9 +++++++--
 drivers/block/ps3disk.c       |  8 ++++++--
 drivers/block/ps3vram.c       |  7 ++++++-
 drivers/block/sunvdc.c        | 14 +++++++++++---
 drivers/block/z2ram.c         |  7 +++++--
 drivers/block/zram/zram_drv.c |  6 +++++-
 drivers/mtd/ubi/block.c       |  8 +++++++-
 drivers/nvdimm/blk.c          | 21 +++++++++++++++------
 drivers/nvdimm/btt.c          | 24 +++++++++++++++---------
 drivers/nvme/host/multipath.c | 14 ++++++++++++--
 10 files changed, 89 insertions(+), 29 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH 01/13] block/brd: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 02/13] nvme-multipath: " Luis Chamberlain
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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 | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 530b31240203..6065f493876f 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;
 
 	mutex_lock(&brd_devices_mutex);
 	list_for_each_entry(brd, &brd_devices, brd_list) {
@@ -422,16 +423,20 @@ 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;
 
 	return 0;
 
+out_cleanup_disk:
+	blk_cleanup_disk(disk);
 out_free_dev:
 	mutex_lock(&brd_devices_mutex);
 	list_del(&brd->brd_list);
 	mutex_unlock(&brd_devices_mutex);
 	kfree(brd);
-	return -ENOMEM;
+	return err;
 }
 
 static void brd_probe(dev_t dev)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 02/13] nvme-multipath: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 01/13] block/brd: add error handling support for add_disk() Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-16  0:01   ` Keith Busch
                     ` (2 more replies)
  2021-10-15 23:52 ` [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
                   ` (13 subsequent siblings)
  15 siblings, 3 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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 | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index e8ccdd398f78..022837f7be41 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -496,13 +496,23 @@ 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) {
+			clear_bit(NVME_NSHEAD_DISK_LIVE, &ns->flags);
+			return;
+		}
 		nvme_add_ns_head_cdev(head);
 	}
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 01/13] block/brd: add error handling support for add_disk() Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 02/13] nvme-multipath: " Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-31 17:47   ` Dan Williams
  2021-10-15 23:52 ` [PATCH 04/13] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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] 36+ messages in thread

* [PATCH 04/13] nvdimm/btt: use goto error labels on btt_blk_init()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (2 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-31 17:51   ` Dan Williams
  2021-10-15 23:52 ` [PATCH 05/13] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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] 36+ messages in thread

* [PATCH 05/13] nvdimm/btt: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (3 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 04/13] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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] 36+ messages in thread

* [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (4 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 05/13] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-16  0:13   ` Dan Williams
  2021-10-15 23:52 ` [PATCH 07/13] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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] 36+ messages in thread

* [PATCH 07/13] nvdimm/blk: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (5 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 08/13] zram: " Luis Chamberlain
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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] 36+ messages in thread

* [PATCH 08/13] zram: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (6 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 07/13] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-25 16:55   ` Minchan Kim
  2021-10-15 23:52 ` [PATCH 09/13] z2ram: " Luis Chamberlain
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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 0a9309a2ef54..bdbded107e6b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -1983,7 +1983,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));
 
@@ -1991,6 +1993,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] 36+ messages in thread

* [PATCH 09/13] z2ram: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (7 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 08/13] zram: " Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 10/13] ps3disk: " Luis Chamberlain
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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. Only the disk is cleaned up inside
z2ram_register_disk() as the caller deals with the rest.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/z2ram.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index 4eef218108c6..ccc52c935faf 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -318,6 +318,7 @@ static const struct blk_mq_ops z2_mq_ops = {
 static int z2ram_register_disk(int minor)
 {
 	struct gendisk *disk;
+	int err;
 
 	disk = blk_mq_alloc_disk(&tag_set, NULL);
 	if (IS_ERR(disk))
@@ -333,8 +334,10 @@ static int z2ram_register_disk(int minor)
 		sprintf(disk->disk_name, "z2ram");
 
 	z2ram_gendisk[minor] = disk;
-	add_disk(disk);
-	return 0;
+	err = add_disk(disk);
+	if (err)
+		blk_cleanup_disk(disk);
+	return err;
 }
 
 static int __init z2_init(void)
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 10/13] ps3disk: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (8 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 09/13] z2ram: " Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-29 15:05   ` Geoff Levand
  2021-10-15 23:52 ` [PATCH 11/13] ps3vram: " Luis Chamberlain
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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/ps3disk.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ps3disk.c b/drivers/block/ps3disk.c
index 8d51efbe045d..3054adf77460 100644
--- a/drivers/block/ps3disk.c
+++ b/drivers/block/ps3disk.c
@@ -467,9 +467,13 @@ static int ps3disk_probe(struct ps3_system_bus_device *_dev)
 		 gendisk->disk_name, priv->model, priv->raw_capacity >> 11,
 		 get_capacity(gendisk) >> 11);
 
-	device_add_disk(&dev->sbd.core, gendisk, NULL);
-	return 0;
+	error = device_add_disk(&dev->sbd.core, gendisk, NULL);
+	if (error)
+		goto fail_cleanup_disk;
 
+	return 0;
+fail_cleanup_disk:
+	blk_cleanup_disk(gendisk);
 fail_free_tag_set:
 	blk_mq_free_tag_set(&priv->tag_set);
 fail_teardown:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 11/13] ps3vram: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (9 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 10/13] ps3disk: " Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-29 15:09   ` Geoff Levand
  2021-10-15 23:52 ` [PATCH 12/13] block/sunvdc: " Luis Chamberlain
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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/ps3vram.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/block/ps3vram.c b/drivers/block/ps3vram.c
index c7b19e128b03..af2a0d09c598 100644
--- a/drivers/block/ps3vram.c
+++ b/drivers/block/ps3vram.c
@@ -755,9 +755,14 @@ static int ps3vram_probe(struct ps3_system_bus_device *dev)
 	dev_info(&dev->core, "%s: Using %llu MiB of GPU memory\n",
 		 gendisk->disk_name, get_capacity(gendisk) >> 11);
 
-	device_add_disk(&dev->core, gendisk, NULL);
+	error = device_add_disk(&dev->core, gendisk, NULL);
+	if (error)
+		goto out_cleanup_disk;
+
 	return 0;
 
+out_cleanup_disk:
+	blk_cleanup_disk(gendisk);
 out_cache_cleanup:
 	remove_proc_entry(DEVICE_NAME, NULL);
 	ps3vram_cache_cleanup(dev);
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 12/13] block/sunvdc: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (10 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 11/13] ps3vram: " Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-15 23:52 ` [PATCH 13/13] mtd/ubi/block: " Luis Chamberlain
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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.

We re-use the same free tag call, so we also add a label for
that as well.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/sunvdc.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c
index 4d4bb810c2ae..6f45a53f7cbf 100644
--- a/drivers/block/sunvdc.c
+++ b/drivers/block/sunvdc.c
@@ -826,8 +826,8 @@ static int probe_disk(struct vdc_port *port)
 	if (IS_ERR(g)) {
 		printk(KERN_ERR PFX "%s: Could not allocate gendisk.\n",
 		       port->vio.name);
-		blk_mq_free_tag_set(&port->tag_set);
-		return PTR_ERR(g);
+		err = PTR_ERR(g);
+		goto out_free_tag;
 	}
 
 	port->disk = g;
@@ -879,9 +879,17 @@ static int probe_disk(struct vdc_port *port)
 	       port->vdisk_size, (port->vdisk_size >> (20 - 9)),
 	       port->vio.ver.major, port->vio.ver.minor);
 
-	device_add_disk(&port->vio.vdev->dev, g, NULL);
+	err = device_add_disk(&port->vio.vdev->dev, g, NULL);
+	if (err)
+		goto out_cleanup_disk;
 
 	return 0;
+
+out_cleanup_disk:
+	blk_cleanup_disk(g);
+out_free_tag:
+	blk_mq_free_tag_set(&port->tag_set);
+	return err;
 }
 
 static struct ldc_channel_config vdc_ldc_cfg = {
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 13/13] mtd/ubi/block: add error handling support for add_disk()
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (11 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 12/13] block/sunvdc: " Luis Chamberlain
@ 2021-10-15 23:52 ` Luis Chamberlain
  2021-10-17 15:26 ` [PATCH 00/13] block: add_disk() error handling stragglers Geoff Levand
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-15 23:52 UTC (permalink / raw)
  To: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	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/mtd/ubi/block.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index e003b4b44ffa..062e6c2c45f5 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -447,12 +447,18 @@ int ubiblock_create(struct ubi_volume_info *vi)
 	list_add_tail(&dev->list, &ubiblock_devices);
 
 	/* Must be the last step: anyone can call file ops from now on */
-	add_disk(dev->gd);
+	ret = add_disk(dev->gd);
+	if (ret)
+		goto out_destroy_wq;
+
 	dev_info(disk_to_dev(dev->gd), "created from ubi%d:%d(%s)",
 		 dev->ubi_num, dev->vol_id, vi->name);
 	mutex_unlock(&devices_mutex);
 	return 0;
 
+out_destroy_wq:
+	list_del(&dev->list);
+	destroy_workqueue(dev->wq);
 out_remove_minor:
 	idr_remove(&ubiblock_minor_idr, gd->first_minor);
 out_cleanup_disk:
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 02/13] nvme-multipath: add error handling support for add_disk()
  2021-10-15 23:52 ` [PATCH 02/13] nvme-multipath: " Luis Chamberlain
@ 2021-10-16  0:01   ` Keith Busch
  2021-10-16  4:39   ` Christoph Hellwig
  2021-10-18  5:34   ` Chaitanya Kulkarni
  2 siblings, 0 replies; 36+ messages in thread
From: Keith Busch @ 2021-10-16  0:01 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, hch, sagi, linux-block,
	linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

On Fri, Oct 15, 2021 at 04:52:08PM -0700, 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.

Looks good, thank you.

Reviewed-by: Keith Busch <kbusch@kernel.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-10-15 23:52 ` [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
@ 2021-10-16  0:13   ` Dan Williams
  2021-10-19 16:06     ` Luis Chamberlain
  2021-11-03  0:10     ` Luis Chamberlain
  0 siblings, 2 replies; 36+ messages in thread
From: Dan Williams @ 2021-10-16  0:13 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> 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.
>

Just fyi, I'm preparing patches to delete this driver completely as it
is unused by any shipping platform. I hope to get that removal into
v5.16.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 02/13] nvme-multipath: add error handling support for add_disk()
  2021-10-15 23:52 ` [PATCH 02/13] nvme-multipath: " Luis Chamberlain
  2021-10-16  0:01   ` Keith Busch
@ 2021-10-16  4:39   ` Christoph Hellwig
  2021-10-18  5:34   ` Chaitanya Kulkarni
  2 siblings, 0 replies; 36+ messages in thread
From: Christoph Hellwig @ 2021-10-16  4:39 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: axboe, geoff, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi,
	linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme,
	linux-kernel

Thanks,

applied to nvme-5.16.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/13] block: add_disk() error handling stragglers
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (12 preceding siblings ...)
  2021-10-15 23:52 ` [PATCH 13/13] mtd/ubi/block: " Luis Chamberlain
@ 2021-10-17 15:26 ` Geoff Levand
  2021-10-18 16:15   ` Luis Chamberlain
  2021-10-30 17:03 ` (subset) " Jens Axboe
  2021-10-30 17:07 ` Jens Axboe
  15 siblings, 1 reply; 36+ messages in thread
From: Geoff Levand @ 2021-10-17 15:26 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

Hi Luis,

On 10/15/21 4:52 PM, Luis Chamberlain wrote:
> This patch set consists of al the straggler drivers for which we have
> have no patch reviews done for yet. I'd like to ask for folks to please
> consider chiming in, specially if you're the maintainer for the driver.
> Additionally if you can specify if you'll take the patch in yourself or
> if you want Jens to take it, that'd be great too.

Do you have a git repo with the patch set applied that I can use to test with?

Thanks.

-Geoff

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 02/13] nvme-multipath: add error handling support for add_disk()
  2021-10-15 23:52 ` [PATCH 02/13] nvme-multipath: " Luis Chamberlain
  2021-10-16  0:01   ` Keith Busch
  2021-10-16  4:39   ` Christoph Hellwig
@ 2021-10-18  5:34   ` Chaitanya Kulkarni
  2 siblings, 0 replies; 36+ messages in thread
From: Chaitanya Kulkarni @ 2021-10-18  5:34 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, geoff, mpe, benh, paulus, jim, minchan,
	ngupta, senozhatsky, richard, miquel.raynal, vigneshr,
	dan.j.williams, vishal.l.verma, dave.jiang, ira.weiny, kbusch,
	hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

On 10/15/21 16:52, 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>


Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>




^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/13] block: add_disk() error handling stragglers
  2021-10-17 15:26 ` [PATCH 00/13] block: add_disk() error handling stragglers Geoff Levand
@ 2021-10-18 16:15   ` Luis Chamberlain
  2021-10-22  3:10     ` Geoff Levand
  0 siblings, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-18 16:15 UTC (permalink / raw)
  To: Geoff Levand
  Cc: axboe, mpe, benh, paulus, jim, minchan, ngupta, senozhatsky,
	richard, miquel.raynal, vigneshr, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, kbusch, hch, sagi, linux-block,
	linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

On Sun, Oct 17, 2021 at 08:26:33AM -0700, Geoff Levand wrote:
> Hi Luis,
> 
> On 10/15/21 4:52 PM, Luis Chamberlain wrote:
> > This patch set consists of al the straggler drivers for which we have
> > have no patch reviews done for yet. I'd like to ask for folks to please
> > consider chiming in, specially if you're the maintainer for the driver.
> > Additionally if you can specify if you'll take the patch in yourself or
> > if you want Jens to take it, that'd be great too.
> 
> Do you have a git repo with the patch set applied that I can use to test with?

Sure, although the second to last patch is in a state of flux given
the ataflop driver currently is broken and so we're seeing how to fix
that first:

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

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-10-16  0:13   ` Dan Williams
@ 2021-10-19 16:06     ` Luis Chamberlain
  2021-11-03  0:10     ` Luis Chamberlain
  1 sibling, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-19 16:06 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Fri, Oct 15, 2021 at 05:13:48PM -0700, Dan Williams wrote:
> On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > 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.
> >
> 
> Just fyi, I'm preparing patches to delete this driver completely as it
> is unused by any shipping platform. I hope to get that removal into
> v5.16.

I'll remove this from my queue, any chance you can review the changes
for nvdimm/btt?

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/13] block: add_disk() error handling stragglers
  2021-10-18 16:15   ` Luis Chamberlain
@ 2021-10-22  3:10     ` Geoff Levand
  2021-10-25 15:58       ` Luis Chamberlain
  0 siblings, 1 reply; 36+ messages in thread
From: Geoff Levand @ 2021-10-22  3:10 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: axboe, mpe, benh, paulus, jim, minchan, ngupta, senozhatsky,
	richard, miquel.raynal, vigneshr, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, kbusch, hch, sagi, linux-block,
	linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

Hi Luis,

On 10/18/21 9:15 AM, Luis Chamberlain wrote:
> On Sun, Oct 17, 2021 at 08:26:33AM -0700, Geoff Levand wrote:
>> Hi Luis,
>>
>> On 10/15/21 4:52 PM, Luis Chamberlain wrote:
>>> This patch set consists of al the straggler drivers for which we have
>>> have no patch reviews done for yet. I'd like to ask for folks to please
>>> consider chiming in, specially if you're the maintainer for the driver.
>>> Additionally if you can specify if you'll take the patch in yourself or
>>> if you want Jens to take it, that'd be great too.
>>
>> Do you have a git repo with the patch set applied that I can use to test with?
> 
> Sure, although the second to last patch is in a state of flux given
> the ataflop driver currently is broken and so we're seeing how to fix
> that first:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20211011-for-axboe-add-disk-error-handling

That branch has so many changes applied on top of the base v5.15-rc4
that the patches I need to apply to test on PS3 with don't apply.

Do you have something closer to say v5.15-rc5?  Preferred would be
just your add_disk() error handling patches plus what they depend
on.

Thanks.

-Geoff

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 00/13] block: add_disk() error handling stragglers
  2021-10-22  3:10     ` Geoff Levand
@ 2021-10-25 15:58       ` Luis Chamberlain
  0 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-10-25 15:58 UTC (permalink / raw)
  To: Geoff Levand
  Cc: axboe, mpe, benh, paulus, jim, minchan, ngupta, senozhatsky,
	richard, miquel.raynal, vigneshr, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, kbusch, hch, sagi, linux-block,
	linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

On Thu, Oct 21, 2021 at 08:10:49PM -0700, Geoff Levand wrote:
> Hi Luis,
> 
> On 10/18/21 9:15 AM, Luis Chamberlain wrote:
> > On Sun, Oct 17, 2021 at 08:26:33AM -0700, Geoff Levand wrote:
> >> Hi Luis,
> >>
> >> On 10/15/21 4:52 PM, Luis Chamberlain wrote:
> >>> This patch set consists of al the straggler drivers for which we have
> >>> have no patch reviews done for yet. I'd like to ask for folks to please
> >>> consider chiming in, specially if you're the maintainer for the driver.
> >>> Additionally if you can specify if you'll take the patch in yourself or
> >>> if you want Jens to take it, that'd be great too.
> >>
> >> Do you have a git repo with the patch set applied that I can use to test with?
> > 
> > Sure, although the second to last patch is in a state of flux given
> > the ataflop driver currently is broken and so we're seeing how to fix
> > that first:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20211011-for-axboe-add-disk-error-handling
> 
> That branch has so many changes applied on top of the base v5.15-rc4
> that the patches I need to apply to test on PS3 with don't apply.
> 
> Do you have something closer to say v5.15-rc5?  Preferred would be
> just your add_disk() error handling patches plus what they depend
> on.

If you just want to test the ps3 changes, I've put this branch together
just for yo, its based on v5.15-rc6:

https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20211025-ps3-add-disk

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 08/13] zram: add error handling support for add_disk()
  2021-10-15 23:52 ` [PATCH 08/13] zram: " Luis Chamberlain
@ 2021-10-25 16:55   ` Minchan Kim
  0 siblings, 0 replies; 36+ messages in thread
From: Minchan Kim @ 2021-10-25 16:55 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: axboe, geoff, mpe, benh, paulus, jim, ngupta, senozhatsky,
	richard, miquel.raynal, vigneshr, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, kbusch, hch, sagi, linux-block,
	linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

On Fri, Oct 15, 2021 at 04:52:14PM -0700, 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.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Acked-by: Minchan Kim <minchan@kernel.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 10/13] ps3disk: add error handling support for add_disk()
  2021-10-15 23:52 ` [PATCH 10/13] ps3disk: " Luis Chamberlain
@ 2021-10-29 15:05   ` Geoff Levand
  0 siblings, 0 replies; 36+ messages in thread
From: Geoff Levand @ 2021-10-29 15:05 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

Hi Luis,

On 10/15/21 4:52 PM, 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.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

I tested your 20211011-for-axboe-add-disk-error-handling branch
on PS3 and the ps3disk changes seem to be working OK.

Tested-by: Geoff Levand <geoff@infradead.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 11/13] ps3vram: add error handling support for add_disk()
  2021-10-15 23:52 ` [PATCH 11/13] ps3vram: " Luis Chamberlain
@ 2021-10-29 15:09   ` Geoff Levand
  0 siblings, 0 replies; 36+ messages in thread
From: Geoff Levand @ 2021-10-29 15:09 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, mpe, benh, paulus, jim, minchan, ngupta,
	senozhatsky, richard, miquel.raynal, vigneshr, dan.j.williams,
	vishal.l.verma, dave.jiang, ira.weiny, kbusch, hch, sagi
  Cc: linux-block, linuxppc-dev, linux-mtd, nvdimm, linux-nvme, linux-kernel

Hi Luis,

On 10/15/21 4:52 PM, 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.

I didn't yet test this ps3vram related change, but based
on the ps3disk testing I think this change will be OK.

Acked-by: Geoff Levand <geoff@infradead.org>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: (subset) [PATCH 00/13] block: add_disk() error handling stragglers
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (13 preceding siblings ...)
  2021-10-17 15:26 ` [PATCH 00/13] block: add_disk() error handling stragglers Geoff Levand
@ 2021-10-30 17:03 ` Jens Axboe
  2021-10-30 17:07 ` Jens Axboe
  15 siblings, 0 replies; 36+ messages in thread
From: Jens Axboe @ 2021-10-30 17:03 UTC (permalink / raw)
  To: kbusch, dan.j.williams, richard, jim, vishal.l.verma, dave.jiang,
	miquel.raynal, vigneshr, ngupta, ira.weiny, senozhatsky, hch,
	paulus, Luis Chamberlain, sagi, mpe, minchan, geoff, benh
  Cc: linux-mtd, linuxppc-dev, linux-kernel, linux-nvme, nvdimm, linux-block

On Fri, 15 Oct 2021 16:52:06 -0700, Luis Chamberlain wrote:
> This patch set consists of al the straggler drivers for which we have
> have no patch reviews done for yet. I'd like to ask for folks to please
> consider chiming in, specially if you're the maintainer for the driver.
> Additionally if you can specify if you'll take the patch in yourself or
> if you want Jens to take it, that'd be great too.
> 
> Luis Chamberlain (13):
>   block/brd: 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()
>   zram: add error handling support for add_disk()
>   z2ram: add error handling support for add_disk()
>   ps3disk: add error handling support for add_disk()
>   ps3vram: add error handling support for add_disk()
>   block/sunvdc: add error handling support for add_disk()
>   mtd/ubi/block: add error handling support for add_disk()
> 
> [...]

Applied, thanks!

[08/13] zram: add error handling support for add_disk()
        commit: 5e2e1cc4131cf4d21629c94331f2351b7dc8b87c
[10/13] ps3disk: add error handling support for add_disk()
        commit: ff4cbe0fcf5d749f76040f782f0618656cd23e33
[11/13] ps3vram: add error handling support for add_disk()
        commit: 3c30883acab1d20ecbd3c48dc12b147b51548742

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: (subset) [PATCH 00/13] block: add_disk() error handling stragglers
  2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
                   ` (14 preceding siblings ...)
  2021-10-30 17:03 ` (subset) " Jens Axboe
@ 2021-10-30 17:07 ` Jens Axboe
  15 siblings, 0 replies; 36+ messages in thread
From: Jens Axboe @ 2021-10-30 17:07 UTC (permalink / raw)
  To: vigneshr, richard, geoff, vishal.l.verma, kbusch, sagi, minchan,
	mpe, ira.weiny, hch, senozhatsky, dave.jiang, miquel.raynal,
	paulus, dan.j.williams, benh, jim, ngupta, Luis Chamberlain
  Cc: linux-block, linux-nvme, linux-mtd, linuxppc-dev, nvdimm, linux-kernel

On Fri, 15 Oct 2021 16:52:06 -0700, Luis Chamberlain wrote:
> This patch set consists of al the straggler drivers for which we have
> have no patch reviews done for yet. I'd like to ask for folks to please
> consider chiming in, specially if you're the maintainer for the driver.
> Additionally if you can specify if you'll take the patch in yourself or
> if you want Jens to take it, that'd be great too.
> 
> Luis Chamberlain (13):
>   block/brd: 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()
>   zram: add error handling support for add_disk()
>   z2ram: add error handling support for add_disk()
>   ps3disk: add error handling support for add_disk()
>   ps3vram: add error handling support for add_disk()
>   block/sunvdc: add error handling support for add_disk()
>   mtd/ubi/block: add error handling support for add_disk()
> 
> [...]

Applied, thanks!

[01/13] block/brd: add error handling support for add_disk()
        commit: e1528830bd4ebf435d91c154e309e6e028336210

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed
  2021-10-15 23:52 ` [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
@ 2021-10-31 17:47   ` Dan Williams
  2021-11-02 17:03     ` Luis Chamberlain
  0 siblings, 1 reply; 36+ messages in thread
From: Dan Williams @ 2021-10-31 17:47 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> 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".

Perhaps put this in:

    commit $abbrev_commit ("block: add flag for add_disk() completion notation")

...format, but I can't seem to find that commit?

If you're touching the changelog how about one that clarifies the
impact and drops "we"?

"del_gendisk() is not required if the disk has not been added. On
kernels prior to commit $abbrev_commit ("block: add flag for
add_disk() completion notation")
it is mandatory to not call del_gendisk() if the underlying device has
not been through device_add()."

Fixes: 41cd8b70c37a ("libnvdimm, btt: add support for blk integrity")

With that you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 04/13] nvdimm/btt: use goto error labels on btt_blk_init()
  2021-10-15 23:52 ` [PATCH 04/13] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
@ 2021-10-31 17:51   ` Dan Williams
  0 siblings, 0 replies; 36+ messages in thread
From: Dan Williams @ 2021-10-31 17:51 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> 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;

I tend to not use a goto when there is nothing to unwind.

The rest looks good to me. After dropping "goto out;" you can add:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed
  2021-10-31 17:47   ` Dan Williams
@ 2021-11-02 17:03     ` Luis Chamberlain
  0 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-11-02 17:03 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Sun, Oct 31, 2021 at 10:47:22AM -0700, Dan Williams wrote:
> On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > 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".
> 
> Perhaps put this in:
> 
>     commit $abbrev_commit ("block: add flag for add_disk() completion notation")
> 
> ...format, but I can't seem to find that commit?

Indeed, that patch got dropped and it would seem Christoph preferred
a simpler approach with the new disk_live()

commit 40b3a52ffc5bc3b5427d5d35b035cfb19d03fdd6
Author: Christoph Hellwig <hch@lst.de>
Date:   Wed Aug 18 16:45:32 2021 +0200

    block: add a sanity check for a live disk in del_gendisk

> If you're touching the changelog how about one that clarifies the
> impact and drops "we"?
> "del_gendisk() is not required if the disk has not been added. On
> kernels prior to commit $abbrev_commit ("block: add flag for
> add_disk() completion notation")
> it is mandatory to not call del_gendisk() if the underlying device has
> not been through device_add()."
> 
> Fixes: 41cd8b70c37a ("libnvdimm, btt: add support for blk integrity")
> 
> With that you can add:
> 
> Reviewed-by: Dan Williams <dan.j.williams@intel.com>

You got it.

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-10-16  0:13   ` Dan Williams
  2021-10-19 16:06     ` Luis Chamberlain
@ 2021-11-03  0:10     ` Luis Chamberlain
  2021-11-03  0:49       ` Dan Williams
  1 sibling, 1 reply; 36+ messages in thread
From: Luis Chamberlain @ 2021-11-03  0:10 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Fri, Oct 15, 2021 at 05:13:48PM -0700, Dan Williams wrote:
> On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > 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.
> >
> 
> Just fyi, I'm preparing patches to delete this driver completely as it
> is unused by any shipping platform. I hope to get that removal into
> v5.16.

Curious if are you going to nuking it on v5.16? Otherwise it would stand
in the way of the last few patches to add __must_check for the final
add_disk() error handling changes.

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-11-03  0:10     ` Luis Chamberlain
@ 2021-11-03  0:49       ` Dan Williams
  2021-11-03  1:28         ` Jens Axboe
  2021-11-03 12:08         ` Luis Chamberlain
  0 siblings, 2 replies; 36+ messages in thread
From: Dan Williams @ 2021-11-03  0:49 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Tue, Nov 2, 2021 at 5:10 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> On Fri, Oct 15, 2021 at 05:13:48PM -0700, Dan Williams wrote:
> > On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > >
> > > 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.
> > >
> >
> > Just fyi, I'm preparing patches to delete this driver completely as it
> > is unused by any shipping platform. I hope to get that removal into
> > v5.16.
>
> Curious if are you going to nuking it on v5.16? Otherwise it would stand
> in the way of the last few patches to add __must_check for the final
> add_disk() error handling changes.

True, I don't think I can get it nuked in time, so you can add my
Reviewed-by for this one.

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-11-03  0:49       ` Dan Williams
@ 2021-11-03  1:28         ` Jens Axboe
  2021-11-03 12:09           ` Luis Chamberlain
  2021-11-03 12:08         ` Luis Chamberlain
  1 sibling, 1 reply; 36+ messages in thread
From: Jens Axboe @ 2021-11-03  1:28 UTC (permalink / raw)
  To: Dan Williams, Luis Chamberlain
  Cc: Geoff Levand, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Jim Paris, Minchan Kim, Nitin Gupta, senozhatsky,
	Richard Weinberger, miquel.raynal, vigneshr, Vishal L Verma,
	Dave Jiang, Weiny, Ira, Keith Busch, Christoph Hellwig,
	Sagi Grimberg, linux-block, linuxppc-dev, linux-mtd,
	Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On 11/2/21 6:49 PM, Dan Williams wrote:
> On Tue, Nov 2, 2021 at 5:10 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>>
>> On Fri, Oct 15, 2021 at 05:13:48PM -0700, Dan Williams wrote:
>>> On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>>>>
>>>> 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.
>>>>
>>>
>>> Just fyi, I'm preparing patches to delete this driver completely as it
>>> is unused by any shipping platform. I hope to get that removal into
>>> v5.16.
>>
>> Curious if are you going to nuking it on v5.16? Otherwise it would stand
>> in the way of the last few patches to add __must_check for the final
>> add_disk() error handling changes.
> 
> True, I don't think I can get it nuked in time, so you can add my
> Reviewed-by for this one.

Luis, I lost track of the nv* patches from this discussion. If you want
them in 5.16 and they are reviewed, please do resend and I'll pick them
up for the middle-of-merge-window push.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-11-03  0:49       ` Dan Williams
  2021-11-03  1:28         ` Jens Axboe
@ 2021-11-03 12:08         ` Luis Chamberlain
  1 sibling, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-11-03 12:08 UTC (permalink / raw)
  To: Dan Williams
  Cc: Jens Axboe, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Tue, Nov 02, 2021 at 05:49:12PM -0700, Dan Williams wrote:
> On Tue, Nov 2, 2021 at 5:10 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >
> > On Fri, Oct 15, 2021 at 05:13:48PM -0700, Dan Williams wrote:
> > > On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> > > >
> > > > 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.
> > > >
> > >
> > > Just fyi, I'm preparing patches to delete this driver completely as it
> > > is unused by any shipping platform. I hope to get that removal into
> > > v5.16.
> >
> > Curious if are you going to nuking it on v5.16? Otherwise it would stand
> > in the way of the last few patches to add __must_check for the final
> > add_disk() error handling changes.
> 
> True, I don't think I can get it nuked in time, so you can add my
> Reviewed-by for this one.

This patch required the previous patch in this series to also be
applied. Can I apply your Reviewed-by there too?

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-11-03  1:28         ` Jens Axboe
@ 2021-11-03 12:09           ` Luis Chamberlain
  0 siblings, 0 replies; 36+ messages in thread
From: Luis Chamberlain @ 2021-11-03 12:09 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Dan Williams, Geoff Levand, Michael Ellerman,
	Benjamin Herrenschmidt, Paul Mackerras, Jim Paris, Minchan Kim,
	Nitin Gupta, senozhatsky, Richard Weinberger, miquel.raynal,
	vigneshr, Vishal L Verma, Dave Jiang, Weiny, Ira, Keith Busch,
	Christoph Hellwig, Sagi Grimberg, linux-block, linuxppc-dev,
	linux-mtd, Linux NVDIMM, linux-nvme, Linux Kernel Mailing List

On Tue, Nov 02, 2021 at 07:28:02PM -0600, Jens Axboe wrote:
> On 11/2/21 6:49 PM, Dan Williams wrote:
> > On Tue, Nov 2, 2021 at 5:10 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >>
> >> On Fri, Oct 15, 2021 at 05:13:48PM -0700, Dan Williams wrote:
> >>> On Fri, Oct 15, 2021 at 4:53 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
> >>>>
> >>>> 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.
> >>>>
> >>>
> >>> Just fyi, I'm preparing patches to delete this driver completely as it
> >>> is unused by any shipping platform. I hope to get that removal into
> >>> v5.16.
> >>
> >> Curious if are you going to nuking it on v5.16? Otherwise it would stand
> >> in the way of the last few patches to add __must_check for the final
> >> add_disk() error handling changes.
> > 
> > True, I don't think I can get it nuked in time, so you can add my
> > Reviewed-by for this one.
> 
> Luis, I lost track of the nv* patches from this discussion. If you want
> them in 5.16 and they are reviewed, please do resend and I'll pick them
> up for the middle-of-merge-window push.

Sure thing, I'll resend whatever is left. I also noticed for some reason
I forgot to convert nvdimm/pmem and so I'll roll those new patches in,
but I suspect that those might be too late unless we get them reviewed
in time.

  Luis

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2021-11-03 12:09 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 23:52 [PATCH 00/13] block: add_disk() error handling stragglers Luis Chamberlain
2021-10-15 23:52 ` [PATCH 01/13] block/brd: add error handling support for add_disk() Luis Chamberlain
2021-10-15 23:52 ` [PATCH 02/13] nvme-multipath: " Luis Chamberlain
2021-10-16  0:01   ` Keith Busch
2021-10-16  4:39   ` Christoph Hellwig
2021-10-18  5:34   ` Chaitanya Kulkarni
2021-10-15 23:52 ` [PATCH 03/13] nvdimm/btt: do not call del_gendisk() if not needed Luis Chamberlain
2021-10-31 17:47   ` Dan Williams
2021-11-02 17:03     ` Luis Chamberlain
2021-10-15 23:52 ` [PATCH 04/13] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
2021-10-31 17:51   ` Dan Williams
2021-10-15 23:52 ` [PATCH 05/13] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
2021-10-15 23:52 ` [PATCH 06/13] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
2021-10-16  0:13   ` Dan Williams
2021-10-19 16:06     ` Luis Chamberlain
2021-11-03  0:10     ` Luis Chamberlain
2021-11-03  0:49       ` Dan Williams
2021-11-03  1:28         ` Jens Axboe
2021-11-03 12:09           ` Luis Chamberlain
2021-11-03 12:08         ` Luis Chamberlain
2021-10-15 23:52 ` [PATCH 07/13] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
2021-10-15 23:52 ` [PATCH 08/13] zram: " Luis Chamberlain
2021-10-25 16:55   ` Minchan Kim
2021-10-15 23:52 ` [PATCH 09/13] z2ram: " Luis Chamberlain
2021-10-15 23:52 ` [PATCH 10/13] ps3disk: " Luis Chamberlain
2021-10-29 15:05   ` Geoff Levand
2021-10-15 23:52 ` [PATCH 11/13] ps3vram: " Luis Chamberlain
2021-10-29 15:09   ` Geoff Levand
2021-10-15 23:52 ` [PATCH 12/13] block/sunvdc: " Luis Chamberlain
2021-10-15 23:52 ` [PATCH 13/13] mtd/ubi/block: " Luis Chamberlain
2021-10-17 15:26 ` [PATCH 00/13] block: add_disk() error handling stragglers Geoff Levand
2021-10-18 16:15   ` Luis Chamberlain
2021-10-22  3:10     ` Geoff Levand
2021-10-25 15:58       ` Luis Chamberlain
2021-10-30 17:03 ` (subset) " Jens Axboe
2021-10-30 17:07 ` 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).