dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
* [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions
@ 2021-08-30 21:25 Luis Chamberlain
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk() Luis Chamberlain
                   ` (7 more replies)
  0 siblings, 8 replies; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Luis Chamberlain

Jens,

I think this first set is ready, but pending review of just two patches:

  * mmc/core/block
  * dm

All other patches have a respective Reviewed-by tag. The above two
patches were integrated back into the series once I understood
Christoph's concerns, and adjusted the patch as such.

This goes rebased onto your for-next as of today. If anyone wants to
explore the pending full set this is up on my linux-next branch
20210830-for-axboe-add-disk-error-handling-next [0].

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

Luis Chamberlain (8):
  scsi/sd: add error handling support for add_disk()
  scsi/sr: add error handling support for add_disk()
  nvme: add error handling support for add_disk()
  mmc/core/block: add error handling support for add_disk()
  md: add error handling support for add_disk()
  dm: add add_disk() error handling
  loop: add error handling support for add_disk()
  nbd: add error handling support for add_disk()

 drivers/block/loop.c     | 9 ++++++++-
 drivers/block/nbd.c      | 6 +++++-
 drivers/md/dm.c          | 4 +++-
 drivers/md/md.c          | 7 ++++++-
 drivers/mmc/core/block.c | 7 ++++++-
 drivers/nvme/host/core.c | 9 ++++++++-
 drivers/scsi/sd.c        | 6 +++++-
 drivers/scsi/sr.c        | 5 ++++-
 8 files changed, 45 insertions(+), 8 deletions(-)

-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:13   ` Hannes Reinecke
  2021-09-07  1:29   ` Ming Lei
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 2/8] scsi/sr: " Luis Chamberlain
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig,
	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.

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

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 610ebba0d66e..8c1273fff23e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3487,7 +3487,11 @@ static int sd_probe(struct device *dev)
 		pm_runtime_set_autosuspend_delay(dev,
 			sdp->host->hostt->rpm_autosuspend_delay);
 	}
-	device_add_disk(dev, gd, NULL);
+
+	error = device_add_disk(dev, gd, NULL);
+	if (error)
+		goto out_free_index;
+
 	if (sdkp->capacity)
 		sd_dif_config_host(sdkp);
 
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk() Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:13   ` Hannes Reinecke
  2021-09-07  1:37   ` Ming Lei
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 3/8] nvme: " Luis Chamberlain
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig,
	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.

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

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 2942a4ec9bdd..72fd21844367 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -779,7 +779,10 @@ static int sr_probe(struct device *dev)
 	dev_set_drvdata(dev, cd);
 	disk->flags |= GENHD_FL_REMOVABLE;
 	sr_revalidate_disk(cd);
-	device_add_disk(&sdev->sdev_gendev, disk, NULL);
+
+	error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
+	if (error)
+		goto fail_minor;
 
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 3/8] nvme: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk() Luis Chamberlain
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 2/8] scsi/sr: " Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:16   ` Hannes Reinecke
  2021-09-06  8:09   ` Christoph Hellwig
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 4/8] mmc/core/block: " Luis Chamberlain
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig,
	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.

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

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 8679a108f571..687d3be563a3 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3763,7 +3763,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 
 	nvme_get_ctrl(ctrl);
 
-	device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
+	if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
+		goto out_cleanup_ns_from_list;
+
 	if (!nvme_ns_head_multipath(ns->head))
 		nvme_add_ns_cdev(ns);
 
@@ -3773,6 +3775,11 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
 
 	return;
 
+ out_cleanup_ns_from_list:
+	nvme_put_ctrl(ctrl);
+	down_write(&ctrl->namespaces_rwsem);
+	list_del_init(&ns->list);
+	up_write(&ctrl->namespaces_rwsem);
  out_unlink_ns:
 	mutex_lock(&ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 4/8] mmc/core/block: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (2 preceding siblings ...)
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 3/8] nvme: " Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06 17:10   ` Ulf Hansson
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 5/8] md: " Luis Chamberlain
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, 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.

The caller only cleanups the disk if we pass on an allocated md
but on error we return return ERR_PTR(ret), and so we must do all
the unwinding ourselves.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/mmc/core/block.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 6a15fdf6e5f2..9b2856aa6231 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2453,9 +2453,14 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	/* used in ->open, must be set before add_disk: */
 	if (area_type == MMC_BLK_DATA_AREA_MAIN)
 		dev_set_drvdata(&card->dev, md);
-	device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
+	ret = device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
+	if (ret)
+		goto err_cleanup_queue;
 	return md;
 
+ err_cleanup_queue:
+	blk_cleanup_queue(md->disk->queue);
+	blk_mq_free_tag_set(&md->queue.tag_set);
  err_kfree:
 	kfree(md);
  out:
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 5/8] md: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (3 preceding siblings ...)
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 4/8] mmc/core/block: " Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:17   ` Hannes Reinecke
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 6/8] dm: add add_disk() error handling Luis Chamberlain
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig,
	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 just do the unwinding of what was not done before, and are
sure to unlock prior to bailing.

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

diff --git a/drivers/md/md.c b/drivers/md/md.c
index ae8fe54ea358..5c0d3536d7c7 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5704,7 +5704,11 @@ static int md_alloc(dev_t dev, char *name)
 	 * through to md_open, so make sure it doesn't get too far
 	 */
 	mutex_lock(&mddev->open_mutex);
-	add_disk(disk);
+	error = add_disk(disk);
+	if (error) {
+		blk_cleanup_disk(disk);
+		goto abort_unlock;
+	}
 
 	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
 	if (error) {
@@ -5718,6 +5722,7 @@ static int md_alloc(dev_t dev, char *name)
 	if (mddev->kobj.sd &&
 	    sysfs_create_group(&mddev->kobj, &md_bitmap_group))
 		pr_debug("pointless warning\n");
+ abort_unlock:
 	mutex_unlock(&mddev->open_mutex);
  abort:
 	mutex_unlock(&disks_mutex);
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 6/8] dm: add add_disk() error handling
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (4 preceding siblings ...)
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 5/8] md: " Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:19   ` Hannes Reinecke
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 7/8] loop: add error handling support for add_disk() Luis Chamberlain
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 8/8] nbd: " Luis Chamberlain
  7 siblings, 1 reply; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, 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.

There are two calls to dm_setup_md_queue() which can fail then,
one on dm_early_create() and we can easily see that the error path
there calls dm_destroy in the error path. The other use case is on
the ioctl table_load case. If that fails userspace needs to call
the DM_DEV_REMOVE_CMD to cleanup the state - similar to any other
failure.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/md/dm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 7981b7287628..4e6b9d6ac508 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2077,7 +2077,9 @@ int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t)
 	if (r)
 		return r;
 
-	add_disk(md->disk);
+	r = add_disk(md->disk);
+	if (r)
+		return r;
 
 	r = dm_sysfs_init(md);
 	if (r) {
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 7/8] loop: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (5 preceding siblings ...)
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 6/8] dm: add add_disk() error handling Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:19   ` Hannes Reinecke
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 8/8] nbd: " Luis Chamberlain
  7 siblings, 1 reply; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig,
	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.

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

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index fa1c298a8cfb..b8b9e2349e77 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2393,10 +2393,17 @@ static int loop_add(int i)
 	disk->events		= DISK_EVENT_MEDIA_CHANGE;
 	disk->event_flags	= DISK_EVENT_FLAG_UEVENT;
 	sprintf(disk->disk_name, "loop%d", i);
-	add_disk(disk);
+
+	err = add_disk(disk);
+	if (err)
+		goto out_cleanup_disk;
+
 	mutex_unlock(&loop_ctl_mutex);
+
 	return i;
 
+out_cleanup_disk:
+	blk_cleanup_disk(disk);
 out_cleanup_tags:
 	blk_mq_free_tag_set(&lo->tag_set);
 out_free_idr:
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* [dm-devel] [PATCH v3 8/8] nbd: add error handling support for add_disk()
  2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (6 preceding siblings ...)
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 7/8] loop: add error handling support for add_disk() Luis Chamberlain
@ 2021-08-30 21:25 ` Luis Chamberlain
  2021-09-06  6:20   ` Hannes Reinecke
  7 siblings, 1 reply; 23+ messages in thread
From: Luis Chamberlain @ 2021-08-30 21:25 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig,
	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.

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

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 5170a630778d..741365295157 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -1757,7 +1757,9 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
 	disk->fops = &nbd_fops;
 	disk->private_data = nbd;
 	sprintf(disk->disk_name, "nbd%d", index);
-	add_disk(disk);
+	err = add_disk(disk);
+	if (err)
+		goto out_err_disk;
 
 	/*
 	 * Now publish the device.
@@ -1766,6 +1768,8 @@ static struct nbd_device *nbd_dev_add(int index, unsigned int refs)
 	nbd_total_devices++;
 	return nbd;
 
+out_err_disk:
+	blk_cleanup_disk(disk);
 out_free_idr:
 	mutex_lock(&nbd_index_mutex);
 	idr_remove(&nbd_index_idr, index);
-- 
2.30.2

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-06  6:13   ` Hannes Reinecke
  2021-09-07  1:29   ` Ming Lei
  1 sibling, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:13 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig

On 8/30/21 11:25 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/scsi/sd.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 2/8] scsi/sr: " Luis Chamberlain
@ 2021-09-06  6:13   ` Hannes Reinecke
  2021-09-07  1:37   ` Ming Lei
  1 sibling, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:13 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig

On 8/30/21 11:25 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/scsi/sr.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 3/8] nvme: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 3/8] nvme: " Luis Chamberlain
@ 2021-09-06  6:16   ` Hannes Reinecke
  2021-09-06  8:09     ` Christoph Hellwig
  2021-09-06  8:09   ` Christoph Hellwig
  1 sibling, 1 reply; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:16 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig

On 8/30/21 11:25 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/nvme/host/core.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 8679a108f571..687d3be563a3 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3763,7 +3763,9 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
>   
>   	nvme_get_ctrl(ctrl);
>   
> -	device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups);
> +	if (device_add_disk(ctrl->device, ns->disk, nvme_ns_id_attr_groups))
> +		goto out_cleanup_ns_from_list;
> +
>   	if (!nvme_ns_head_multipath(ns->head))
>   		nvme_add_ns_cdev(ns);
>   
> @@ -3773,6 +3775,11 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid,
>   
>   	return;
>   
> + out_cleanup_ns_from_list:
> +	nvme_put_ctrl(ctrl);
> +	down_write(&ctrl->namespaces_rwsem);
> +	list_del_init(&ns->list);
> +	up_write(&ctrl->namespaces_rwsem);
>    out_unlink_ns:
>   	mutex_lock(&ctrl->subsys->lock);
>   	list_del_rcu(&ns->siblings);
> 
I would rather turn this around, and call 'nvme_put_ctrl()' after 
removing the namespace from the list. But it's probably more a style 
issue, come to think of it.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 5/8] md: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 5/8] md: " Luis Chamberlain
@ 2021-09-06  6:17   ` Hannes Reinecke
  0 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:17 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig

On 8/30/21 11:25 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.
> 
> We just do the unwinding of what was not done before, and are
> sure to unlock prior to bailing.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/md/md.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 6/8] dm: add add_disk() error handling
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 6/8] dm: add add_disk() error handling Luis Chamberlain
@ 2021-09-06  6:19   ` Hannes Reinecke
  0 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:19 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd

On 8/30/21 11:25 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.
> 
> There are two calls to dm_setup_md_queue() which can fail then,
> one on dm_early_create() and we can easily see that the error path
> there calls dm_destroy in the error path. The other use case is on
> the ioctl table_load case. If that fails userspace needs to call
> the DM_DEV_REMOVE_CMD to cleanup the state - similar to any other
> failure.
> 
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/md/dm.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 7/8] loop: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 7/8] loop: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-06  6:19   ` Hannes Reinecke
  0 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:19 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig

On 8/30/21 11:25 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/block/loop.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 8/8] nbd: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 8/8] nbd: " Luis Chamberlain
@ 2021-09-06  6:20   ` Hannes Reinecke
  0 siblings, 0 replies; 23+ messages in thread
From: Hannes Reinecke @ 2021-09-06  6:20 UTC (permalink / raw)
  To: Luis Chamberlain, axboe, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: linux-block, bvanassche, linux-scsi, linux-mmc, linux-kernel,
	linux-nvme, ming.lei, hch, dm-devel, nbd, Christoph Hellwig

On 8/30/21 11:25 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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>   drivers/block/nbd.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel

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

* Re: [dm-devel] [PATCH v3 3/8] nvme: add error handling support for add_disk()
  2021-09-06  6:16   ` Hannes Reinecke
@ 2021-09-06  8:09     ` Christoph Hellwig
  0 siblings, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2021-09-06  8:09 UTC (permalink / raw)
  To: Hannes Reinecke
  Cc: ulf.hansson, snitzer, linux-nvme, linux-kernel, dm-devel,
	Christoph Hellwig, agk, beanhuo, ming.lei, sagi, linux-scsi, hch,
	jejb, josef, nbd, linux-block, avri.altman, kbusch, swboyd,
	bvanassche, axboe, martin.petersen, linux-mmc, adrian.hunter,
	Luis Chamberlain

On Mon, Sep 06, 2021 at 08:16:35AM +0200, Hannes Reinecke wrote:
> I would rather turn this around, and call 'nvme_put_ctrl()' after removing 
> the namespace from the list. But it's probably more a style issue, come to 
> think of it.

The order in the patch is the inverse of the order before the failure,
which generally is the right thing to do.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 3/8] nvme: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 3/8] nvme: " Luis Chamberlain
  2021-09-06  6:16   ` Hannes Reinecke
@ 2021-09-06  8:09   ` Christoph Hellwig
  1 sibling, 0 replies; 23+ messages in thread
From: Christoph Hellwig @ 2021-09-06  8:09 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: ulf.hansson, snitzer, linux-nvme, linux-kernel, dm-devel,
	Christoph Hellwig, agk, beanhuo, ming.lei, sagi, linux-scsi, hch,
	jejb, josef, nbd, linux-block, avri.altman, kbusch, swboyd,
	bvanassche, axboe, martin.petersen, linux-mmc, adrian.hunter

Thanks,

applied to nvme-5.15.

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 4/8] mmc/core/block: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 4/8] mmc/core/block: " Luis Chamberlain
@ 2021-09-06 17:10   ` Ulf Hansson
  0 siblings, 0 replies; 23+ messages in thread
From: Ulf Hansson @ 2021-09-06 17:10 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: Mike Snitzer, linux-nvme, Linux Kernel Mailing List, dm-devel,
	agk, Bean Huo (beanhuo),
	Ming Lei, sagi, linux-scsi, Christoph Hellwig,
	James E.J. Bottomley, Josef Bacik, nbd, linux-block, Avri Altman,
	kbusch, Stephen Boyd, Bart Van Assche, Jens Axboe,
	Martin K . Petersen, linux-mmc, Adrian Hunter

On Mon, 30 Aug 2021 at 23:26, Luis Chamberlain <mcgrof@kernel.org> 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.
>
> The caller only cleanups the disk if we pass on an allocated md
> but on error we return return ERR_PTR(ret), and so we must do all
> the unwinding ourselves.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Queued for v5.16 on the temporary devel branch, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/core/block.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 6a15fdf6e5f2..9b2856aa6231 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2453,9 +2453,14 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>         /* used in ->open, must be set before add_disk: */
>         if (area_type == MMC_BLK_DATA_AREA_MAIN)
>                 dev_set_drvdata(&card->dev, md);
> -       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
> +       ret = device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
> +       if (ret)
> +               goto err_cleanup_queue;
>         return md;
>
> + err_cleanup_queue:
> +       blk_cleanup_queue(md->disk->queue);
> +       blk_mq_free_tag_set(&md->queue.tag_set);
>   err_kfree:
>         kfree(md);
>   out:
> --
> 2.30.2
>

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk() Luis Chamberlain
  2021-09-06  6:13   ` Hannes Reinecke
@ 2021-09-07  1:29   ` Ming Lei
  2021-09-13 17:21     ` Luis Chamberlain
  1 sibling, 1 reply; 23+ messages in thread
From: Ming Lei @ 2021-09-07  1:29 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: ulf.hansson, snitzer, linux-nvme, linux-kernel, dm-devel,
	Christoph Hellwig, agk, beanhuo, sagi, linux-scsi, hch, jejb,
	josef, swboyd, linux-block, avri.altman, kbusch, nbd, bvanassche,
	axboe, martin.petersen, linux-mmc, adrian.hunter

On Mon, Aug 30, 2021 at 02:25:31PM -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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  drivers/scsi/sd.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 610ebba0d66e..8c1273fff23e 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3487,7 +3487,11 @@ static int sd_probe(struct device *dev)
>  		pm_runtime_set_autosuspend_delay(dev,
>  			sdp->host->hostt->rpm_autosuspend_delay);
>  	}
> -	device_add_disk(dev, gd, NULL);
> +
> +	error = device_add_disk(dev, gd, NULL);
> +	if (error)
> +		goto out_free_index;
> +

The error handling is actually wrong, see 

	https://lore.kernel.org/linux-scsi/c93f3010-13c9-e07f-1458-b6b47a27057b@acm.org/T/#t

Maybe you can base on that patch.


Thanks,
Ming

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()
  2021-08-30 21:25 ` [dm-devel] [PATCH v3 2/8] scsi/sr: " Luis Chamberlain
  2021-09-06  6:13   ` Hannes Reinecke
@ 2021-09-07  1:37   ` Ming Lei
  2021-09-13 17:26     ` Luis Chamberlain
  1 sibling, 1 reply; 23+ messages in thread
From: Ming Lei @ 2021-09-07  1:37 UTC (permalink / raw)
  To: Luis Chamberlain
  Cc: ulf.hansson, snitzer, linux-nvme, linux-kernel, dm-devel,
	Christoph Hellwig, agk, beanhuo, sagi, linux-scsi, hch, jejb,
	josef, swboyd, linux-block, avri.altman, kbusch, nbd, bvanassche,
	axboe, martin.petersen, linux-mmc, adrian.hunter

On Mon, Aug 30, 2021 at 02:25:32PM -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.
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  drivers/scsi/sr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 2942a4ec9bdd..72fd21844367 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c
> @@ -779,7 +779,10 @@ static int sr_probe(struct device *dev)
>  	dev_set_drvdata(dev, cd);
>  	disk->flags |= GENHD_FL_REMOVABLE;
>  	sr_revalidate_disk(cd);
> -	device_add_disk(&sdev->sdev_gendev, disk, NULL);
> +
> +	error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
> +	if (error)
> +		goto fail_minor;

You don't undo register_cdrom(), maybe you can use kref_put(&cd->kref, sr_kref_release);
to simplify the error handling.


Thanks,
Ming

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk()
  2021-09-07  1:29   ` Ming Lei
@ 2021-09-13 17:21     ` Luis Chamberlain
  0 siblings, 0 replies; 23+ messages in thread
From: Luis Chamberlain @ 2021-09-13 17:21 UTC (permalink / raw)
  To: Ming Lei
  Cc: ulf.hansson, snitzer, linux-nvme, linux-kernel, dm-devel,
	Christoph Hellwig, agk, beanhuo, sagi, linux-scsi, hch, jejb,
	josef, swboyd, linux-block, avri.altman, kbusch, nbd, bvanassche,
	axboe, martin.petersen, linux-mmc, adrian.hunter

On Tue, Sep 07, 2021 at 09:29:07AM +0800, Ming Lei wrote:
> On Mon, Aug 30, 2021 at 02:25:31PM -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.
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > ---
> >  drivers/scsi/sd.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> > index 610ebba0d66e..8c1273fff23e 100644
> > --- a/drivers/scsi/sd.c
> > +++ b/drivers/scsi/sd.c
> > @@ -3487,7 +3487,11 @@ static int sd_probe(struct device *dev)
> >  		pm_runtime_set_autosuspend_delay(dev,
> >  			sdp->host->hostt->rpm_autosuspend_delay);
> >  	}
> > -	device_add_disk(dev, gd, NULL);
> > +
> > +	error = device_add_disk(dev, gd, NULL);
> > +	if (error)
> > +		goto out_free_index;
> > +
> 
> The error handling is actually wrong, see 
> 
> 	https://lore.kernel.org/linux-scsi/c93f3010-13c9-e07f-1458-b6b47a27057b@acm.org/T/#t
> 
> Maybe you can base on that patch.

Done, thanks!

 Luis

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

* Re: [dm-devel] [PATCH v3 2/8] scsi/sr: add error handling support for add_disk()
  2021-09-07  1:37   ` Ming Lei
@ 2021-09-13 17:26     ` Luis Chamberlain
  0 siblings, 0 replies; 23+ messages in thread
From: Luis Chamberlain @ 2021-09-13 17:26 UTC (permalink / raw)
  To: Ming Lei
  Cc: ulf.hansson, snitzer, linux-nvme, linux-kernel, dm-devel,
	Christoph Hellwig, agk, beanhuo, sagi, linux-scsi, hch, jejb,
	josef, swboyd, linux-block, avri.altman, kbusch, nbd, bvanassche,
	axboe, martin.petersen, linux-mmc, adrian.hunter

On Tue, Sep 07, 2021 at 09:37:05AM +0800, Ming Lei wrote:
> On Mon, Aug 30, 2021 at 02:25:32PM -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.
> > 
> > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> > ---
> >  drivers/scsi/sr.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> > index 2942a4ec9bdd..72fd21844367 100644
> > --- a/drivers/scsi/sr.c
> > +++ b/drivers/scsi/sr.c
> > @@ -779,7 +779,10 @@ static int sr_probe(struct device *dev)
> >  	dev_set_drvdata(dev, cd);
> >  	disk->flags |= GENHD_FL_REMOVABLE;
> >  	sr_revalidate_disk(cd);
> > -	device_add_disk(&sdev->sdev_gendev, disk, NULL);
> > +
> > +	error = device_add_disk(&sdev->sdev_gendev, disk, NULL);
> > +	if (error)
> > +		goto fail_minor;
> 
> You don't undo register_cdrom(), maybe you can use kref_put(&cd->kref, sr_kref_release);
> to simplify the error handling.

Works with me, thanks!

  Luis

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


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

end of thread, other threads:[~2021-09-13 17:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-30 21:25 [dm-devel] [PATCH v3 0/8] block: first batch of add_disk() error handling conversions Luis Chamberlain
2021-08-30 21:25 ` [dm-devel] [PATCH v3 1/8] scsi/sd: add error handling support for add_disk() Luis Chamberlain
2021-09-06  6:13   ` Hannes Reinecke
2021-09-07  1:29   ` Ming Lei
2021-09-13 17:21     ` Luis Chamberlain
2021-08-30 21:25 ` [dm-devel] [PATCH v3 2/8] scsi/sr: " Luis Chamberlain
2021-09-06  6:13   ` Hannes Reinecke
2021-09-07  1:37   ` Ming Lei
2021-09-13 17:26     ` Luis Chamberlain
2021-08-30 21:25 ` [dm-devel] [PATCH v3 3/8] nvme: " Luis Chamberlain
2021-09-06  6:16   ` Hannes Reinecke
2021-09-06  8:09     ` Christoph Hellwig
2021-09-06  8:09   ` Christoph Hellwig
2021-08-30 21:25 ` [dm-devel] [PATCH v3 4/8] mmc/core/block: " Luis Chamberlain
2021-09-06 17:10   ` Ulf Hansson
2021-08-30 21:25 ` [dm-devel] [PATCH v3 5/8] md: " Luis Chamberlain
2021-09-06  6:17   ` Hannes Reinecke
2021-08-30 21:25 ` [dm-devel] [PATCH v3 6/8] dm: add add_disk() error handling Luis Chamberlain
2021-09-06  6:19   ` Hannes Reinecke
2021-08-30 21:25 ` [dm-devel] [PATCH v3 7/8] loop: add error handling support for add_disk() Luis Chamberlain
2021-09-06  6:19   ` Hannes Reinecke
2021-08-30 21:25 ` [dm-devel] [PATCH v3 8/8] nbd: " Luis Chamberlain
2021-09-06  6:20   ` Hannes Reinecke

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