* [PATCH v4 0/4] last set for add_disk() error handling
@ 2021-11-03 18:12 ` Luis Chamberlain
0 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
This v4 only updates the last 2 patches from my v3 series of stragglers
to account for Christoph's request to split the __register_blkdev()
probe call changes into 3 patches, one for documentation, the other 2
patches for each respective driver.
I also noticed I had a typo on the documentation, so fixed that.
Luis Chamberlain (4):
block: update __register_blkdev() probe documentation
ataflop: address add_disk() error handling on probe
floppy: address add_disk() error handling on probe
block: add __must_check for *add_disk*() callers
block/genhd.c | 11 +++++++----
drivers/block/ataflop.c | 16 +++++++++++++---
drivers/block/floppy.c | 15 +++++++++++++--
include/linux/genhd.h | 6 +++---
4 files changed, 36 insertions(+), 12 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 0/4] last set for add_disk() error handling
@ 2021-11-03 18:12 ` Luis Chamberlain
0 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
This v4 only updates the last 2 patches from my v3 series of stragglers
to account for Christoph's request to split the __register_blkdev()
probe call changes into 3 patches, one for documentation, the other 2
patches for each respective driver.
I also noticed I had a typo on the documentation, so fixed that.
Luis Chamberlain (4):
block: update __register_blkdev() probe documentation
ataflop: address add_disk() error handling on probe
floppy: address add_disk() error handling on probe
block: add __must_check for *add_disk*() callers
block/genhd.c | 11 +++++++----
drivers/block/ataflop.c | 16 +++++++++++++---
drivers/block/floppy.c | 15 +++++++++++++--
include/linux/genhd.h | 6 +++---
4 files changed, 36 insertions(+), 12 deletions(-)
--
2.33.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v4 1/4] block: update __register_blkdev() probe documentation
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:12 ` Luis Chamberlain
-1 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
__register_blkdev() is used to register a probe callback, and
that callback is typically used to call add_disk(). Now that
we are able to capture errors for add_disk(), we need to fix
those probe calls where add_disk() fails and clean up resources.
We don't extend the probe call to return the error given:
1) we'd have to always special-case the case where the disk
was already present, as otherwise concurrent requests to
open an existing block device would fail, and this would be
a userspace visible change
2) the error from ilookup() on blkdev_get_no_open() is sufficient
3) The only thing the probe call is used for is to support
pre-devtmpfs, pre-udev semantics that want to create disks when
their pre-created device node is accessed, and so we don't care
for failures on probe there.
Expand documentation for the probe callback to ensure users cleanup
resources if add_disk() is used and to clarify this interface may be
removed in the future.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
block/genhd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/block/genhd.c b/block/genhd.c
index 4ed87f25276a..2f5b7e24e88a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -213,7 +213,10 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
* @major: the requested major device number [1..BLKDEV_MAJOR_MAX-1]. If
* @major = 0, try to allocate any unused major number.
* @name: the name of the new block device as a zero terminated string
- * @probe: allback that is called on access to any minor number of @major
+ * @probe: pre-devtmpfs / pre-udev callback used to create disks when their
+ * pre-created device node is accessed. When a probe call uses
+ * add_disk() and it fails the driver must cleanup resources. This
+ * interface may soon be removed.
*
* The @name must be unique within the system.
*
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 1/4] block: update __register_blkdev() probe documentation
@ 2021-11-03 18:12 ` Luis Chamberlain
0 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
__register_blkdev() is used to register a probe callback, and
that callback is typically used to call add_disk(). Now that
we are able to capture errors for add_disk(), we need to fix
those probe calls where add_disk() fails and clean up resources.
We don't extend the probe call to return the error given:
1) we'd have to always special-case the case where the disk
was already present, as otherwise concurrent requests to
open an existing block device would fail, and this would be
a userspace visible change
2) the error from ilookup() on blkdev_get_no_open() is sufficient
3) The only thing the probe call is used for is to support
pre-devtmpfs, pre-udev semantics that want to create disks when
their pre-created device node is accessed, and so we don't care
for failures on probe there.
Expand documentation for the probe callback to ensure users cleanup
resources if add_disk() is used and to clarify this interface may be
removed in the future.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
block/genhd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/block/genhd.c b/block/genhd.c
index 4ed87f25276a..2f5b7e24e88a 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -213,7 +213,10 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
* @major: the requested major device number [1..BLKDEV_MAJOR_MAX-1]. If
* @major = 0, try to allocate any unused major number.
* @name: the name of the new block device as a zero terminated string
- * @probe: allback that is called on access to any minor number of @major
+ * @probe: pre-devtmpfs / pre-udev callback used to create disks when their
+ * pre-created device node is accessed. When a probe call uses
+ * add_disk() and it fails the driver must cleanup resources. This
+ * interface may soon be removed.
*
* The @name must be unique within the system.
*
--
2.33.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/4] ataflop: address add_disk() error handling on probe
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:12 ` Luis Chamberlain
-1 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/block/ataflop.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 170dd193cef6..e981b351f37e 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2012,6 +2012,7 @@ static void ataflop_probe(dev_t dev)
{
int drive = MINOR(dev) & 3;
int type = MINOR(dev) >> 2;
+ int err = 0;
if (type)
type--;
@@ -2019,11 +2020,20 @@ static void ataflop_probe(dev_t dev)
if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
return;
if (!unit[drive].disk[type]) {
- if (ataflop_alloc_disk(drive, type) == 0) {
- add_disk(unit[drive].disk[type]);
- unit[drive].registered[type] = true;
+ err = ataflop_alloc_disk(drive, type);
+ if (err == 0) {
+ err = add_disk(unit[drive].disk[type]);
+ if (err)
+ goto err_out;
+ else
+ unit[drive].registered[type] = true;
}
}
+ return;
+
+err_out:
+ blk_cleanup_disk(unit[drive].disk[type]);
+ unit[drive].disk[type] = NULL;
}
static void atari_floppy_cleanup(void)
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 2/4] ataflop: address add_disk() error handling on probe
@ 2021-11-03 18:12 ` Luis Chamberlain
0 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/block/ataflop.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 170dd193cef6..e981b351f37e 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2012,6 +2012,7 @@ static void ataflop_probe(dev_t dev)
{
int drive = MINOR(dev) & 3;
int type = MINOR(dev) >> 2;
+ int err = 0;
if (type)
type--;
@@ -2019,11 +2020,20 @@ static void ataflop_probe(dev_t dev)
if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
return;
if (!unit[drive].disk[type]) {
- if (ataflop_alloc_disk(drive, type) == 0) {
- add_disk(unit[drive].disk[type]);
- unit[drive].registered[type] = true;
+ err = ataflop_alloc_disk(drive, type);
+ if (err == 0) {
+ err = add_disk(unit[drive].disk[type]);
+ if (err)
+ goto err_out;
+ else
+ unit[drive].registered[type] = true;
}
}
+ return;
+
+err_out:
+ blk_cleanup_disk(unit[drive].disk[type]);
+ unit[drive].disk[type] = NULL;
}
static void atari_floppy_cleanup(void)
--
2.33.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 3/4] floppy: address add_disk() error handling on probe
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:12 ` Luis Chamberlain
-1 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/block/floppy.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 3873e789478e..255e88efb535 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4522,6 +4522,7 @@ static void floppy_probe(dev_t dev)
{
unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+ int err = 0;
if (drive >= N_DRIVE || !floppy_available(drive) ||
type >= ARRAY_SIZE(floppy_type))
@@ -4529,10 +4530,20 @@ static void floppy_probe(dev_t dev)
mutex_lock(&floppy_probe_lock);
if (!disks[drive][type]) {
- if (floppy_alloc_disk(drive, type) == 0)
- add_disk(disks[drive][type]);
+ err = floppy_alloc_disk(drive, type);
+ if (err == 0) {
+ err = add_disk(disks[drive][type]);
+ if (err)
+ goto err_out;
+ }
}
mutex_unlock(&floppy_probe_lock);
+ return;
+
+err_out:
+ blk_cleanup_disk(disks[drive][type]);
+ disks[drive][type] = NULL;
+ mutex_unlock(&floppy_probe_lock);
}
static int __init do_floppy_init(void)
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 3/4] floppy: address add_disk() error handling on probe
@ 2021-11-03 18:12 ` Luis Chamberlain
0 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
drivers/block/floppy.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 3873e789478e..255e88efb535 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4522,6 +4522,7 @@ static void floppy_probe(dev_t dev)
{
unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+ int err = 0;
if (drive >= N_DRIVE || !floppy_available(drive) ||
type >= ARRAY_SIZE(floppy_type))
@@ -4529,10 +4530,20 @@ static void floppy_probe(dev_t dev)
mutex_lock(&floppy_probe_lock);
if (!disks[drive][type]) {
- if (floppy_alloc_disk(drive, type) == 0)
- add_disk(disks[drive][type]);
+ err = floppy_alloc_disk(drive, type);
+ if (err == 0) {
+ err = add_disk(disks[drive][type]);
+ if (err)
+ goto err_out;
+ }
}
mutex_unlock(&floppy_probe_lock);
+ return;
+
+err_out:
+ blk_cleanup_disk(disks[drive][type]);
+ disks[drive][type] = NULL;
+ mutex_unlock(&floppy_probe_lock);
}
static int __init do_floppy_init(void)
--
2.33.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 4/4] block: add __must_check for *add_disk*() callers
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:12 ` Luis Chamberlain
-1 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
Now that we have done a spring cleaning on all drivers and added
error checking / handling, let's keep it that way and ensure
no new drivers fail to stick with it.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
block/genhd.c | 6 +++---
include/linux/genhd.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 2f5b7e24e88a..2263f7862241 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -394,8 +394,8 @@ static void disk_scan_partitions(struct gendisk *disk)
* This function registers the partitioning information in @disk
* with the kernel.
*/
-int device_add_disk(struct device *parent, struct gendisk *disk,
- const struct attribute_group **groups)
+int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
+ const struct attribute_group **groups)
{
struct device *ddev = disk_to_dev(disk);
@@ -542,7 +542,7 @@ int device_add_disk(struct device *parent, struct gendisk *disk,
out_free_ext_minor:
if (disk->major == BLOCK_EXT_MAJOR)
blk_free_ext_minor(disk->first_minor);
- return WARN_ON_ONCE(ret); /* keep until all callers handle errors */
+ return ret;
}
EXPORT_SYMBOL(device_add_disk);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 59eabbc3a36b..f7d6810e68b3 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -205,9 +205,9 @@ static inline dev_t disk_devt(struct gendisk *disk)
void disk_uevent(struct gendisk *disk, enum kobject_action action);
/* block/genhd.c */
-int device_add_disk(struct device *parent, struct gendisk *disk,
- const struct attribute_group **groups);
-static inline int add_disk(struct gendisk *disk)
+int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
+ const struct attribute_group **groups);
+static inline int __must_check add_disk(struct gendisk *disk)
{
return device_add_disk(NULL, disk, NULL);
}
--
2.33.0
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v4 4/4] block: add __must_check for *add_disk*() callers
@ 2021-11-03 18:12 ` Luis Chamberlain
0 siblings, 0 replies; 22+ messages in thread
From: Luis Chamberlain @ 2021-11-03 18:12 UTC (permalink / raw)
To: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, mcgrof
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
Now that we have done a spring cleaning on all drivers and added
error checking / handling, let's keep it that way and ensure
no new drivers fail to stick with it.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
block/genhd.c | 6 +++---
include/linux/genhd.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 2f5b7e24e88a..2263f7862241 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -394,8 +394,8 @@ static void disk_scan_partitions(struct gendisk *disk)
* This function registers the partitioning information in @disk
* with the kernel.
*/
-int device_add_disk(struct device *parent, struct gendisk *disk,
- const struct attribute_group **groups)
+int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
+ const struct attribute_group **groups)
{
struct device *ddev = disk_to_dev(disk);
@@ -542,7 +542,7 @@ int device_add_disk(struct device *parent, struct gendisk *disk,
out_free_ext_minor:
if (disk->major == BLOCK_EXT_MAJOR)
blk_free_ext_minor(disk->first_minor);
- return WARN_ON_ONCE(ret); /* keep until all callers handle errors */
+ return ret;
}
EXPORT_SYMBOL(device_add_disk);
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 59eabbc3a36b..f7d6810e68b3 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -205,9 +205,9 @@ static inline dev_t disk_devt(struct gendisk *disk)
void disk_uevent(struct gendisk *disk, enum kobject_action action);
/* block/genhd.c */
-int device_add_disk(struct device *parent, struct gendisk *disk,
- const struct attribute_group **groups);
-static inline int add_disk(struct gendisk *disk)
+int __must_check device_add_disk(struct device *parent, struct gendisk *disk,
+ const struct attribute_group **groups);
+static inline int __must_check add_disk(struct gendisk *disk)
{
return device_add_disk(NULL, disk, NULL);
}
--
2.33.0
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v4 1/4] block: update __register_blkdev() probe documentation
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:18 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:18 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 1/4] block: update __register_blkdev() probe documentation
@ 2021-11-03 18:18 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:18 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/4] ataflop: address add_disk() error handling on probe
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:20 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:20 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
On Wed, Nov 03, 2021 at 11:12:56AM -0700, Luis Chamberlain wrote:
> We need to cleanup resources on the probe() callback registered
> with __register_blkdev(), now that add_disk() error handling is
> supported. Address this.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
> drivers/block/ataflop.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
> index 170dd193cef6..e981b351f37e 100644
> --- a/drivers/block/ataflop.c
> +++ b/drivers/block/ataflop.c
> @@ -2012,6 +2012,7 @@ static void ataflop_probe(dev_t dev)
> {
> int drive = MINOR(dev) & 3;
> int type = MINOR(dev) >> 2;
> + int err = 0;
>
> if (type)
> type--;
> @@ -2019,11 +2020,20 @@ static void ataflop_probe(dev_t dev)
> if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
> return;
> if (!unit[drive].disk[type]) {
> + err = ataflop_alloc_disk(drive, type);
> + if (err == 0) {
> + err = add_disk(unit[drive].disk[type]);
> + if (err)
> + goto err_out;
> + else
> + unit[drive].registered[type] = true;
This looks weird. Why not:
if (unit[drive].disk[type])
return;
if (ataflop_alloc_disk(drive, type))
return;
if (add_disk(unit[drive].disk[type]))
goto cleanup_disk;
unit[drive].registered[type] = true;
return;
cleanup_disk:
blk_cleanup_disk(unit[drive].disk[type]);
unit[drive].disk[type] = NULL;
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/4] ataflop: address add_disk() error handling on probe
@ 2021-11-03 18:20 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:20 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
On Wed, Nov 03, 2021 at 11:12:56AM -0700, Luis Chamberlain wrote:
> We need to cleanup resources on the probe() callback registered
> with __register_blkdev(), now that add_disk() error handling is
> supported. Address this.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
> drivers/block/ataflop.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
> index 170dd193cef6..e981b351f37e 100644
> --- a/drivers/block/ataflop.c
> +++ b/drivers/block/ataflop.c
> @@ -2012,6 +2012,7 @@ static void ataflop_probe(dev_t dev)
> {
> int drive = MINOR(dev) & 3;
> int type = MINOR(dev) >> 2;
> + int err = 0;
>
> if (type)
> type--;
> @@ -2019,11 +2020,20 @@ static void ataflop_probe(dev_t dev)
> if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
> return;
> if (!unit[drive].disk[type]) {
> + err = ataflop_alloc_disk(drive, type);
> + if (err == 0) {
> + err = add_disk(unit[drive].disk[type]);
> + if (err)
> + goto err_out;
> + else
> + unit[drive].registered[type] = true;
This looks weird. Why not:
if (unit[drive].disk[type])
return;
if (ataflop_alloc_disk(drive, type))
return;
if (add_disk(unit[drive].disk[type]))
goto cleanup_disk;
unit[drive].registered[type] = true;
return;
cleanup_disk:
blk_cleanup_disk(unit[drive].disk[type]);
unit[drive].disk[type] = NULL;
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/4] floppy: address add_disk() error handling on probe
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:20 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:20 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
On Wed, Nov 03, 2021 at 11:12:57AM -0700, Luis Chamberlain wrote:
> We need to cleanup resources on the probe() callback registered
> with __register_blkdev(), now that add_disk() error handling is
> supported. Address this.
Same comment as for ataflop.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 3/4] floppy: address add_disk() error handling on probe
@ 2021-11-03 18:20 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:20 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
On Wed, Nov 03, 2021 at 11:12:57AM -0700, Luis Chamberlain wrote:
> We need to cleanup resources on the probe() callback registered
> with __register_blkdev(), now that add_disk() error handling is
> supported. Address this.
Same comment as for ataflop.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 4/4] block: add __must_check for *add_disk*() callers
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 18:20 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:20 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 4/4] block: add __must_check for *add_disk*() callers
@ 2021-11-03 18:20 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:20 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/4] ataflop: address add_disk() error handling on probe
2021-11-03 18:20 ` Christoph Hellwig
@ 2021-11-03 18:21 ` Christoph Hellwig
-1 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:21 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
Or maybe I should stop nitpicking for these cruft drivers.
I'm ok with this and the next patch:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 2/4] ataflop: address add_disk() error handling on probe
@ 2021-11-03 18:21 ` Christoph Hellwig
0 siblings, 0 replies; 22+ messages in thread
From: Christoph Hellwig @ 2021-11-03 18:21 UTC (permalink / raw)
To: Luis Chamberlain
Cc: axboe, hch, penguin-kernel, dan.j.williams, vishal.l.verma,
dave.jiang, ira.weiny, richard, miquel.raynal, vigneshr, efremov,
song, martin.petersen, hare, jack, ming.lei, tj, linux-mtd,
linux-scsi, linux-raid, linux-block, linux-kernel
Or maybe I should stop nitpicking for these cruft drivers.
I'm ok with this and the next patch:
Reviewed-by: Christoph Hellwig <hch@lst.de>
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/4] last set for add_disk() error handling
2021-11-03 18:12 ` Luis Chamberlain
@ 2021-11-03 19:28 ` Jens Axboe
-1 siblings, 0 replies; 22+ messages in thread
From: Jens Axboe @ 2021-11-03 19:28 UTC (permalink / raw)
To: Luis Chamberlain, hch, penguin-kernel, dan.j.williams,
vishal.l.verma, dave.jiang, ira.weiny, richard, miquel.raynal,
vigneshr, efremov, song, martin.petersen, hare, jack, ming.lei,
tj
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
On 11/3/21 12:12 PM, Luis Chamberlain wrote:
> This v4 only updates the last 2 patches from my v3 series of stragglers
> to account for Christoph's request to split the __register_blkdev()
> probe call changes into 3 patches, one for documentation, the other 2
> patches for each respective driver.
Part of the reason why I think this has taken so long is that there's
a hundreds of series, and then you get partial updates, etc. It's really
super hard to keep track of...
Can we please just have one final series, not 1 and then another one
that turns the last two into more?
--
Jens Axboe
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v4 0/4] last set for add_disk() error handling
@ 2021-11-03 19:28 ` Jens Axboe
0 siblings, 0 replies; 22+ messages in thread
From: Jens Axboe @ 2021-11-03 19:28 UTC (permalink / raw)
To: Luis Chamberlain, hch, penguin-kernel, dan.j.williams,
vishal.l.verma, dave.jiang, ira.weiny, richard, miquel.raynal,
vigneshr, efremov, song, martin.petersen, hare, jack, ming.lei,
tj
Cc: linux-mtd, linux-scsi, linux-raid, linux-block, linux-kernel
On 11/3/21 12:12 PM, Luis Chamberlain wrote:
> This v4 only updates the last 2 patches from my v3 series of stragglers
> to account for Christoph's request to split the __register_blkdev()
> probe call changes into 3 patches, one for documentation, the other 2
> patches for each respective driver.
Part of the reason why I think this has taken so long is that there's
a hundreds of series, and then you get partial updates, etc. It's really
super hard to keep track of...
Can we please just have one final series, not 1 and then another one
that turns the last two into more?
--
Jens Axboe
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2021-11-03 19:29 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 18:12 [PATCH v4 0/4] last set for add_disk() error handling Luis Chamberlain
2021-11-03 18:12 ` Luis Chamberlain
2021-11-03 18:12 ` [PATCH v4 1/4] block: update __register_blkdev() probe documentation Luis Chamberlain
2021-11-03 18:12 ` Luis Chamberlain
2021-11-03 18:18 ` Christoph Hellwig
2021-11-03 18:18 ` Christoph Hellwig
2021-11-03 18:12 ` [PATCH v4 2/4] ataflop: address add_disk() error handling on probe Luis Chamberlain
2021-11-03 18:12 ` Luis Chamberlain
2021-11-03 18:20 ` Christoph Hellwig
2021-11-03 18:20 ` Christoph Hellwig
2021-11-03 18:21 ` Christoph Hellwig
2021-11-03 18:21 ` Christoph Hellwig
2021-11-03 18:12 ` [PATCH v4 3/4] floppy: " Luis Chamberlain
2021-11-03 18:12 ` Luis Chamberlain
2021-11-03 18:20 ` Christoph Hellwig
2021-11-03 18:20 ` Christoph Hellwig
2021-11-03 18:12 ` [PATCH v4 4/4] block: add __must_check for *add_disk*() callers Luis Chamberlain
2021-11-03 18:12 ` Luis Chamberlain
2021-11-03 18:20 ` Christoph Hellwig
2021-11-03 18:20 ` Christoph Hellwig
2021-11-03 19:28 ` [PATCH v4 0/4] last set for add_disk() error handling Jens Axboe
2021-11-03 19:28 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.