linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev()
@ 2021-07-15 20:59 Luis Chamberlain
  2021-07-15 20:59 ` [RFC 01/12] floppy: fix add_disk() assumption on exit Luis Chamberlain
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

This is the 4th group of driver conversion examples. This set is
complete, as there are only a few drivers which use __register_blkdev().
The __register_blkdev() call uses an optional probe call, and if set
will be used when blk_request_module() is called for the block device.
Since these probe calls can fail, now that we added *add_disk*() error
handling, take advatage of this and extend the probe call to capture
the errors.

There are only a few drivers which benefit from this. The meat of the
work is the last patch, which includes all driver conversions. The rest
of the patches are adding add_disk() error handling in the other
places of the drivers.

Luis Chamberlain (12):
  floppy: fix add_disk() assumption on exit
  floppy: use blk_cleanup_disk()
  floppy: add error handling support for add_disk()
  scsi/sd: use blk_cleanup_queue() insted of put_disk()
  scsi/sd: add error handling support for add_disk()
  scsi/sr: use blk_cleanup_disk() instead of put_disk()
  scsi/sr: add error handling support for add_disk()
  block/ataflop: use the blk_cleanup_disk() helper
  block/ataflop: add a helper for removing disks
  block/ataflop add error handling support for add_disk()
  block/brd: add error handling support for add_disk()
  block: make probe in blk_request_module() return an error

 block/genhd.c           | 15 ++++++----
 drivers/block/ataflop.c | 66 ++++++++++++++++++++++++-----------------
 drivers/block/brd.c     | 17 ++++++++---
 drivers/block/floppy.c  | 48 ++++++++++++++----------------
 drivers/block/loop.c    |  6 ++--
 drivers/scsi/sd.c       | 11 +++++--
 drivers/scsi/sr.c       |  7 +++--
 fs/block_dev.c          |  5 +++-
 include/linux/genhd.h   |  4 +--
 9 files changed, 105 insertions(+), 74 deletions(-)

-- 
2.27.0


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

* [RFC 01/12] floppy: fix add_disk() assumption on exit
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 02/12] floppy: use blk_cleanup_disk() Luis Chamberlain
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

Even if the queue was cleaned up on exit,
putting the disk *is* still required, and
likewise, blk_cleanup_queue() on a null queue
should be a no-op now, after the patch titled
"block: skip queue if NULL on blk_cleanup_queue()"
so it is safe to use even if the queue is NULL.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/floppy.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 87460e0e5c72..b68f4b6bf737 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4953,19 +4953,6 @@ static void __exit floppy_module_exit(void)
 				blk_cleanup_queue(disks[drive][i]->queue);
 		}
 		blk_mq_free_tag_set(&tag_sets[drive]);
-
-		/*
-		 * These disks have not called add_disk().  Don't put down
-		 * queue reference in put_disk().
-		 */
-		if (!(allowed_drive_mask & (1 << drive)) ||
-		    fdc_state[FDC(drive)].version == FDC_NONE) {
-			for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
-				if (disks[drive][i])
-					disks[drive][i]->queue = NULL;
-			}
-		}
-
 		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
 			if (disks[drive][i])
 				put_disk(disks[drive][i]);
-- 
2.27.0


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

* [RFC 02/12] floppy: use blk_cleanup_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
  2021-07-15 20:59 ` [RFC 01/12] floppy: fix add_disk() assumption on exit Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 03/12] floppy: add error handling support for add_disk() Luis Chamberlain
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

Use the blk_cleanup_queue() followed by put_disk() can be
replaced with blk_cleanup_disk(). No need for two separate
loops.

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

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index b68f4b6bf737..ba690affc751 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4950,13 +4950,9 @@ static void __exit floppy_module_exit(void)
 		}
 		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
 			if (disks[drive][i])
-				blk_cleanup_queue(disks[drive][i]->queue);
+				blk_cleanup_disk(disks[drive][i]);
 		}
 		blk_mq_free_tag_set(&tag_sets[drive]);
-		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
-			if (disks[drive][i])
-				put_disk(disks[drive][i]);
-		}
 	}
 
 	cancel_delayed_work_sync(&fd_timeout);
-- 
2.27.0


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

* [RFC 03/12] floppy: add error handling support for add_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
  2021-07-15 20:59 ` [RFC 01/12] floppy: fix add_disk() assumption on exit Luis Chamberlain
  2021-07-15 20:59 ` [RFC 02/12] floppy: use blk_cleanup_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 04/12] scsi/sd: use blk_cleanup_queue() insted of put_disk() Luis Chamberlain
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, 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.

We take advantage of the new ability of del_gendisk() ...

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

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index ba690affc751..cad17b49e700 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4478,6 +4478,7 @@ static const struct blk_mq_ops floppy_mq_ops = {
 };
 
 static struct platform_device floppy_device[N_DRIVE];
+static bool registered[N_DRIVE];
 
 static bool floppy_available(int drive)
 {
@@ -4693,8 +4694,12 @@ static int __init do_floppy_init(void)
 		if (err)
 			goto out_remove_drives;
 
-		device_add_disk(&floppy_device[drive].dev, disks[drive][0],
-				NULL);
+		registered[drive] = true;
+
+		err = device_add_disk(&floppy_device[drive].dev,
+				      disks[drive][0], NULL);
+		if (err)
+			goto out_remove_drives;
 	}
 
 	return 0;
@@ -4703,7 +4708,8 @@ static int __init do_floppy_init(void)
 	while (drive--) {
 		if (floppy_available(drive)) {
 			del_gendisk(disks[drive][0]);
-			platform_device_unregister(&floppy_device[drive]);
+			if (registered[drive])
+				platform_device_unregister(&floppy_device[drive]);
 		}
 	}
 out_release_dma:
@@ -4946,7 +4952,8 @@ static void __exit floppy_module_exit(void)
 				if (disks[drive][i])
 					del_gendisk(disks[drive][i]);
 			}
-			platform_device_unregister(&floppy_device[drive]);
+			if (registered[drive])
+				platform_device_unregister(&floppy_device[drive]);
 		}
 		for (i = 0; i < ARRAY_SIZE(floppy_type); i++) {
 			if (disks[drive][i])
-- 
2.27.0


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

* [RFC 04/12] scsi/sd: use blk_cleanup_queue() insted of put_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (2 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 03/12] floppy: add error handling support for add_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 05/12] scsi/sd: add error handling support for add_disk() Luis Chamberlain
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

The single put_disk() is useful if you know you're not doing
a cleanup after add_disk(), but since we want to add support
for that, just use the normal form of blk_cleanup_disk() to
cleanup the queue and put the disk.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/scsi/sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6d2d63629a90..6d0a82da7131 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3510,7 +3510,7 @@ static int sd_probe(struct device *dev)
  out_free_index:
 	ida_free(&sd_index_ida, index);
  out_put:
-	put_disk(gd);
+	blk_cleanup_disk(gd);
  out_free:
 	sd_zbc_release_disk(sdkp);
 	kfree(sdkp);
-- 
2.27.0


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

* [RFC 05/12] scsi/sd: add error handling support for add_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (3 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 04/12] scsi/sd: use blk_cleanup_queue() insted of put_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 06/12] scsi/sr: use blk_cleanup_disk() instead of put_disk() Luis Chamberlain
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

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

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

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 6d0a82da7131..01af61a38e4e 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3489,7 +3489,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.27.0


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

* [RFC 06/12] scsi/sr: use blk_cleanup_disk() instead of put_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (4 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 05/12] scsi/sd: add error handling support for add_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 07/12] scsi/sr: add error handling support for add_disk() Luis Chamberlain
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

The single put_disk() is useful if you know you're not doing
a cleanup after add_disk(), but since we want to add support
for that, just use the normal form of blk_cleanup_disk() to
cleanup the queue and put the disk.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/scsi/sr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 94c254e9012e..362f04a3761a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -789,7 +789,7 @@ static int sr_probe(struct device *dev)
 	clear_bit(minor, sr_index_bits);
 	spin_unlock(&sr_index_lock);
 fail_put:
-	put_disk(disk);
+	blk_cleanup_disk(disk);
 	mutex_destroy(&cd->lock);
 fail_free:
 	kfree(cd);
-- 
2.27.0


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

* [RFC 07/12] scsi/sr: add error handling support for add_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (5 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 06/12] scsi/sr: use blk_cleanup_disk() instead of put_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 08/12] block/ataflop: use the blk_cleanup_disk() helper Luis Chamberlain
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

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

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

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 362f04a3761a..ed097b69821e 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -776,7 +776,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.27.0


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

* [RFC 08/12] block/ataflop: use the blk_cleanup_disk() helper
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (6 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 07/12] scsi/sr: add error handling support for add_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 09/12] block/ataflop: add a helper for removing disks Luis Chamberlain
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

Use the helper to replace two lines with one.

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

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index a093644ac39f..abb6fde0bd81 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2076,8 +2076,7 @@ static int __init atari_floppy_init (void)
 
 err:
 	while (--i >= 0) {
-		blk_cleanup_queue(unit[i].disk[0]->queue);
-		put_disk(unit[i].disk[0]);
+		blk_cleanup_disk(unit[i].disk[0]);
 		blk_mq_free_tag_set(&unit[i].tag_set);
 	}
 
@@ -2135,8 +2134,7 @@ static void __exit atari_floppy_exit(void)
 			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_cleanup_disk(unit[i].disk[0]);
 		}
 		blk_mq_free_tag_set(&unit[i].tag_set);
 	}
-- 
2.27.0


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

* [RFC 09/12] block/ataflop: add a helper for removing disks
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (7 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 08/12] block/ataflop: use the blk_cleanup_disk() helper Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 10/12] block/ataflop add error handling support for add_disk() Luis Chamberlain
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

The del_gendisk() can be used now safely even if add_disk()
did not complete, and so using a helper allows other code to
use this later.

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 drivers/block/ataflop.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index abb6fde0bd81..73eb80b58888 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -1988,6 +1988,23 @@ static int ataflop_alloc_disk(unsigned int drive, unsigned int type)
 
 static DEFINE_MUTEX(ataflop_probe_lock);
 
+static void atari_floppy_disk_remove(void)
+{
+	int i, 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_disk(unit[i].disk[0]);
+		}
+		blk_mq_free_tag_set(&unit[i].tag_set);
+	}
+	unregister_blkdev(FLOPPY_MAJOR, "fd");
+
+}
+
 static void ataflop_probe(dev_t dev)
 {
 	int drive = MINOR(dev) & 3;
@@ -2127,19 +2144,7 @@ __setup("floppy=", atari_floppy_setup);
 
 static void __exit atari_floppy_exit(void)
 {
-	int i, 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_disk(unit[i].disk[0]);
-		}
-		blk_mq_free_tag_set(&unit[i].tag_set);
-	}
-	unregister_blkdev(FLOPPY_MAJOR, "fd");
-
+	atari_floppy_disk_remove();
 	del_timer_sync(&fd_timer);
 	atari_stram_free( DMABuffer );
 }
-- 
2.27.0


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

* [RFC 10/12] block/ataflop add error handling support for add_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (8 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 09/12] block/ataflop: add a helper for removing disks Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 11/12] block/brd: " Luis Chamberlain
  2021-07-15 20:59 ` [RFC 12/12] block: make probe in blk_request_module() return an error Luis Chamberlain
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, 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.

We reuse the atari_floppy_disk_remove() call.

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

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 73eb80b58888..4b3f1158fa04 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2081,7 +2081,9 @@ static int __init atari_floppy_init (void)
 	for (i = 0; i < FD_MAX_UNITS; i++) {
 		unit[i].track = -1;
 		unit[i].flags = 0;
-		add_disk(unit[i].disk[0]);
+		ret = add_disk(unit[i].disk[0]);
+		if (ret)
+			goto err_out_dma;
 	}
 
 	printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n",
@@ -2091,13 +2093,10 @@ static int __init atari_floppy_init (void)
 
 	return 0;
 
+err_out_dma:
+	atari_stram_free(DMABuffer);
 err:
-	while (--i >= 0) {
-		blk_cleanup_disk(unit[i].disk[0]);
-		blk_mq_free_tag_set(&unit[i].tag_set);
-	}
-
-	unregister_blkdev(FLOPPY_MAJOR, "fd");
+	atari_floppy_disk_remove();
 out_unlock:
 	mutex_unlock(&ataflop_probe_lock);
 	return ret;
-- 
2.27.0


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

* [RFC 11/12] block/brd: add error handling support for add_disk()
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (9 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 10/12] block/ataflop add error handling support for add_disk() Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  2021-07-15 20:59 ` [RFC 12/12] block: make probe in blk_request_module() return an error Luis Chamberlain
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

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

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

diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 95694113e38e..ca017ca315c5 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -375,6 +375,7 @@ static int brd_alloc(int i)
 	struct brd_device *brd;
 	struct gendisk *disk;
 	char buf[DISK_NAME_LEN];
+	int err = -ENOMEM;
 
 	brd = kzalloc(sizeof(*brd), GFP_KERNEL);
 	if (!brd)
@@ -413,14 +414,19 @@ static int brd_alloc(int i)
 	/* Tell the block layer that this is not a rotational device */
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
 	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, disk->queue);
-	add_disk(disk);
+	err = add_disk(disk);
+	if (err)
+		goto out_cleanup_disk;
+
 	list_add_tail(&brd->brd_list, &brd_devices);
 
 	return 0;
 
+out_cleanup_disk:
+	blk_cleanup_disk(disk);
 out_free_dev:
 	kfree(brd);
-	return -ENOMEM;
+	return err;
 }
 
 static void brd_probe(dev_t dev)
-- 
2.27.0


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

* [RFC 12/12] block: make probe in blk_request_module() return an error
  2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
                   ` (10 preceding siblings ...)
  2021-07-15 20:59 ` [RFC 11/12] block/brd: " Luis Chamberlain
@ 2021-07-15 20:59 ` Luis Chamberlain
  11 siblings, 0 replies; 13+ messages in thread
From: Luis Chamberlain @ 2021-07-15 20:59 UTC (permalink / raw)
  To: axboe
  Cc: hare, bvanassche, ming.lei, hch, jack, osandov, linux-block,
	linux-kernel, Luis Chamberlain

This makes the probe callback use din blk_request_module() return
an error. We do this as add_disk() now has error handling, and
so we can bail earlier than before. If a probe is not implemented
then its not used.

This is mostly useful for the following drivers:

  * ataflop
  * brd
  * floppy
  * loop
  * scsi/sd

Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/genhd.c           | 15 +++++++++------
 drivers/block/ataflop.c | 20 +++++++++++++++-----
 drivers/block/brd.c     |  7 +++++--
 drivers/block/floppy.c  | 14 ++++++++++----
 drivers/block/loop.c    |  6 +++---
 drivers/scsi/sd.c       |  3 ++-
 fs/block_dev.c          |  5 ++++-
 include/linux/genhd.h   |  4 ++--
 8 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 72703d243b44..ca6393df09ad 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -161,7 +161,7 @@ static struct blk_major_name {
 	struct blk_major_name *next;
 	int major;
 	char name[16];
-	void (*probe)(dev_t devt);
+	int (*probe)(dev_t devt);
 } *major_names[BLKDEV_MAJOR_HASH_SIZE];
 static DEFINE_MUTEX(major_names_lock);
 
@@ -190,7 +190,7 @@ 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: callback that is called on access to any minor number of @major
  *
  * The @name must be unique within the system.
  *
@@ -208,7 +208,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
  * Use register_blkdev instead for any new code.
  */
 int __register_blkdev(unsigned int major, const char *name,
-		void (*probe)(dev_t devt))
+		int (*probe)(dev_t devt))
 {
 	struct blk_major_name **n, *p;
 	int index, ret = 0;
@@ -728,17 +728,18 @@ static ssize_t disk_badblocks_store(struct device *dev,
 	return badblocks_store(disk->bb, page, len, 0);
 }
 
-void blk_request_module(dev_t devt)
+int blk_request_module(dev_t devt)
 {
 	unsigned int major = MAJOR(devt);
 	struct blk_major_name **n;
+	int err;
 
 	mutex_lock(&major_names_lock);
 	for (n = &major_names[major_to_index(major)]; *n; n = &(*n)->next) {
 		if ((*n)->major == major && (*n)->probe) {
-			(*n)->probe(devt);
+			err = (*n)->probe(devt);
 			mutex_unlock(&major_names_lock);
-			return;
+			return err;
 		}
 	}
 	mutex_unlock(&major_names_lock);
@@ -746,6 +747,8 @@ void blk_request_module(dev_t devt)
 	if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
 		/* Make old-style 2.4 aliases work */
 		request_module("block-major-%d", MAJOR(devt));
+
+	return 0;
 }
 
 /*
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 4b3f1158fa04..ce08b8e60163 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2005,22 +2005,32 @@ static void atari_floppy_disk_remove(void)
 
 }
 
-static void ataflop_probe(dev_t dev)
+static int ataflop_probe(dev_t dev)
 {
 	int drive = MINOR(dev) & 3;
 	int type  = MINOR(dev) >> 2;
+	int err = -ENODEV;
 
 	if (type)
 		type--;
 
-	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
-		return;
+	if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) {
+		err = -EINVAL;
+		goto out;
+	}
+
 	mutex_lock(&ataflop_probe_lock);
 	if (!unit[drive].disk[type]) {
-		if (ataflop_alloc_disk(drive, type) == 0)
-			add_disk(unit[drive].disk[type]);
+		if (ataflop_alloc_disk(drive, type) == 0) {
+			err = add_disk(unit[drive].disk[type]);
+			if (err)
+				blk_cleanup_disk(unit[drive].disk[type]);
+		}
 	}
 	mutex_unlock(&ataflop_probe_lock);
+
+out:
+	return err;
 }
 
 static int __init atari_floppy_init (void)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index ca017ca315c5..104adf70a15a 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -429,10 +429,11 @@ static int brd_alloc(int i)
 	return err;
 }
 
-static void brd_probe(dev_t dev)
+static int brd_probe(dev_t dev)
 {
 	int i = MINOR(dev) / max_part;
 	struct brd_device *brd;
+	int err = 0;
 
 	mutex_lock(&brd_devices_mutex);
 	list_for_each_entry(brd, &brd_devices, brd_list) {
@@ -440,9 +441,11 @@ static void brd_probe(dev_t dev)
 			goto out_unlock;
 	}
 
-	brd_alloc(i);
+	err = brd_alloc(i);
 out_unlock:
 	mutex_unlock(&brd_devices_mutex);
+
+	return err;
 }
 
 static void brd_del_one(struct brd_device *brd)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index cad17b49e700..52bc645f3060 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4517,21 +4517,27 @@ static int floppy_alloc_disk(unsigned int drive, unsigned int type)
 
 static DEFINE_MUTEX(floppy_probe_lock);
 
-static void floppy_probe(dev_t dev)
+static int floppy_probe(dev_t dev)
 {
 	unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
 	unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+	int err;
 
 	if (drive >= N_DRIVE || !floppy_available(drive) ||
 	    type >= ARRAY_SIZE(floppy_type))
-		return;
+		return -EINVAL;
 
 	mutex_lock(&floppy_probe_lock);
 	if (!disks[drive][type]) {
-		if (floppy_alloc_disk(drive, type) == 0)
-			add_disk(disks[drive][type]);
+		if (floppy_alloc_disk(drive, type) == 0) {
+			err = add_disk(disks[drive][type]);
+			if (err)
+				blk_cleanup_disk(disks[drive][type]);
+		}
 	}
 	mutex_unlock(&floppy_probe_lock);
+
+	return err;
 }
 
 static int __init do_floppy_init(void)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index efbd8e29aca7..081cccd39bc9 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2358,13 +2358,13 @@ static void loop_remove(struct loop_device *lo)
 	kfree(lo);
 }
 
-static void loop_probe(dev_t dev)
+static int loop_probe(dev_t dev)
 {
 	int idx = MINOR(dev) >> part_shift;
 
 	if (max_loop && idx >= max_loop)
-		return;
-	loop_add(idx);
+		return -EINVAL;
+	return loop_add(idx);
 }
 
 static int loop_control_remove(int idx)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 01af61a38e4e..800dfd5984bd 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -633,8 +633,9 @@ static struct scsi_driver sd_template = {
  * Don't request a new module, as that could deadlock in multipath
  * environment.
  */
-static void sd_default_probe(dev_t devt)
+static int sd_default_probe(dev_t devt)
 {
+	return 0;
 }
 
 /*
diff --git a/fs/block_dev.c b/fs/block_dev.c
index c41d0e550d39..531feb097cb2 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1326,10 +1326,13 @@ struct block_device *blkdev_get_no_open(dev_t dev)
 {
 	struct block_device *bdev;
 	struct gendisk *disk;
+	int err;
 
 	bdev = bdget(dev);
 	if (!bdev) {
-		blk_request_module(dev);
+		err = blk_request_module(dev);
+		if (err)
+			return NULL;
 		bdev = bdget(dev);
 		if (!bdev)
 			return NULL;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 73024416d2d5..9db56d4562ae 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -312,7 +312,7 @@ struct gendisk *__blk_alloc_disk(int node);
 void blk_cleanup_disk(struct gendisk *disk);
 
 int __register_blkdev(unsigned int major, const char *name,
-		void (*probe)(dev_t devt));
+		int (*probe)(dev_t devt));
 #define register_blkdev(major, name) \
 	__register_blkdev(major, name, NULL)
 void unregister_blkdev(unsigned int major, const char *name);
@@ -342,7 +342,7 @@ static inline void bd_unlink_disk_holder(struct block_device *bdev,
 
 dev_t part_devt(struct gendisk *disk, u8 partno);
 dev_t blk_lookup_devt(const char *name, int partno);
-void blk_request_module(dev_t devt);
+int blk_request_module(dev_t devt);
 #ifdef CONFIG_BLOCK
 void printk_all_partitions(void);
 #else /* CONFIG_BLOCK */
-- 
2.27.0


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

end of thread, other threads:[~2021-07-15 21:00 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15 20:59 [RFC 00/12] block: *add_disk*() driver conversions __register_blkdev() Luis Chamberlain
2021-07-15 20:59 ` [RFC 01/12] floppy: fix add_disk() assumption on exit Luis Chamberlain
2021-07-15 20:59 ` [RFC 02/12] floppy: use blk_cleanup_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 03/12] floppy: add error handling support for add_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 04/12] scsi/sd: use blk_cleanup_queue() insted of put_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 05/12] scsi/sd: add error handling support for add_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 06/12] scsi/sr: use blk_cleanup_disk() instead of put_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 07/12] scsi/sr: add error handling support for add_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 08/12] block/ataflop: use the blk_cleanup_disk() helper Luis Chamberlain
2021-07-15 20:59 ` [RFC 09/12] block/ataflop: add a helper for removing disks Luis Chamberlain
2021-07-15 20:59 ` [RFC 10/12] block/ataflop add error handling support for add_disk() Luis Chamberlain
2021-07-15 20:59 ` [RFC 11/12] block/brd: " Luis Chamberlain
2021-07-15 20:59 ` [RFC 12/12] block: make probe in blk_request_module() return an error 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).