linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 00/14] last set for add_disk() error handling
@ 2021-11-03 23:04 Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 01/14] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
                   ` (14 more replies)
  0 siblings, 15 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

Jens,

as requested, I've folded all pending changes into this series. This
v5 pegs on Christoph's reviewed-by tags and since I was respinning I
modified the ataprobe and floppy driver changes as he suggested.

I think this is it. The world of floppy has been exciting for v5.16.

This goes based on your axboe/for-next tree as of just a few minutes ago.

Luis Chamberlain (13):
  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()
  nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
  nvdimm/pmem: use add_disk() error handling
  z2ram: 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()
  block: update __register_blkdev() probe documentation
  ataflop: address add_disk() error handling on probe
  floppy: address add_disk() error handling on probe
  block: add __must_check for *add_disk*() callers

Tetsuo Handa (1):
  ataflop: remove ataflop_probe_lock mutex

 block/genhd.c           | 11 +++++---
 drivers/block/ataflop.c | 61 +++++++++++++++++++++++++----------------
 drivers/block/floppy.c  | 17 +++++++++---
 drivers/block/sunvdc.c  | 14 ++++++++--
 drivers/block/z2ram.c   |  7 +++--
 drivers/mtd/ubi/block.c |  8 +++++-
 drivers/nvdimm/blk.c    | 21 ++++++++++----
 drivers/nvdimm/btt.c    | 20 +++++++++-----
 drivers/nvdimm/pmem.c   | 21 ++++++++++----
 include/linux/genhd.h   |  6 ++--
 10 files changed, 127 insertions(+), 59 deletions(-)

-- 
2.33.0


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

* [PATCH v5 01/14] nvdimm/btt: use goto error labels on btt_blk_init()
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 02/14] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

This will make it easier to share common error paths.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/nvdimm/btt.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 9b9d5e506e11..5cb6d7ac6e36 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1519,6 +1519,7 @@ 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)
@@ -1534,19 +1535,22 @@ 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);
+	return rc;
 }
 
 static void btt_blk_cleanup(struct btt *btt)
-- 
2.33.0


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

* [PATCH v5 02/14] nvdimm/btt: add error handling support for add_disk()
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 01/14] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 03/14] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

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

Reviewed-by: Christoph Hellwig <hch@lst.de>
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 5cb6d7ac6e36..38ed53eeea5e 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1541,7 +1541,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.33.0


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

* [PATCH v5 03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 01/14] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 02/14] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 04/14] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
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 b6c6866f9259..4eef67918a7e 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -239,6 +239,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);
@@ -255,20 +256,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.33.0


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

* [PATCH v5 04/14] nvdimm/blk: add error handling support for add_disk()
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (2 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 03/14] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned Luis Chamberlain
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

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.

Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
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 4eef67918a7e..228c33b8d1d6 100644
--- a/drivers/nvdimm/blk.c
+++ b/drivers/nvdimm/blk.c
@@ -264,7 +264,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.33.0


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

* [PATCH v5 05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (3 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 04/14] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 06/14] nvdimm/pmem: use add_disk() error handling Luis Chamberlain
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

Prior to devm being able to use pmem_release_disk() there are other
failure which can occur for which we must account for and release the
disk for. Address those few cases.

Fixes: 3dd60fb9d95d ("nvdimm/pmem: stop using q_usage_count as external pgmap refcount")
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/nvdimm/pmem.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index c74d7bceb222..bcfc36e7295f 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -428,8 +428,10 @@ static int pmem_attach_disk(struct device *dev,
 		bb_range.end = res->end;
 	}
 
-	if (IS_ERR(addr))
-		return PTR_ERR(addr);
+	if (IS_ERR(addr)) {
+		rc = PTR_ERR(addr);
+		goto out;
+	}
 	pmem->virt_addr = addr;
 
 	blk_queue_write_cache(q, true, fua);
@@ -454,7 +456,8 @@ static int pmem_attach_disk(struct device *dev,
 		flags = DAXDEV_F_SYNC;
 	dax_dev = alloc_dax(pmem, disk->disk_name, &pmem_dax_ops, flags);
 	if (IS_ERR(dax_dev)) {
-		return PTR_ERR(dax_dev);
+		rc = PTR_ERR(dax_dev);
+		goto out;
 	}
 	dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
 	pmem->dax_dev = dax_dev;
@@ -469,8 +472,10 @@ static int pmem_attach_disk(struct device *dev,
 					  "badblocks");
 	if (!pmem->bb_state)
 		dev_warn(dev, "'badblocks' notification disabled\n");
-
 	return 0;
+out:
+	blk_cleanup_disk(pmem->disk);
+	return rc;
 }
 
 static int nd_pmem_probe(struct device *dev)
-- 
2.33.0


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

* [PATCH v5 06/14] nvdimm/pmem: use add_disk() error handling
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (4 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 07/14] z2ram: add error handling support for add_disk() Luis Chamberlain
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

Now that device_add_disk() supports returning an error, use
that. We must unwind alloc_dax() on error.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/nvdimm/pmem.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index bcfc36e7295f..37fc03058556 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -462,7 +462,9 @@ static int pmem_attach_disk(struct device *dev,
 	dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
 	pmem->dax_dev = dax_dev;
 
-	device_add_disk(dev, disk, pmem_attribute_groups);
+	rc = device_add_disk(dev, disk, pmem_attribute_groups);
+	if (rc)
+		goto out_cleanup_dax;
 	if (devm_add_action_or_reset(dev, pmem_release_disk, pmem))
 		return -ENOMEM;
 
@@ -473,6 +475,10 @@ static int pmem_attach_disk(struct device *dev,
 	if (!pmem->bb_state)
 		dev_warn(dev, "'badblocks' notification disabled\n");
 	return 0;
+
+out_cleanup_dax:
+	kill_dax(pmem->dax_dev);
+	put_dax(pmem->dax_dev);
 out:
 	blk_cleanup_disk(pmem->disk);
 	return rc;
-- 
2.33.0


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

* [PATCH v5 07/14] z2ram: add error handling support for add_disk()
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (5 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 06/14] nvdimm/pmem: use add_disk() error handling Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 08/14] block/sunvdc: " Luis Chamberlain
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
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.33.0


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

* [PATCH v5 08/14] block/sunvdc: add error handling support for add_disk()
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (6 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 07/14] z2ram: add error handling support for add_disk() Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 09/14] mtd/ubi/block: " Luis Chamberlain
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

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.

Reviewed-by: Christoph Hellwig <hch@lst.de>
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.33.0


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

* [PATCH v5 09/14] mtd/ubi/block: add error handling support for add_disk()
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (7 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 08/14] block/sunvdc: " Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 10/14] ataflop: remove ataflop_probe_lock mutex Luis Chamberlain
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

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

Reviewed-by: Christoph Hellwig <hch@lst.de>
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.33.0


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

* [PATCH v5 10/14] ataflop: remove ataflop_probe_lock mutex
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (8 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 09/14] mtd/ubi/block: " Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 11/14] block: update __register_blkdev() probe documentation Luis Chamberlain
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel,
	Tetsuo Handa, Michael Schmitz

From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>

Commit bf9c0538e485b591 ("ataflop: use a separate gendisk for each media
format") introduced ataflop_probe_lock mutex, but forgot to unlock the
mutex when atari_floppy_init() (i.e. module loading) succeeded. This will
result in double lock deadlock if ataflop_probe() is called. Also,
unregister_blkdev() must not be called from atari_floppy_init() with
ataflop_probe_lock held when atari_floppy_init() failed, for
ataflop_probe() waits for ataflop_probe_lock with major_names_lock held
(i.e. AB-BA deadlock).

__register_blkdev() needs to be called last in order to avoid calling
ataflop_probe() when atari_floppy_init() is about to fail, for memory for
completing already-started ataflop_probe() safely will be released as soon
as atari_floppy_init() released ataflop_probe_lock mutex.

As with commit 8b52d8be86d72308 ("loop: reorder loop_exit"),
unregister_blkdev() needs to be called first in order to avoid calling
ataflop_alloc_disk() from ataflop_probe() after del_gendisk() from
atari_floppy_exit().

By relocating __register_blkdev() / unregister_blkdev() as explained above,
we can remove ataflop_probe_lock mutex, for probe function and __exit
function are serialized by major_names_lock mutex.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Fixes: bf9c0538e485b591 ("ataflop: use a separate gendisk for each media format")
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Tested-by: Michael Schmitz <schmitzmic@gmail.com>
---
 drivers/block/ataflop.c | 47 +++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index d14bdc3589b2..170dd193cef6 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2008,8 +2008,6 @@ static int ataflop_alloc_disk(unsigned int drive, unsigned int type)
 	return 0;
 }
 
-static DEFINE_MUTEX(ataflop_probe_lock);
-
 static void ataflop_probe(dev_t dev)
 {
 	int drive = MINOR(dev) & 3;
@@ -2020,14 +2018,32 @@ static void ataflop_probe(dev_t dev)
 
 	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
 		return;
-	mutex_lock(&ataflop_probe_lock);
 	if (!unit[drive].disk[type]) {
 		if (ataflop_alloc_disk(drive, type) == 0) {
 			add_disk(unit[drive].disk[type]);
 			unit[drive].registered[type] = true;
 		}
 	}
-	mutex_unlock(&ataflop_probe_lock);
+}
+
+static void atari_floppy_cleanup(void)
+{
+	int i;
+	int type;
+
+	for (i = 0; i < FD_MAX_UNITS; i++) {
+		for (type = 0; type < NUM_DISK_MINORS; type++) {
+			if (!unit[i].disk[type])
+				continue;
+			del_gendisk(unit[i].disk[type]);
+			blk_cleanup_queue(unit[i].disk[type]->queue);
+			put_disk(unit[i].disk[type]);
+		}
+		blk_mq_free_tag_set(&unit[i].tag_set);
+	}
+
+	del_timer_sync(&fd_timer);
+	atari_stram_free(DMABuffer);
 }
 
 static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs)
@@ -2053,11 +2069,6 @@ static int __init atari_floppy_init (void)
 		/* Amiga, Mac, ... don't have Atari-compatible floppy :-) */
 		return -ENODEV;
 
-	mutex_lock(&ataflop_probe_lock);
-	ret = __register_blkdev(FLOPPY_MAJOR, "fd", ataflop_probe);
-	if (ret)
-		goto out_unlock;
-
 	for (i = 0; i < FD_MAX_UNITS; i++) {
 		memset(&unit[i].tag_set, 0, sizeof(unit[i].tag_set));
 		unit[i].tag_set.ops = &ataflop_mq_ops;
@@ -2113,7 +2124,12 @@ static int __init atari_floppy_init (void)
 	       UseTrackbuffer ? "" : "no ");
 	config_types();
 
-	return 0;
+	ret = __register_blkdev(FLOPPY_MAJOR, "fd", ataflop_probe);
+	if (ret) {
+		printk(KERN_ERR "atari_floppy_init: cannot register block device\n");
+		atari_floppy_cleanup();
+	}
+	return ret;
 
 err_out_dma:
 	atari_stram_free(DMABuffer);
@@ -2121,9 +2137,6 @@ static int __init atari_floppy_init (void)
 	while (--i >= 0)
 		atari_cleanup_floppy_disk(&unit[i]);
 
-	unregister_blkdev(FLOPPY_MAJOR, "fd");
-out_unlock:
-	mutex_unlock(&ataflop_probe_lock);
 	return ret;
 }
 
@@ -2168,14 +2181,8 @@ __setup("floppy=", atari_floppy_setup);
 
 static void __exit atari_floppy_exit(void)
 {
-	int i;
-
-	for (i = 0; i < FD_MAX_UNITS; i++)
-		atari_cleanup_floppy_disk(&unit[i]);
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
-
-	del_timer_sync(&fd_timer);
-	atari_stram_free( DMABuffer );
+	atari_floppy_cleanup();
 }
 
 module_init(atari_floppy_init)
-- 
2.33.0


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

* [PATCH v5 11/14] block: update __register_blkdev() probe documentation
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (9 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 10/14] ataflop: remove ataflop_probe_lock mutex Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 12/14] ataflop: address add_disk() error handling on probe Luis Chamberlain
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

__register_blkdev() is used to register a probe callback, and
that callback is typically used to call add_disk(). Now that
we are able to capture errors for add_disk(), we need to fix
those probe calls where add_disk() fails and clean up resources.

We don't extend the probe call to return the error given:

1) we'd have to always special-case the case where the disk
   was already present, as otherwise concurrent requests to
   open an existing block device would fail, and this would be
   a userspace visible change
2) the error from ilookup() on blkdev_get_no_open() is sufficient
3) The only thing the probe call is used for is to support
   pre-devtmpfs, pre-udev semantics that want to create disks when
   their pre-created device node is accessed, and so we don't care
   for failures on probe there.

Expand documentation for the probe callback to ensure users cleanup
resources if add_disk() is used and to clarify this interface may be
removed in the future.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/genhd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/genhd.c b/block/genhd.c
index 4ed87f25276a..2f5b7e24e88a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -213,7 +213,10 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
  * @major: the requested major device number [1..BLKDEV_MAJOR_MAX-1]. If
  *         @major = 0, try to allocate any unused major number.
  * @name: the name of the new block device as a zero terminated string
- * @probe: allback that is called on access to any minor number of @major
+ * @probe: pre-devtmpfs / pre-udev callback used to create disks when their
+ *	   pre-created device node is accessed. When a probe call uses
+ *	   add_disk() and it fails the driver must cleanup resources. This
+ *	   interface may soon be removed.
  *
  * The @name must be unique within the system.
  *
-- 
2.33.0


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

* [PATCH v5 12/14] ataflop: address add_disk() error handling on probe
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (10 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 11/14] block: update __register_blkdev() probe documentation Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 13/14] floppy: " Luis Chamberlain
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/ataflop.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 170dd193cef6..de8c3785899a 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2018,12 +2018,18 @@ static void ataflop_probe(dev_t dev)
 
 	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
 		return;
-	if (!unit[drive].disk[type]) {
-		if (ataflop_alloc_disk(drive, type) == 0) {
-			add_disk(unit[drive].disk[type]);
-			unit[drive].registered[type] = true;
-		}
-	}
+	if (unit[drive].disk[type])
+		return
+	if (ataflop_alloc_disk(drive, type))
+		return;
+	if (add_disk(unit[drive].disk[type]))
+		goto cleanup_disk;
+	unit[drive].registered[type] = true;
+	return;
+
+cleanup_disk:
+	blk_cleanup_disk(unit[drive].disk[type]);
+	unit[drive].disk[type] = NULL;
 }
 
 static void atari_floppy_cleanup(void)
-- 
2.33.0


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

* [PATCH v5 13/14] floppy: address add_disk() error handling on probe
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (11 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 12/14] ataflop: address add_disk() error handling on probe Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-03 23:04 ` [PATCH v5 14/14] block: add __must_check for *add_disk*() callers Luis Chamberlain
  2021-11-04 11:49 ` [PATCH v5 00/14] last set for add_disk() error handling Jens Axboe
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/floppy.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 3873e789478e..c4267da716fe 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4528,10 +4528,19 @@ static void floppy_probe(dev_t dev)
 		return;
 
 	mutex_lock(&floppy_probe_lock);
-	if (!disks[drive][type]) {
-		if (floppy_alloc_disk(drive, type) == 0)
-			add_disk(disks[drive][type]);
-	}
+	if (disks[drive][type])
+		goto out;
+	if (floppy_alloc_disk(drive, type))
+		goto out;
+	if (add_disk(disks[drive][type]))
+		goto cleanup_disk;
+out:
+	mutex_unlock(&floppy_probe_lock);
+	return;
+
+cleanup_disk:
+	blk_cleanup_disk(disks[drive][type]);
+	disks[drive][type] = NULL;
 	mutex_unlock(&floppy_probe_lock);
 }
 
-- 
2.33.0


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

* [PATCH v5 14/14] block: add __must_check for *add_disk*() callers
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (12 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 13/14] floppy: " Luis Chamberlain
@ 2021-11-03 23:04 ` Luis Chamberlain
  2021-11-04 11:49 ` [PATCH v5 00/14] last set for add_disk() error handling Jens Axboe
  14 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-03 23:04 UTC (permalink / raw)
  To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
	dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
	song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
  Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel

Now that we have done a spring cleaning on all drivers and added
error checking / handling, let's keep it that way and ensure
no new drivers fail to stick with it.

Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/genhd.c         | 6 +++---
 include/linux/genhd.h | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 2f5b7e24e88a..2263f7862241 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -394,8 +394,8 @@ static void disk_scan_partitions(struct gendisk *disk)
  * This function registers the partitioning information in @disk
  * with the kernel.
  */
-int device_add_disk(struct device *parent, struct gendisk *disk,
-		     const struct attribute_group **groups)
+int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
+				 const struct attribute_group **groups)
 
 {
 	struct device *ddev = disk_to_dev(disk);
@@ -542,7 +542,7 @@ int device_add_disk(struct device *parent, struct gendisk *disk,
 out_free_ext_minor:
 	if (disk->major == BLOCK_EXT_MAJOR)
 		blk_free_ext_minor(disk->first_minor);
-	return WARN_ON_ONCE(ret); /* keep until all callers handle errors */
+	return ret;
 }
 EXPORT_SYMBOL(device_add_disk);
 
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 59eabbc3a36b..f7d6810e68b3 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -205,9 +205,9 @@ static inline dev_t disk_devt(struct gendisk *disk)
 void disk_uevent(struct gendisk *disk, enum kobject_action action);
 
 /* block/genhd.c */
-int device_add_disk(struct device *parent, struct gendisk *disk,
-		const struct attribute_group **groups);
-static inline int add_disk(struct gendisk *disk)
+int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
+				 const struct attribute_group **groups);
+static inline int __must_check add_disk(struct gendisk *disk)
 {
 	return device_add_disk(NULL, disk, NULL);
 }
-- 
2.33.0


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

* Re: [PATCH v5 00/14] last set for add_disk() error handling
  2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
                   ` (13 preceding siblings ...)
  2021-11-03 23:04 ` [PATCH v5 14/14] block: add __must_check for *add_disk*() callers Luis Chamberlain
@ 2021-11-04 11:49 ` Jens Axboe
  2021-11-04 12:53   ` Jens Axboe
  14 siblings, 1 reply; 20+ messages in thread
From: Jens Axboe @ 2021-11-04 11:49 UTC (permalink / raw)
  To: miquel.raynal, martin.petersen, hare, Luis Chamberlain, jack,
	hch, song, dave.jiang, richard, vishal.l.verma, penguin-kernel,
	tj, ira.weiny, vigneshr, dan.j.williams, ming.lei, efremov
  Cc: linux-raid, linux-mtd, linux-kernel, linux-block, linux-scsi

On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
> Jens,
> 
> as requested, I've folded all pending changes into this series. This
> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
> modified the ataprobe and floppy driver changes as he suggested.
> 
> I think this is it. The world of floppy has been exciting for v5.16.
> 
> [...]

Applied, thanks!

[01/14] nvdimm/btt: use goto error labels on btt_blk_init()
        commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
[02/14] nvdimm/btt: add error handling support for add_disk()
        commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
[03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
        commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
[04/14] nvdimm/blk: add error handling support for add_disk()
        commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
[05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
        commit: accf58afb689f81daadde24080ea1164ad2db75f
[06/14] nvdimm/pmem: use add_disk() error handling
        commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
[07/14] z2ram: add error handling support for add_disk()
        commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
[08/14] block/sunvdc: add error handling support for add_disk()
        commit: f583eaef0af39b792d74e39721b5ba4b6948a270
[09/14] mtd/ubi/block: add error handling support for add_disk()
        commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
[10/14] ataflop: remove ataflop_probe_lock mutex
        commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
[11/14] block: update __register_blkdev() probe documentation
        commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
[12/14] ataflop: address add_disk() error handling on probe
        commit: 46a7db492e7a27408bc164cbe6424683e79529b0
[13/14] floppy: address add_disk() error handling on probe
        commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
[14/14] block: add __must_check for *add_disk*() callers
        commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6

Best regards,
-- 
Jens Axboe



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

* Re: [PATCH v5 00/14] last set for add_disk() error handling
  2021-11-04 11:49 ` [PATCH v5 00/14] last set for add_disk() error handling Jens Axboe
@ 2021-11-04 12:53   ` Jens Axboe
  2021-11-04 17:07     ` Luis Chamberlain
  0 siblings, 1 reply; 20+ messages in thread
From: Jens Axboe @ 2021-11-04 12:53 UTC (permalink / raw)
  To: miquel.raynal, martin.petersen, hare, Luis Chamberlain, jack,
	hch, song, dave.jiang, richard, vishal.l.verma, penguin-kernel,
	tj, ira.weiny, vigneshr, dan.j.williams, ming.lei, efremov
  Cc: linux-raid, linux-mtd, linux-kernel, linux-block, linux-scsi

On 11/4/21 5:49 AM, Jens Axboe wrote:
> On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
>> Jens,
>>
>> as requested, I've folded all pending changes into this series. This
>> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
>> modified the ataprobe and floppy driver changes as he suggested.
>>
>> I think this is it. The world of floppy has been exciting for v5.16.
>>
>> [...]
> 
> Applied, thanks!
> 
> [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
>         commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
> [02/14] nvdimm/btt: add error handling support for add_disk()
>         commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
> [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
>         commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
> [04/14] nvdimm/blk: add error handling support for add_disk()
>         commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
> [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
>         commit: accf58afb689f81daadde24080ea1164ad2db75f
> [06/14] nvdimm/pmem: use add_disk() error handling
>         commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
> [07/14] z2ram: add error handling support for add_disk()
>         commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
> [08/14] block/sunvdc: add error handling support for add_disk()
>         commit: f583eaef0af39b792d74e39721b5ba4b6948a270
> [09/14] mtd/ubi/block: add error handling support for add_disk()
>         commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
> [10/14] ataflop: remove ataflop_probe_lock mutex
>         commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
> [11/14] block: update __register_blkdev() probe documentation
>         commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
> [12/14] ataflop: address add_disk() error handling on probe
>         commit: 46a7db492e7a27408bc164cbe6424683e79529b0
> [13/14] floppy: address add_disk() error handling on probe
>         commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
> [14/14] block: add __must_check for *add_disk*() callers
>         commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6

rivers/scsi/sd.c: In function ‘sd_probe’:
drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
 3573 |         device_add_disk(dev, gd, NULL);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/sr.c: In function ‘sr_probe’:
drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
  731 |         device_add_disk(&sdev->sdev_gendev, disk, NULL);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Dropping the last two patches...

-- 
Jens Axboe


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

* Re: [PATCH v5 00/14] last set for add_disk() error handling
  2021-11-04 12:53   ` Jens Axboe
@ 2021-11-04 17:07     ` Luis Chamberlain
  2021-11-04 17:10       ` Jens Axboe
  0 siblings, 1 reply; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-04 17:07 UTC (permalink / raw)
  To: Jens Axboe, martin.petersen
  Cc: miquel.raynal, hare, jack, hch, song, dave.jiang, richard,
	vishal.l.verma, penguin-kernel, tj, ira.weiny, vigneshr,
	dan.j.williams, ming.lei, efremov, linux-raid, linux-mtd,
	linux-kernel, linux-block, linux-scsi

On Thu, Nov 04, 2021 at 06:53:34AM -0600, Jens Axboe wrote:
> On 11/4/21 5:49 AM, Jens Axboe wrote:
> > On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
> >> Jens,
> >>
> >> as requested, I've folded all pending changes into this series. This
> >> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
> >> modified the ataprobe and floppy driver changes as he suggested.
> >>
> >> I think this is it. The world of floppy has been exciting for v5.16.
> >>
> >> [...]
> > 
> > Applied, thanks!
> > 
> > [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
> >         commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
> > [02/14] nvdimm/btt: add error handling support for add_disk()
> >         commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
> > [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
> >         commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
> > [04/14] nvdimm/blk: add error handling support for add_disk()
> >         commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
> > [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
> >         commit: accf58afb689f81daadde24080ea1164ad2db75f
> > [06/14] nvdimm/pmem: use add_disk() error handling
> >         commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
> > [07/14] z2ram: add error handling support for add_disk()
> >         commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
> > [08/14] block/sunvdc: add error handling support for add_disk()
> >         commit: f583eaef0af39b792d74e39721b5ba4b6948a270
> > [09/14] mtd/ubi/block: add error handling support for add_disk()
> >         commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
> > [10/14] ataflop: remove ataflop_probe_lock mutex
> >         commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
> > [11/14] block: update __register_blkdev() probe documentation
> >         commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
> > [12/14] ataflop: address add_disk() error handling on probe
> >         commit: 46a7db492e7a27408bc164cbe6424683e79529b0
> > [13/14] floppy: address add_disk() error handling on probe
> >         commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
> > [14/14] block: add __must_check for *add_disk*() callers
> >         commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
> 
> rivers/scsi/sd.c: In function ‘sd_probe’:
> drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>  3573 |         device_add_disk(dev, gd, NULL);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/scsi/sr.c: In function ‘sr_probe’:
> drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>   731 |         device_add_disk(&sdev->sdev_gendev, disk, NULL);
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> 
> Dropping the last two patches...

Martin K Peterson has the respective patches needed queued up on his tree
for v5.16:

https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=e9d658c2175b95a8f091b12ddefb271683aeacd9
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=2a7a891f4c406822801ecd676b076c64de072c9e

Would the last patch be sent once that gets to Linus?

Also curious why drop the last two patches instead just the last one for
now?

  Luis

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

* Re: [PATCH v5 00/14] last set for add_disk() error handling
  2021-11-04 17:07     ` Luis Chamberlain
@ 2021-11-04 17:10       ` Jens Axboe
  2021-11-04 18:16         ` Luis Chamberlain
  0 siblings, 1 reply; 20+ messages in thread
From: Jens Axboe @ 2021-11-04 17:10 UTC (permalink / raw)
  To: Luis Chamberlain, martin.petersen
  Cc: miquel.raynal, hare, jack, hch, song, dave.jiang, richard,
	vishal.l.verma, penguin-kernel, tj, ira.weiny, vigneshr,
	dan.j.williams, ming.lei, efremov, linux-raid, linux-mtd,
	linux-kernel, linux-block, linux-scsi

On 11/4/21 11:07 AM, Luis Chamberlain wrote:
> On Thu, Nov 04, 2021 at 06:53:34AM -0600, Jens Axboe wrote:
>> On 11/4/21 5:49 AM, Jens Axboe wrote:
>>> On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
>>>> Jens,
>>>>
>>>> as requested, I've folded all pending changes into this series. This
>>>> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
>>>> modified the ataprobe and floppy driver changes as he suggested.
>>>>
>>>> I think this is it. The world of floppy has been exciting for v5.16.
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
>>>         commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
>>> [02/14] nvdimm/btt: add error handling support for add_disk()
>>>         commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
>>> [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
>>>         commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
>>> [04/14] nvdimm/blk: add error handling support for add_disk()
>>>         commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
>>> [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
>>>         commit: accf58afb689f81daadde24080ea1164ad2db75f
>>> [06/14] nvdimm/pmem: use add_disk() error handling
>>>         commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
>>> [07/14] z2ram: add error handling support for add_disk()
>>>         commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
>>> [08/14] block/sunvdc: add error handling support for add_disk()
>>>         commit: f583eaef0af39b792d74e39721b5ba4b6948a270
>>> [09/14] mtd/ubi/block: add error handling support for add_disk()
>>>         commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
>>> [10/14] ataflop: remove ataflop_probe_lock mutex
>>>         commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
>>> [11/14] block: update __register_blkdev() probe documentation
>>>         commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
>>> [12/14] ataflop: address add_disk() error handling on probe
>>>         commit: 46a7db492e7a27408bc164cbe6424683e79529b0
>>> [13/14] floppy: address add_disk() error handling on probe
>>>         commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
>>> [14/14] block: add __must_check for *add_disk*() callers
>>>         commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
>>
>> rivers/scsi/sd.c: In function ‘sd_probe’:
>> drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>>  3573 |         device_add_disk(dev, gd, NULL);
>>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/scsi/sr.c: In function ‘sr_probe’:
>> drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>>   731 |         device_add_disk(&sdev->sdev_gendev, disk, NULL);
>>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>>
>> Dropping the last two patches...
> 
> Martin K Peterson has the respective patches needed queued up on his tree
> for v5.16:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=e9d658c2175b95a8f091b12ddefb271683aeacd9
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=2a7a891f4c406822801ecd676b076c64de072c9e
> 
> Would the last patch be sent once that gets to Linus?

But that dependency wasn't clear in the patches posted, and it leaves me
wondering if there are others? I obviously can't queue up a patch that
adds a must_check to a function, when we still have callers that don't
properly check it.

That should have been made clear, and that last patch never should've
been part of the series. Please send it once Linus's tree has all
callers checking the result.

> Also curious why drop the last two patches instead just the last one for
> now?

Sorry, meant just the last one.

-- 
Jens Axboe


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

* Re: [PATCH v5 00/14] last set for add_disk() error handling
  2021-11-04 17:10       ` Jens Axboe
@ 2021-11-04 18:16         ` Luis Chamberlain
  0 siblings, 0 replies; 20+ messages in thread
From: Luis Chamberlain @ 2021-11-04 18:16 UTC (permalink / raw)
  To: Jens Axboe
  Cc: martin.petersen, miquel.raynal, hare, jack, hch, song,
	dave.jiang, richard, vishal.l.verma, penguin-kernel, tj,
	ira.weiny, vigneshr, dan.j.williams, ming.lei, efremov,
	linux-raid, linux-mtd, linux-kernel, linux-block, linux-scsi

On Thu, Nov 04, 2021 at 11:10:48AM -0600, Jens Axboe wrote:
> On 11/4/21 11:07 AM, Luis Chamberlain wrote:
> > On Thu, Nov 04, 2021 at 06:53:34AM -0600, Jens Axboe wrote:
> >> On 11/4/21 5:49 AM, Jens Axboe wrote:
> >>> On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
> >>>> Jens,
> >>>>
> >>>> as requested, I've folded all pending changes into this series. This
> >>>> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
> >>>> modified the ataprobe and floppy driver changes as he suggested.
> >>>>
> >>>> I think this is it. The world of floppy has been exciting for v5.16.
> >>>>
> >>>> [...]
> >>>
> >>> Applied, thanks!
> >>>
> >>> [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
> >>>         commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
> >>> [02/14] nvdimm/btt: add error handling support for add_disk()
> >>>         commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
> >>> [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
> >>>         commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
> >>> [04/14] nvdimm/blk: add error handling support for add_disk()
> >>>         commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
> >>> [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
> >>>         commit: accf58afb689f81daadde24080ea1164ad2db75f
> >>> [06/14] nvdimm/pmem: use add_disk() error handling
> >>>         commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
> >>> [07/14] z2ram: add error handling support for add_disk()
> >>>         commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
> >>> [08/14] block/sunvdc: add error handling support for add_disk()
> >>>         commit: f583eaef0af39b792d74e39721b5ba4b6948a270
> >>> [09/14] mtd/ubi/block: add error handling support for add_disk()
> >>>         commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
> >>> [10/14] ataflop: remove ataflop_probe_lock mutex
> >>>         commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
> >>> [11/14] block: update __register_blkdev() probe documentation
> >>>         commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
> >>> [12/14] ataflop: address add_disk() error handling on probe
> >>>         commit: 46a7db492e7a27408bc164cbe6424683e79529b0
> >>> [13/14] floppy: address add_disk() error handling on probe
> >>>         commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
> >>> [14/14] block: add __must_check for *add_disk*() callers
> >>>         commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
> >>
> >> rivers/scsi/sd.c: In function ‘sd_probe’:
> >> drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> >>  3573 |         device_add_disk(dev, gd, NULL);
> >>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> drivers/scsi/sr.c: In function ‘sr_probe’:
> >> drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> >>   731 |         device_add_disk(&sdev->sdev_gendev, disk, NULL);
> >>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >>
> >> Dropping the last two patches...
> > 
> > Martin K Peterson has the respective patches needed queued up on his tree
> > for v5.16:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=e9d658c2175b95a8f091b12ddefb271683aeacd9
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=2a7a891f4c406822801ecd676b076c64de072c9e
> > 
> > Would the last patch be sent once that gets to Linus?
> 
> But that dependency wasn't clear in the patches posted,

Sorry my mistake.

> and it leaves me
> wondering if there are others?

There should not be, becauase at least my patches on top of linux-next
compiles fine as per 0-day builds on tons of configs.

> I obviously can't queue up a patch that
> adds a must_check to a function, when we still have callers that don't
> properly check it.

Absolutely.

> That should have been made clear, and that last patch never should've
> been part of the series. Please send it once Linus's tree has all
> callers checking the result.

Indeed. Will track and will do.

> > Also curious why drop the last two patches instead just the last one for
> > now?
> 
> Sorry, meant just the last one.

Ah ok!

  Luis

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

end of thread, other threads:[~2021-11-04 18:16 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 23:04 [PATCH v5 00/14] last set for add_disk() error handling Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 01/14] nvdimm/btt: use goto error labels on btt_blk_init() Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 02/14] nvdimm/btt: add error handling support for add_disk() Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 03/14] nvdimm/blk: avoid calling del_gendisk() on early failures Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 04/14] nvdimm/blk: add error handling support for add_disk() Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 06/14] nvdimm/pmem: use add_disk() error handling Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 07/14] z2ram: add error handling support for add_disk() Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 08/14] block/sunvdc: " Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 09/14] mtd/ubi/block: " Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 10/14] ataflop: remove ataflop_probe_lock mutex Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 11/14] block: update __register_blkdev() probe documentation Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 12/14] ataflop: address add_disk() error handling on probe Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 13/14] floppy: " Luis Chamberlain
2021-11-03 23:04 ` [PATCH v5 14/14] block: add __must_check for *add_disk*() callers Luis Chamberlain
2021-11-04 11:49 ` [PATCH v5 00/14] last set for add_disk() error handling Jens Axboe
2021-11-04 12:53   ` Jens Axboe
2021-11-04 17:07     ` Luis Chamberlain
2021-11-04 17:10       ` Jens Axboe
2021-11-04 18:16         ` Luis Chamberlain

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).