linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/14] block: 6th batch of add_disk() error handling conversions
@ 2021-09-04  1:35 Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 01/14] block/swim3: add error handling support for add_disk() Luis Chamberlain
                   ` (13 more replies)
  0 siblings, 14 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

This is the 6th of 7 batch of driver conversions over to use the
new add_disk() error handling. This series addresses the wonderful
and extremely exciting world of floppy drivers. You can find the full
set of my patches on my 20210901-for-axboe-add-disk-error-handling
branch [0]. This is based on axboe/master.

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

Luis Chamberlain (14):
  block/swim3: add error handling support for add_disk()
  floppy: fix add_disk() assumption on exit due to new developments
  floppy: use blk_cleanup_disk()
  floppy: fix calling platform_device_unregister() on invalid drives
  floppy: add error handling support for add_disk()
  amiflop: add error handling support for add_disk()
  swim: simplify using blk_cleanup_disk() on swim_remove()
  swim: add helper for disk cleanup
  swim: add a floppy registration bool which triggers del_gendisk()
  swim: add error handling support for add_disk()
  block/ataflop: use the blk_cleanup_disk() helper
  block/ataflop: add registration bool before calling del_gendisk()
  block/ataflop: provide a helper for cleanup up an atari disk
  block/ataflop add error handling support for add_disk()

 drivers/block/amiflop.c |  7 ++++--
 drivers/block/ataflop.c | 47 +++++++++++++++++++++++++----------------
 drivers/block/floppy.c  | 34 +++++++++++------------------
 drivers/block/swim.c    | 35 ++++++++++++++++++------------
 drivers/block/swim3.c   |  4 +++-
 5 files changed, 71 insertions(+), 56 deletions(-)

-- 
2.30.2


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

* [PATCH 01/14] block/swim3: add error handling support for add_disk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 02/14] floppy: fix add_disk() assumption on exit due to new developments Luis Chamberlain
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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/swim3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/block/swim3.c b/drivers/block/swim3.c
index 965af0a3e95b..f7e3482e846b 100644
--- a/drivers/block/swim3.c
+++ b/drivers/block/swim3.c
@@ -1229,7 +1229,9 @@ static int swim3_attach(struct macio_dev *mdev,
 	disk->flags |= GENHD_FL_REMOVABLE;
 	sprintf(disk->disk_name, "fd%d", floppy_count);
 	set_capacity(disk, 2880);
-	add_disk(disk);
+	rc = add_disk(disk);
+	if (rc)
+		goto out_cleanup_disk;
 
 	disks[floppy_count++] = disk;
 	return 0;
-- 
2.30.2


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

* [PATCH 02/14] floppy: fix add_disk() assumption on exit due to new developments
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 01/14] block/swim3: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 03/14] floppy: use blk_cleanup_disk() Luis Chamberlain
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

After the patch titled "floppy: use blk_mq_alloc_disk and
blk_cleanup_disk" the floppy driver was modified to allocate
the blk_mq_alloc_disk() which allocates the disk with the
queue. This is further clarified later with the patch titled
"block: remove alloc_disk and alloc_disk_node". This clarifies
that:

   Most drivers should use and have been converted to use
   blk_alloc_disk and blk_mq_alloc_disk.  Only the scsi
   ULPs and dasd still allocate a disk separately from the
   request_queue so don't bother with convenience macros for
   something that should not see significant new users and
   remove these wrappers.

And then we have the patch titled, "block: hold a request_queue
reference for the lifetime of struct gendisk" which ensures
that a queue is *always* present for sure during the entire
lifetime of a disk.

In the floppy driver's case then the disk always comes with the
queue. So even if 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 not happen now as disk->queue is valid from
disk allocation time on.

Automatic backport code scrapers should hopefully not cherry pick
this patch as a stable fix candidate without full due dilligence to
ensure all the work done on the block layer to make this happen is
merged first.

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 fef79ea52e3e..3e6390fd5f2b 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.30.2


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

* [PATCH 03/14] floppy: use blk_cleanup_disk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 01/14] block/swim3: add error handling support for add_disk() Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 02/14] floppy: fix add_disk() assumption on exit due to new developments Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 04/14] floppy: fix calling platform_device_unregister() on invalid drives Luis Chamberlain
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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 3e6390fd5f2b..9538146e520e 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.30.2


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

* [PATCH 04/14] floppy: fix calling platform_device_unregister() on invalid drives
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (2 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 03/14] floppy: use blk_cleanup_disk() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 05/14] floppy: add error handling support for add_disk() Luis Chamberlain
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

platform_device_unregister() should only be called when
a respective platform_device_register() is called. However
the floppy driver currently allows failures when registring
a drive and a bail out could easily cause an invalid call
to platform_device_unregister() where it was not intended.

Fix this by adding a bool to keep track of when the platform
device was registered for a drive.

This does not fix any known panic / bug. This issue was found
through code inspection while preparing the driver to use the
up and coming support for device_add_disk() error handling.
From what I can tell from code inspection, chances of this
ever happening should be insanely small, perhaps OOM.

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

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 9538146e520e..3592a6277d0b 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,6 +4694,8 @@ static int __init do_floppy_init(void)
 		if (err)
 			goto out_remove_drives;
 
+		registered[drive] = true;
+
 		device_add_disk(&floppy_device[drive].dev, disks[drive][0],
 				NULL);
 	}
@@ -4703,7 +4706,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 +4950,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.30.2


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

* [PATCH 05/14] floppy: add error handling support for add_disk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (3 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 04/14] floppy: fix calling platform_device_unregister() on invalid drives Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 06/14] amiflop: " Luis Chamberlain
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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/floppy.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 3592a6277d0b..0434f28742e7 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4696,8 +4696,10 @@ static int __init do_floppy_init(void)
 
 		registered[drive] = true;
 
-		device_add_disk(&floppy_device[drive].dev, disks[drive][0],
-				NULL);
+		err = device_add_disk(&floppy_device[drive].dev,
+				      disks[drive][0], NULL);
+		if (err)
+			goto out_remove_drives;
 	}
 
 	return 0;
-- 
2.30.2


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

* [PATCH 06/14] amiflop: add error handling support for add_disk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (4 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 05/14] floppy: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 07/14] swim: simplify using blk_cleanup_disk() on swim_remove() Luis Chamberlain
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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. The caller for fd_alloc_disk() deals with
the rest of the cleanup like the tag.

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

diff --git a/drivers/block/amiflop.c b/drivers/block/amiflop.c
index 8b1714021498..7da6a7532cde 100644
--- a/drivers/block/amiflop.c
+++ b/drivers/block/amiflop.c
@@ -1780,6 +1780,7 @@ static const struct blk_mq_ops amiflop_mq_ops = {
 static int fd_alloc_disk(int drive, int system)
 {
 	struct gendisk *disk;
+	int err;
 
 	disk = blk_mq_alloc_disk(&unit[drive].tag_set, NULL);
 	if (IS_ERR(disk))
@@ -1798,8 +1799,10 @@ static int fd_alloc_disk(int drive, int system)
 	set_capacity(disk, 880 * 2);
 
 	unit[drive].gendisk[system] = disk;
-	add_disk(disk);
-	return 0;
+	err = add_disk(disk);
+	if (err)
+		blk_cleanup_disk(disk);
+	return err;
 }
 
 static int fd_alloc_drive(int drive)
-- 
2.30.2


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

* [PATCH 07/14] swim: simplify using blk_cleanup_disk() on swim_remove()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (5 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 06/14] amiflop: " Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 08/14] swim: add helper for disk cleanup Luis Chamberlain
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

We can simplify swim_remove() by using one call instead of two,
just as other drivers do. Use that pattern.

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

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index 7ccc8d2a41bc..f085ea982bb7 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -911,9 +911,8 @@ static int swim_remove(struct platform_device *dev)
 
 	for (drive = 0; drive < swd->floppy_count; drive++) {
 		del_gendisk(swd->unit[drive].disk);
-		blk_cleanup_queue(swd->unit[drive].disk->queue);
+		blk_cleanup_disk(swd->unit[drive].disk);
 		blk_mq_free_tag_set(&swd->unit[drive].tag_set);
-		put_disk(swd->unit[drive].disk);
 	}
 
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
-- 
2.30.2


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

* [PATCH 08/14] swim: add helper for disk cleanup
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (6 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 07/14] swim: simplify using blk_cleanup_disk() on swim_remove() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 09/14] swim: add a floppy registration bool which triggers del_gendisk() Luis Chamberlain
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

Disk cleanup can be shared between exit and bringup. Use a
helper to do the work required. The only functional change at
this point is we're being overly paraoid on exit to check for
a null disk as well now, and this should be safe.

We'll later expand on this, this change just makes subsequent
changes easier to read.

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

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index f085ea982bb7..3deb9d9a59c0 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -771,6 +771,17 @@ static const struct blk_mq_ops swim_mq_ops = {
 	.queue_rq = swim_queue_rq,
 };
 
+static void swim_cleanup_floppy_disk(struct floppy_state *fs)
+{
+	struct gendisk *disk = fs->disk;
+
+	if (!disk)
+		return;
+
+	blk_cleanup_disk(disk);
+	blk_mq_free_tag_set(&fs->tag_set);
+}
+
 static int swim_floppy_init(struct swim_priv *swd)
 {
 	int err;
@@ -835,12 +846,7 @@ static int swim_floppy_init(struct swim_priv *swd)
 exit_put_disks:
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 	do {
-		struct gendisk *disk = swd->unit[drive].disk;
-
-		if (!disk)
-			continue;
-		blk_cleanup_disk(disk);
-		blk_mq_free_tag_set(&swd->unit[drive].tag_set);
+		swim_cleanup_floppy_disk(&swd->unit[drive]);
 	} while (drive--);
 	return err;
 }
@@ -911,8 +917,7 @@ static int swim_remove(struct platform_device *dev)
 
 	for (drive = 0; drive < swd->floppy_count; drive++) {
 		del_gendisk(swd->unit[drive].disk);
-		blk_cleanup_disk(swd->unit[drive].disk);
-		blk_mq_free_tag_set(&swd->unit[drive].tag_set);
+		swim_cleanup_floppy_disk(&swd->unit[drive]);
 	}
 
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
-- 
2.30.2


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

* [PATCH 09/14] swim: add a floppy registration bool which triggers del_gendisk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (7 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 08/14] swim: add helper for disk cleanup Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 10/14] swim: add error handling support for add_disk() Luis Chamberlain
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

Instead of calling del_gendisk() on exit alone, let's add
a registration bool to the floppy disk state, this way this can
be done on the shared caller, swim_cleanup_floppy_disk().

This will be more useful in subsequent patches. Right now, this
just shuffles functionality out to a helper in a safe way.

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

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index 3deb9d9a59c0..b3ee3dad5a8c 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -184,6 +184,7 @@ struct floppy_state {
 
 	int		track;
 	int		ref_count;
+	bool registered;
 
 	struct gendisk *disk;
 	struct blk_mq_tag_set tag_set;
@@ -778,6 +779,9 @@ static void swim_cleanup_floppy_disk(struct floppy_state *fs)
 	if (!disk)
 		return;
 
+	if (fs->registered)
+		del_gendisk(fs->disk);
+
 	blk_cleanup_disk(disk);
 	blk_mq_free_tag_set(&fs->tag_set);
 }
@@ -839,6 +843,7 @@ static int swim_floppy_init(struct swim_priv *swd)
 		swd->unit[drive].disk->private_data = &swd->unit[drive];
 		set_capacity(swd->unit[drive].disk, 2880);
 		add_disk(swd->unit[drive].disk);
+		swd->unit[drive].registered = true;
 	}
 
 	return 0;
@@ -915,10 +920,8 @@ static int swim_remove(struct platform_device *dev)
 	int drive;
 	struct resource *res;
 
-	for (drive = 0; drive < swd->floppy_count; drive++) {
-		del_gendisk(swd->unit[drive].disk);
+	for (drive = 0; drive < swd->floppy_count; drive++)
 		swim_cleanup_floppy_disk(&swd->unit[drive]);
-	}
 
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 
-- 
2.30.2


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

* [PATCH 10/14] swim: add error handling support for add_disk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (8 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 09/14] swim: add a floppy registration bool which triggers del_gendisk() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 11/14] block/ataflop: use the blk_cleanup_disk() helper Luis Chamberlain
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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.

Since we have a caller to do our unwinding for the disk,
and this is already dealt with safely we can re-use our
existing error path goto label which already deals with
the cleanup.

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

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index b3ee3dad5a8c..6f02941a3263 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -842,7 +842,9 @@ static int swim_floppy_init(struct swim_priv *swd)
 		swd->unit[drive].disk->events = DISK_EVENT_MEDIA_CHANGE;
 		swd->unit[drive].disk->private_data = &swd->unit[drive];
 		set_capacity(swd->unit[drive].disk, 2880);
-		add_disk(swd->unit[drive].disk);
+		err = add_disk(swd->unit[drive].disk);
+		if (err)
+			goto exit_put_disks;
 		swd->unit[drive].registered = true;
 	}
 
-- 
2.30.2


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

* [PATCH 11/14] block/ataflop: use the blk_cleanup_disk() helper
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (9 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 10/14] swim: add error handling support for add_disk() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 12/14] block/ataflop: add registration bool before calling del_gendisk() Luis Chamberlain
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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..6a865d42b1fd 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[type]);
 		}
 		blk_mq_free_tag_set(&unit[i].tag_set);
 	}
-- 
2.30.2


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

* [PATCH 12/14] block/ataflop: add registration bool before calling del_gendisk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (10 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 11/14] block/ataflop: use the blk_cleanup_disk() helper Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 13/14] block/ataflop: provide a helper for cleanup up an atari disk Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 14/14] block/ataflop add error handling support for add_disk() Luis Chamberlain
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

The ataflop assumes del_gendisk() is safe to call, this is only
true because add_disk() does not return a failure, but that will
change soon. And so, before we get to adding error handling for
that case, let's make sure we keep track of which disks actually
get registered. Then we use this to only call del_gendisk for them.

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

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 6a865d42b1fd..97f1c19f9c02 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -298,6 +298,7 @@ static struct atari_floppy_struct {
 				   disk change detection) */
 	int flags;		/* flags */
 	struct gendisk *disk[NUM_DISK_MINORS];
+	bool registered[NUM_DISK_MINORS];
 	int ref;
 	int type;
 	struct blk_mq_tag_set tag_set;
@@ -2000,8 +2001,10 @@ static void ataflop_probe(dev_t dev)
 		return;
 	mutex_lock(&ataflop_probe_lock);
 	if (!unit[drive].disk[type]) {
-		if (ataflop_alloc_disk(drive, type) == 0)
+		if (ataflop_alloc_disk(drive, type) == 0) {
 			add_disk(unit[drive].disk[type]);
+			unit[drive].registered[type] = true;
+		}
 	}
 	mutex_unlock(&ataflop_probe_lock);
 }
@@ -2065,6 +2068,7 @@ static int __init atari_floppy_init (void)
 		unit[i].track = -1;
 		unit[i].flags = 0;
 		add_disk(unit[i].disk[0]);
+		unit[i].registered[0] = true;
 	}
 
 	printk(KERN_INFO "Atari floppy driver: max. %cD, %strack buffering\n",
@@ -2133,7 +2137,8 @@ static void __exit atari_floppy_exit(void)
 		for (type = 0; type < NUM_DISK_MINORS; type++) {
 			if (!unit[i].disk[type])
 				continue;
-			del_gendisk(unit[i].disk[type]);
+			if (unit[i].registered[type])
+				del_gendisk(unit[i].disk[type]);
 			blk_cleanup_disk(unit[i].disk[type]);
 		}
 		blk_mq_free_tag_set(&unit[i].tag_set);
-- 
2.30.2


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

* [PATCH 13/14] block/ataflop: provide a helper for cleanup up an atari disk
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (11 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 12/14] block/ataflop: add registration bool before calling del_gendisk() Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  2021-09-04  1:35 ` [PATCH 14/14] block/ataflop add error handling support for add_disk() Luis Chamberlain
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: linux-block, linux-kernel, Luis Chamberlain

Instead of using two separate code paths for cleaning up an atari disk,
use one. We take the more careful approach to check for *all* disk
types, as is done on exit. The init path didn't have that check as
the alternative disk types are only probed for later, they are not
initialized by default.

Yes, there is a shared tag for all disks.

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

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 97f1c19f9c02..e9d874f51597 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2009,6 +2009,20 @@ static void ataflop_probe(dev_t dev)
 	mutex_unlock(&ataflop_probe_lock);
 }
 
+static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs)
+{
+	int type;
+
+	for (type = 0; type < NUM_DISK_MINORS; type++) {
+		if (!fs->disk[type])
+			continue;
+		if (fs->registered[type])
+			del_gendisk(fs->disk[type]);
+		blk_cleanup_disk(fs->disk[type]);
+	}
+	blk_mq_free_tag_set(&fs->tag_set);
+}
+
 static int __init atari_floppy_init (void)
 {
 	int i;
@@ -2079,10 +2093,8 @@ static int __init atari_floppy_init (void)
 	return 0;
 
 err:
-	while (--i >= 0) {
-		blk_cleanup_disk(unit[i].disk[0]);
-		blk_mq_free_tag_set(&unit[i].tag_set);
-	}
+	while (--i >= 0)
+		atari_cleanup_floppy_disk(&unit[i]);
 
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 out_unlock:
@@ -2131,18 +2143,10 @@ __setup("floppy=", atari_floppy_setup);
 
 static void __exit atari_floppy_exit(void)
 {
-	int i, type;
+	int i;
 
-	for (i = 0; i < FD_MAX_UNITS; i++) {
-		for (type = 0; type < NUM_DISK_MINORS; type++) {
-			if (!unit[i].disk[type])
-				continue;
-			if (unit[i].registered[type])
-				del_gendisk(unit[i].disk[type]);
-			blk_cleanup_disk(unit[i].disk[type]);
-		}
-		blk_mq_free_tag_set(&unit[i].tag_set);
-	}
+	for (i = 0; i < FD_MAX_UNITS; i++)
+		atari_cleanup_floppy_disk(&unit[i]);
 	unregister_blkdev(FLOPPY_MAJOR, "fd");
 
 	del_timer_sync(&fd_timer);
-- 
2.30.2


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

* [PATCH 14/14] block/ataflop add error handling support for add_disk()
  2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
                   ` (12 preceding siblings ...)
  2021-09-04  1:35 ` [PATCH 13/14] block/ataflop: provide a helper for cleanup up an atari disk Luis Chamberlain
@ 2021-09-04  1:35 ` Luis Chamberlain
  13 siblings, 0 replies; 15+ messages in thread
From: Luis Chamberlain @ 2021-09-04  1:35 UTC (permalink / raw)
  To: axboe, efremov, hch; +Cc: 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/ataflop.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index e9d874f51597..5dc9b3d32415 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;
 		unit[i].registered[0] = true;
 	}
 
@@ -2092,6 +2094,8 @@ static int __init atari_floppy_init (void)
 
 	return 0;
 
+err_out_dma:
+	atari_stram_free(DMABuffer);
 err:
 	while (--i >= 0)
 		atari_cleanup_floppy_disk(&unit[i]);
-- 
2.30.2


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

end of thread, other threads:[~2021-09-04  1:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04  1:35 [PATCH 00/14] block: 6th batch of add_disk() error handling conversions Luis Chamberlain
2021-09-04  1:35 ` [PATCH 01/14] block/swim3: add error handling support for add_disk() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 02/14] floppy: fix add_disk() assumption on exit due to new developments Luis Chamberlain
2021-09-04  1:35 ` [PATCH 03/14] floppy: use blk_cleanup_disk() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 04/14] floppy: fix calling platform_device_unregister() on invalid drives Luis Chamberlain
2021-09-04  1:35 ` [PATCH 05/14] floppy: add error handling support for add_disk() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 06/14] amiflop: " Luis Chamberlain
2021-09-04  1:35 ` [PATCH 07/14] swim: simplify using blk_cleanup_disk() on swim_remove() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 08/14] swim: add helper for disk cleanup Luis Chamberlain
2021-09-04  1:35 ` [PATCH 09/14] swim: add a floppy registration bool which triggers del_gendisk() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 10/14] swim: add error handling support for add_disk() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 11/14] block/ataflop: use the blk_cleanup_disk() helper Luis Chamberlain
2021-09-04  1:35 ` [PATCH 12/14] block/ataflop: add registration bool before calling del_gendisk() Luis Chamberlain
2021-09-04  1:35 ` [PATCH 13/14] block/ataflop: provide a helper for cleanup up an atari disk Luis Chamberlain
2021-09-04  1:35 ` [PATCH 14/14] block/ataflop add error handling support for add_disk() 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).