LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4 0/6] block: first batch of add_disk() error handling conversions
@ 2021-09-27 21:59 Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 1/6] scsi/sd: add error handling support for add_disk() Luis Chamberlain
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain

This is the first set of driver conversions to use the add_disk() error
handling. You can find these changes along with the entire 7th set of
driver conversions on my 20210927-for-axboe-add-disk-error-handling
branch [0].

Changes on this v4 since the last v3:

 - Rebased onto linux-next tag 20210927
 - I drop the patches already merged
 - Fix the scsi/sd, scsi/sr as noted by Ming Lei
 - md: rebased in light of "md: fix a lock order reversal in md_alloc"
 - Adds Reviewed / Acked by tags where appropriate. I didn't keep the
   review by Hannes on the scsi patches as those patches are now
   slightly fixed

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

Luis Chamberlain (6):
  scsi/sd: add error handling support for add_disk()
  scsi/sr: 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 | 8 +++++++-
 drivers/block/nbd.c  | 6 +++++-
 drivers/md/dm.c      | 4 +++-
 drivers/md/md.c      | 6 +++++-
 drivers/scsi/sd.c    | 8 +++++++-
 drivers/scsi/sr.c    | 7 ++++++-
 6 files changed, 33 insertions(+), 6 deletions(-)

-- 
2.30.2


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

* [PATCH v4 1/6] scsi/sd: add error handling support for add_disk()
  2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
@ 2021-09-27 21:59 ` Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 2/6] scsi/sr: " Luis Chamberlain
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain, Christoph Hellwig

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

As with the error handling for device_add() we follow the same
logic and just put the device so that cleanup is done via the
scsi_disk_release().

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

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 523bf2fdc253..3a101ad4d16e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3449,7 +3449,13 @@ 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) {
+		put_device(&sdkp->dev);
+		goto out;
+	}
+
 	if (sdkp->capacity)
 		sd_dif_config_host(sdkp);
 
-- 
2.30.2


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

* [PATCH v4 2/6] scsi/sr: add error handling support for add_disk()
  2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 1/6] scsi/sd: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-27 21:59 ` Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 3/6] md: " Luis Chamberlain
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain, Christoph Hellwig

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

Just put the cdrom kref and have the unwinding be done by
sr_kref_release().

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

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 8b17b35283aa..d769057b25a0 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -727,7 +727,12 @@ 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) {
+		kref_put(&cd->kref, sr_kref_release);
+		goto fail;
+	}
 
 	sdev_printk(KERN_DEBUG, sdev,
 		    "Attached scsi CD-ROM %s\n", cd->cdi.name);
-- 
2.30.2


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

* [PATCH v4 3/6] md: add error handling support for add_disk()
  2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 1/6] scsi/sd: add error handling support for add_disk() Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 2/6] scsi/sr: " Luis Chamberlain
@ 2021-09-27 21:59 ` Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 4/6] dm: add add_disk() error handling Luis Chamberlain
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain, Christoph Hellwig, Song Liu

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>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 drivers/md/md.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 6c0c3d0d905a..6bd5ad3c30b4 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5700,7 +5700,11 @@ static int md_alloc(dev_t dev, char *name)
 	disk->flags |= GENHD_FL_EXT_DEVT;
 	disk->events |= DISK_EVENT_MEDIA_CHANGE;
 	mddev->gendisk = disk;
-	add_disk(disk);
+	error = add_disk(disk);
+	if (error) {
+		blk_cleanup_disk(disk);
+		goto abort;
+	}
 
 	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
 	if (error) {
-- 
2.30.2


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

* [PATCH v4 4/6] dm: add add_disk() error handling
  2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (2 preceding siblings ...)
  2021-09-27 21:59 ` [PATCH v4 3/6] md: " Luis Chamberlain
@ 2021-09-27 21:59 ` Luis Chamberlain
  2021-10-06 13:17   ` Mike Snitzer
  2021-09-27 21:59 ` [PATCH v4 5/6] loop: add error handling support for add_disk() Luis Chamberlain
  2021-09-27 21:59 ` [PATCH v4 6/6] nbd: " Luis Chamberlain
  5 siblings, 1 reply; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain

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

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.

Reviewed-by: Hannes Reinecke <hare@suse.de>
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 a011d09cb0fa..b83aab8507c2 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -2083,7 +2083,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


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

* [PATCH v4 5/6] loop: add error handling support for add_disk()
  2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (3 preceding siblings ...)
  2021-09-27 21:59 ` [PATCH v4 4/6] dm: add add_disk() error handling Luis Chamberlain
@ 2021-09-27 21:59 ` Luis Chamberlain
  2021-09-27 22:28   ` Jens Axboe
  2021-09-27 21:59 ` [PATCH v4 6/6] nbd: " Luis Chamberlain
  5 siblings, 1 reply; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain, Christoph Hellwig

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

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 7bf4686af774..00ee365ed5e0 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2394,13 +2394,19 @@ static int loop_add(int i)
 	disk->event_flags	= DISK_EVENT_FLAG_UEVENT;
 	sprintf(disk->disk_name, "loop%d", i);
 	/* Make this loop device reachable from pathname. */
-	add_disk(disk);
+	err = add_disk(disk);
+	if (err)
+		goto out_cleanup_disk;
+
 	/* Show this loop device. */
 	mutex_lock(&loop_ctl_mutex);
 	lo->idr_visible = true;
 	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


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

* [PATCH v4 6/6] nbd: add error handling support for add_disk()
  2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
                   ` (4 preceding siblings ...)
  2021-09-27 21:59 ` [PATCH v4 5/6] loop: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-27 21:59 ` Luis Chamberlain
  2021-09-27 22:28   ` Jens Axboe
  5 siblings, 1 reply; 10+ messages in thread
From: Luis Chamberlain @ 2021-09-27 21:59 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, kbusch, sagi, adrian.hunter,
	beanhuo, ulf.hansson, avri.altman, swboyd, agk, snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Luis Chamberlain, Christoph Hellwig

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>
Reviewed-by: Hannes Reinecke <hare@suse.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


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

* Re: [PATCH v4 5/6] loop: add error handling support for add_disk()
  2021-09-27 21:59 ` [PATCH v4 5/6] loop: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-27 22:28   ` Jens Axboe
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2021-09-27 22:28 UTC (permalink / raw)
  To: Luis Chamberlain, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Christoph Hellwig

On 9/27/21 3:59 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.

Applied, thanks.

-- 
Jens Axboe


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

* Re: [PATCH v4 6/6] nbd: add error handling support for add_disk()
  2021-09-27 21:59 ` [PATCH v4 6/6] nbd: " Luis Chamberlain
@ 2021-09-27 22:28   ` Jens Axboe
  0 siblings, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2021-09-27 22:28 UTC (permalink / raw)
  To: Luis Chamberlain, martin.petersen, jejb, kbusch, sagi,
	adrian.hunter, beanhuo, ulf.hansson, avri.altman, swboyd, agk,
	snitzer, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel,
	Christoph Hellwig

On 9/27/21 3:59 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.

Applied, thanks.

-- 
Jens Axboe


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

* Re: [PATCH v4 4/6] dm: add add_disk() error handling
  2021-09-27 21:59 ` [PATCH v4 4/6] dm: add add_disk() error handling Luis Chamberlain
@ 2021-10-06 13:17   ` Mike Snitzer
  0 siblings, 0 replies; 10+ messages in thread
From: Mike Snitzer @ 2021-10-06 13:17 UTC (permalink / raw)
  To: axboe, martin.petersen, jejb, sagi, adrian.hunter, beanhuo,
	ulf.hansson, avri.altman, swboyd, agk, josef
  Cc: hch, hare, bvanassche, ming.lei, linux-scsi, linux-nvme,
	linux-mmc, dm-devel, nbd, linux-block, linux-kernel

On Mon, Sep 27 2021 at  5:59P -0400,
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.
> 
> 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.
> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>

Reviewed-by: Mike Snitzer <snitzer@redhat.com>


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

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-27 21:59 [PATCH v4 0/6] block: first batch of add_disk() error handling conversions Luis Chamberlain
2021-09-27 21:59 ` [PATCH v4 1/6] scsi/sd: add error handling support for add_disk() Luis Chamberlain
2021-09-27 21:59 ` [PATCH v4 2/6] scsi/sr: " Luis Chamberlain
2021-09-27 21:59 ` [PATCH v4 3/6] md: " Luis Chamberlain
2021-09-27 21:59 ` [PATCH v4 4/6] dm: add add_disk() error handling Luis Chamberlain
2021-10-06 13:17   ` Mike Snitzer
2021-09-27 21:59 ` [PATCH v4 5/6] loop: add error handling support for add_disk() Luis Chamberlain
2021-09-27 22:28   ` Jens Axboe
2021-09-27 21:59 ` [PATCH v4 6/6] nbd: " Luis Chamberlain
2021-09-27 22:28   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).