Jens,
as requested, I've folded all pending changes into this series. This
v5 pegs on Christoph's reviewed-by tags and since I was respinning I
modified the ataprobe and floppy driver changes as he suggested.
I think this is it. The world of floppy has been exciting for v5.16.
This goes based on your axboe/for-next tree as of just a few minutes ago.
Luis Chamberlain (13):
nvdimm/btt: use goto error labels on btt_blk_init()
nvdimm/btt: add error handling support for add_disk()
nvdimm/blk: avoid calling del_gendisk() on early failures
nvdimm/blk: add error handling support for add_disk()
nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
nvdimm/pmem: use add_disk() error handling
z2ram: add error handling support for add_disk()
block/sunvdc: add error handling support for add_disk()
mtd/ubi/block: add error handling support for add_disk()
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
Tetsuo Handa (1):
ataflop: remove ataflop_probe_lock mutex
block/genhd.c | 11 +++++---
drivers/block/ataflop.c | 61 +++++++++++++++++++++++++----------------
drivers/block/floppy.c | 17 +++++++++---
drivers/block/sunvdc.c | 14 ++++++++--
drivers/block/z2ram.c | 7 +++--
drivers/mtd/ubi/block.c | 8 +++++-
drivers/nvdimm/blk.c | 21 ++++++++++----
drivers/nvdimm/btt.c | 20 +++++++++-----
drivers/nvdimm/pmem.c | 21 ++++++++++----
include/linux/genhd.h | 6 ++--
10 files changed, 127 insertions(+), 59 deletions(-)
--
2.33.0
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/mtd/ubi/block.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/ubi/block.c b/drivers/mtd/ubi/block.c
index e003b4b44ffa..062e6c2c45f5 100644
--- a/drivers/mtd/ubi/block.c
+++ b/drivers/mtd/ubi/block.c
@@ -447,12 +447,18 @@ int ubiblock_create(struct ubi_volume_info *vi)
list_add_tail(&dev->list, &ubiblock_devices);
/* Must be the last step: anyone can call file ops from now on */
- add_disk(dev->gd);
+ ret = add_disk(dev->gd);
+ if (ret)
+ goto out_destroy_wq;
+
dev_info(disk_to_dev(dev->gd), "created from ubi%d:%d(%s)",
dev->ubi_num, dev->vol_id, vi->name);
mutex_unlock(&devices_mutex);
return 0;
+out_destroy_wq:
+ list_del(&dev->list);
+ destroy_workqueue(dev->wq);
out_remove_minor:
idr_remove(&ubiblock_minor_idr, gd->first_minor);
out_cleanup_disk:
--
2.33.0
From: Tetsuo Handa <[email protected]>
Commit bf9c0538e485b591 ("ataflop: use a separate gendisk for each media
format") introduced ataflop_probe_lock mutex, but forgot to unlock the
mutex when atari_floppy_init() (i.e. module loading) succeeded. This will
result in double lock deadlock if ataflop_probe() is called. Also,
unregister_blkdev() must not be called from atari_floppy_init() with
ataflop_probe_lock held when atari_floppy_init() failed, for
ataflop_probe() waits for ataflop_probe_lock with major_names_lock held
(i.e. AB-BA deadlock).
__register_blkdev() needs to be called last in order to avoid calling
ataflop_probe() when atari_floppy_init() is about to fail, for memory for
completing already-started ataflop_probe() safely will be released as soon
as atari_floppy_init() released ataflop_probe_lock mutex.
As with commit 8b52d8be86d72308 ("loop: reorder loop_exit"),
unregister_blkdev() needs to be called first in order to avoid calling
ataflop_alloc_disk() from ataflop_probe() after del_gendisk() from
atari_floppy_exit().
By relocating __register_blkdev() / unregister_blkdev() as explained above,
we can remove ataflop_probe_lock mutex, for probe function and __exit
function are serialized by major_names_lock mutex.
Signed-off-by: Tetsuo Handa <[email protected]>
Fixes: bf9c0538e485b591 ("ataflop: use a separate gendisk for each media format")
Reviewed-by: Luis Chamberlain <[email protected]>
Tested-by: Michael Schmitz <[email protected]>
---
drivers/block/ataflop.c | 47 +++++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index d14bdc3589b2..170dd193cef6 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2008,8 +2008,6 @@ static int ataflop_alloc_disk(unsigned int drive, unsigned int type)
return 0;
}
-static DEFINE_MUTEX(ataflop_probe_lock);
-
static void ataflop_probe(dev_t dev)
{
int drive = MINOR(dev) & 3;
@@ -2020,14 +2018,32 @@ static void ataflop_probe(dev_t dev)
if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
return;
- mutex_lock(&ataflop_probe_lock);
if (!unit[drive].disk[type]) {
if (ataflop_alloc_disk(drive, type) == 0) {
add_disk(unit[drive].disk[type]);
unit[drive].registered[type] = true;
}
}
- mutex_unlock(&ataflop_probe_lock);
+}
+
+static void atari_floppy_cleanup(void)
+{
+ int i;
+ int 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_queue(unit[i].disk[type]->queue);
+ put_disk(unit[i].disk[type]);
+ }
+ blk_mq_free_tag_set(&unit[i].tag_set);
+ }
+
+ del_timer_sync(&fd_timer);
+ atari_stram_free(DMABuffer);
}
static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs)
@@ -2053,11 +2069,6 @@ static int __init atari_floppy_init (void)
/* Amiga, Mac, ... don't have Atari-compatible floppy :-) */
return -ENODEV;
- mutex_lock(&ataflop_probe_lock);
- ret = __register_blkdev(FLOPPY_MAJOR, "fd", ataflop_probe);
- if (ret)
- goto out_unlock;
-
for (i = 0; i < FD_MAX_UNITS; i++) {
memset(&unit[i].tag_set, 0, sizeof(unit[i].tag_set));
unit[i].tag_set.ops = &ataflop_mq_ops;
@@ -2113,7 +2124,12 @@ static int __init atari_floppy_init (void)
UseTrackbuffer ? "" : "no ");
config_types();
- return 0;
+ ret = __register_blkdev(FLOPPY_MAJOR, "fd", ataflop_probe);
+ if (ret) {
+ printk(KERN_ERR "atari_floppy_init: cannot register block device\n");
+ atari_floppy_cleanup();
+ }
+ return ret;
err_out_dma:
atari_stram_free(DMABuffer);
@@ -2121,9 +2137,6 @@ static int __init atari_floppy_init (void)
while (--i >= 0)
atari_cleanup_floppy_disk(&unit[i]);
- unregister_blkdev(FLOPPY_MAJOR, "fd");
-out_unlock:
- mutex_unlock(&ataflop_probe_lock);
return ret;
}
@@ -2168,14 +2181,8 @@ __setup("floppy=", atari_floppy_setup);
static void __exit atari_floppy_exit(void)
{
- int i;
-
- for (i = 0; i < FD_MAX_UNITS; i++)
- atari_cleanup_floppy_disk(&unit[i]);
unregister_blkdev(FLOPPY_MAJOR, "fd");
-
- del_timer_sync(&fd_timer);
- atari_stram_free( DMABuffer );
+ atari_floppy_cleanup();
}
module_init(atari_floppy_init)
--
2.33.0
Now that device_add_disk() supports returning an error, use
that. We must unwind alloc_dax() on error.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/nvdimm/pmem.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index bcfc36e7295f..37fc03058556 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -462,7 +462,9 @@ static int pmem_attach_disk(struct device *dev,
dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
pmem->dax_dev = dax_dev;
- device_add_disk(dev, disk, pmem_attribute_groups);
+ rc = device_add_disk(dev, disk, pmem_attribute_groups);
+ if (rc)
+ goto out_cleanup_dax;
if (devm_add_action_or_reset(dev, pmem_release_disk, pmem))
return -ENOMEM;
@@ -473,6 +475,10 @@ static int pmem_attach_disk(struct device *dev,
if (!pmem->bb_state)
dev_warn(dev, "'badblocks' notification disabled\n");
return 0;
+
+out_cleanup_dax:
+ kill_dax(pmem->dax_dev);
+ put_dax(pmem->dax_dev);
out:
blk_cleanup_disk(pmem->disk);
return rc;
--
2.33.0
We never checked for errors on add_disk() as this function
returned void. Now that this is fixed, use the shiny new
error handling.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/nvdimm/btt.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvdimm/btt.c b/drivers/nvdimm/btt.c
index 5cb6d7ac6e36..38ed53eeea5e 100644
--- a/drivers/nvdimm/btt.c
+++ b/drivers/nvdimm/btt.c
@@ -1541,7 +1541,9 @@ static int btt_blk_init(struct btt *btt)
}
set_capacity(btt->btt_disk, btt->nlba * btt->sector_size >> 9);
- device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL);
+ rc = device_add_disk(&btt->nd_btt->dev, btt->btt_disk, NULL);
+ if (rc)
+ goto out_cleanup_disk;
btt->nd_btt->size = btt->nlba * (u64)btt->sector_size;
nvdimm_check_and_set_ro(btt->btt_disk);
--
2.33.0
Prior to devm being able to use pmem_release_disk() there are other
failure which can occur for which we must account for and release the
disk for. Address those few cases.
Fixes: 3dd60fb9d95d ("nvdimm/pmem: stop using q_usage_count as external pgmap refcount")
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/nvdimm/pmem.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
index c74d7bceb222..bcfc36e7295f 100644
--- a/drivers/nvdimm/pmem.c
+++ b/drivers/nvdimm/pmem.c
@@ -428,8 +428,10 @@ static int pmem_attach_disk(struct device *dev,
bb_range.end = res->end;
}
- if (IS_ERR(addr))
- return PTR_ERR(addr);
+ if (IS_ERR(addr)) {
+ rc = PTR_ERR(addr);
+ goto out;
+ }
pmem->virt_addr = addr;
blk_queue_write_cache(q, true, fua);
@@ -454,7 +456,8 @@ static int pmem_attach_disk(struct device *dev,
flags = DAXDEV_F_SYNC;
dax_dev = alloc_dax(pmem, disk->disk_name, &pmem_dax_ops, flags);
if (IS_ERR(dax_dev)) {
- return PTR_ERR(dax_dev);
+ rc = PTR_ERR(dax_dev);
+ goto out;
}
dax_write_cache(dax_dev, nvdimm_has_cache(nd_region));
pmem->dax_dev = dax_dev;
@@ -469,8 +472,10 @@ static int pmem_attach_disk(struct device *dev,
"badblocks");
if (!pmem->bb_state)
dev_warn(dev, "'badblocks' notification disabled\n");
-
return 0;
+out:
+ blk_cleanup_disk(pmem->disk);
+ return rc;
}
static int nd_pmem_probe(struct device *dev)
--
2.33.0
We need to cleanup resources on the probe() callback registered
with __register_blkdev(), now that add_disk() error handling is
supported. Address this.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/block/ataflop.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 170dd193cef6..de8c3785899a 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -2018,12 +2018,18 @@ 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;
- }
- }
+ 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;
}
static void atari_floppy_cleanup(void)
--
2.33.0
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 re-use the same free tag call, so we also add a label for
that as well.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
drivers/block/sunvdc.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/block/sunvdc.c b/drivers/block/sunvdc.c
index 4d4bb810c2ae..6f45a53f7cbf 100644
--- a/drivers/block/sunvdc.c
+++ b/drivers/block/sunvdc.c
@@ -826,8 +826,8 @@ static int probe_disk(struct vdc_port *port)
if (IS_ERR(g)) {
printk(KERN_ERR PFX "%s: Could not allocate gendisk.\n",
port->vio.name);
- blk_mq_free_tag_set(&port->tag_set);
- return PTR_ERR(g);
+ err = PTR_ERR(g);
+ goto out_free_tag;
}
port->disk = g;
@@ -879,9 +879,17 @@ static int probe_disk(struct vdc_port *port)
port->vdisk_size, (port->vdisk_size >> (20 - 9)),
port->vio.ver.major, port->vio.ver.minor);
- device_add_disk(&port->vio.vdev->dev, g, NULL);
+ err = device_add_disk(&port->vio.vdev->dev, g, NULL);
+ if (err)
+ goto out_cleanup_disk;
return 0;
+
+out_cleanup_disk:
+ blk_cleanup_disk(g);
+out_free_tag:
+ blk_mq_free_tag_set(&port->tag_set);
+ return err;
}
static struct ldc_channel_config vdc_ldc_cfg = {
--
2.33.0
__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.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
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
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.
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Luis Chamberlain <[email protected]>
---
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
On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
> Jens,
>
> as requested, I've folded all pending changes into this series. This
> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
> modified the ataprobe and floppy driver changes as he suggested.
>
> I think this is it. The world of floppy has been exciting for v5.16.
>
> [...]
Applied, thanks!
[01/14] nvdimm/btt: use goto error labels on btt_blk_init()
commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
[02/14] nvdimm/btt: add error handling support for add_disk()
commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
[03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
[04/14] nvdimm/blk: add error handling support for add_disk()
commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
[05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
commit: accf58afb689f81daadde24080ea1164ad2db75f
[06/14] nvdimm/pmem: use add_disk() error handling
commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
[07/14] z2ram: add error handling support for add_disk()
commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
[08/14] block/sunvdc: add error handling support for add_disk()
commit: f583eaef0af39b792d74e39721b5ba4b6948a270
[09/14] mtd/ubi/block: add error handling support for add_disk()
commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
[10/14] ataflop: remove ataflop_probe_lock mutex
commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
[11/14] block: update __register_blkdev() probe documentation
commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
[12/14] ataflop: address add_disk() error handling on probe
commit: 46a7db492e7a27408bc164cbe6424683e79529b0
[13/14] floppy: address add_disk() error handling on probe
commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
[14/14] block: add __must_check for *add_disk*() callers
commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
Best regards,
--
Jens Axboe
On 11/4/21 5:49 AM, Jens Axboe wrote:
> On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
>> Jens,
>>
>> as requested, I've folded all pending changes into this series. This
>> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
>> modified the ataprobe and floppy driver changes as he suggested.
>>
>> I think this is it. The world of floppy has been exciting for v5.16.
>>
>> [...]
>
> Applied, thanks!
>
> [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
> commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
> [02/14] nvdimm/btt: add error handling support for add_disk()
> commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
> [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
> commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
> [04/14] nvdimm/blk: add error handling support for add_disk()
> commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
> [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
> commit: accf58afb689f81daadde24080ea1164ad2db75f
> [06/14] nvdimm/pmem: use add_disk() error handling
> commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
> [07/14] z2ram: add error handling support for add_disk()
> commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
> [08/14] block/sunvdc: add error handling support for add_disk()
> commit: f583eaef0af39b792d74e39721b5ba4b6948a270
> [09/14] mtd/ubi/block: add error handling support for add_disk()
> commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
> [10/14] ataflop: remove ataflop_probe_lock mutex
> commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
> [11/14] block: update __register_blkdev() probe documentation
> commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
> [12/14] ataflop: address add_disk() error handling on probe
> commit: 46a7db492e7a27408bc164cbe6424683e79529b0
> [13/14] floppy: address add_disk() error handling on probe
> commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
> [14/14] block: add __must_check for *add_disk*() callers
> commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
rivers/scsi/sd.c: In function ‘sd_probe’:
drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
3573 | device_add_disk(dev, gd, NULL);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/sr.c: In function ‘sr_probe’:
drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
731 | device_add_disk(&sdev->sdev_gendev, disk, NULL);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Dropping the last two patches...
--
Jens Axboe
On Thu, Nov 04, 2021 at 06:53:34AM -0600, Jens Axboe wrote:
> On 11/4/21 5:49 AM, Jens Axboe wrote:
> > On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
> >> Jens,
> >>
> >> as requested, I've folded all pending changes into this series. This
> >> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
> >> modified the ataprobe and floppy driver changes as he suggested.
> >>
> >> I think this is it. The world of floppy has been exciting for v5.16.
> >>
> >> [...]
> >
> > Applied, thanks!
> >
> > [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
> > commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
> > [02/14] nvdimm/btt: add error handling support for add_disk()
> > commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
> > [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
> > commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
> > [04/14] nvdimm/blk: add error handling support for add_disk()
> > commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
> > [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
> > commit: accf58afb689f81daadde24080ea1164ad2db75f
> > [06/14] nvdimm/pmem: use add_disk() error handling
> > commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
> > [07/14] z2ram: add error handling support for add_disk()
> > commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
> > [08/14] block/sunvdc: add error handling support for add_disk()
> > commit: f583eaef0af39b792d74e39721b5ba4b6948a270
> > [09/14] mtd/ubi/block: add error handling support for add_disk()
> > commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
> > [10/14] ataflop: remove ataflop_probe_lock mutex
> > commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
> > [11/14] block: update __register_blkdev() probe documentation
> > commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
> > [12/14] ataflop: address add_disk() error handling on probe
> > commit: 46a7db492e7a27408bc164cbe6424683e79529b0
> > [13/14] floppy: address add_disk() error handling on probe
> > commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
> > [14/14] block: add __must_check for *add_disk*() callers
> > commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
>
> rivers/scsi/sd.c: In function ‘sd_probe’:
> drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> 3573 | device_add_disk(dev, gd, NULL);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/scsi/sr.c: In function ‘sr_probe’:
> drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> 731 | device_add_disk(&sdev->sdev_gendev, disk, NULL);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> Dropping the last two patches...
Martin K Peterson has the respective patches needed queued up on his tree
for v5.16:
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=e9d658c2175b95a8f091b12ddefb271683aeacd9
https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=2a7a891f4c406822801ecd676b076c64de072c9e
Would the last patch be sent once that gets to Linus?
Also curious why drop the last two patches instead just the last one for
now?
Luis
On 11/4/21 11:07 AM, Luis Chamberlain wrote:
> On Thu, Nov 04, 2021 at 06:53:34AM -0600, Jens Axboe wrote:
>> On 11/4/21 5:49 AM, Jens Axboe wrote:
>>> On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
>>>> Jens,
>>>>
>>>> as requested, I've folded all pending changes into this series. This
>>>> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
>>>> modified the ataprobe and floppy driver changes as he suggested.
>>>>
>>>> I think this is it. The world of floppy has been exciting for v5.16.
>>>>
>>>> [...]
>>>
>>> Applied, thanks!
>>>
>>> [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
>>> commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
>>> [02/14] nvdimm/btt: add error handling support for add_disk()
>>> commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
>>> [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
>>> commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
>>> [04/14] nvdimm/blk: add error handling support for add_disk()
>>> commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
>>> [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
>>> commit: accf58afb689f81daadde24080ea1164ad2db75f
>>> [06/14] nvdimm/pmem: use add_disk() error handling
>>> commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
>>> [07/14] z2ram: add error handling support for add_disk()
>>> commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
>>> [08/14] block/sunvdc: add error handling support for add_disk()
>>> commit: f583eaef0af39b792d74e39721b5ba4b6948a270
>>> [09/14] mtd/ubi/block: add error handling support for add_disk()
>>> commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
>>> [10/14] ataflop: remove ataflop_probe_lock mutex
>>> commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
>>> [11/14] block: update __register_blkdev() probe documentation
>>> commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
>>> [12/14] ataflop: address add_disk() error handling on probe
>>> commit: 46a7db492e7a27408bc164cbe6424683e79529b0
>>> [13/14] floppy: address add_disk() error handling on probe
>>> commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
>>> [14/14] block: add __must_check for *add_disk*() callers
>>> commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
>>
>> rivers/scsi/sd.c: In function ‘sd_probe’:
>> drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>> 3573 | device_add_disk(dev, gd, NULL);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/scsi/sr.c: In function ‘sr_probe’:
>> drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
>> 731 | device_add_disk(&sdev->sdev_gendev, disk, NULL);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>>
>> Dropping the last two patches...
>
> Martin K Peterson has the respective patches needed queued up on his tree
> for v5.16:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=e9d658c2175b95a8f091b12ddefb271683aeacd9
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=2a7a891f4c406822801ecd676b076c64de072c9e
>
> Would the last patch be sent once that gets to Linus?
But that dependency wasn't clear in the patches posted, and it leaves me
wondering if there are others? I obviously can't queue up a patch that
adds a must_check to a function, when we still have callers that don't
properly check it.
That should have been made clear, and that last patch never should've
been part of the series. Please send it once Linus's tree has all
callers checking the result.
> Also curious why drop the last two patches instead just the last one for
> now?
Sorry, meant just the last one.
--
Jens Axboe
On Thu, Nov 04, 2021 at 11:10:48AM -0600, Jens Axboe wrote:
> On 11/4/21 11:07 AM, Luis Chamberlain wrote:
> > On Thu, Nov 04, 2021 at 06:53:34AM -0600, Jens Axboe wrote:
> >> On 11/4/21 5:49 AM, Jens Axboe wrote:
> >>> On Wed, 3 Nov 2021 16:04:23 -0700, Luis Chamberlain wrote:
> >>>> Jens,
> >>>>
> >>>> as requested, I've folded all pending changes into this series. This
> >>>> v5 pegs on Christoph's reviewed-by tags and since I was respinning I
> >>>> modified the ataprobe and floppy driver changes as he suggested.
> >>>>
> >>>> I think this is it. The world of floppy has been exciting for v5.16.
> >>>>
> >>>> [...]
> >>>
> >>> Applied, thanks!
> >>>
> >>> [01/14] nvdimm/btt: use goto error labels on btt_blk_init()
> >>> commit: 2762ff06aa49e3a13fb4b779120f4f8c12c39fd1
> >>> [02/14] nvdimm/btt: add error handling support for add_disk()
> >>> commit: 16be7974ff5d0a5cd9f345571c3eac1c3f6ba6de
> >>> [03/14] nvdimm/blk: avoid calling del_gendisk() on early failures
> >>> commit: b7421afcec0c77ab58633587ddc29d53e6eb95af
> >>> [04/14] nvdimm/blk: add error handling support for add_disk()
> >>> commit: dc104f4bb2d0a652dee010e47bc89c1ad2ab37c9
> >>> [05/14] nvdimm/pmem: cleanup the disk if pmem_release_disk() is yet assigned
> >>> commit: accf58afb689f81daadde24080ea1164ad2db75f
> >>> [06/14] nvdimm/pmem: use add_disk() error handling
> >>> commit: 5a192ccc32e2981f721343c750b8cfb4c3f41007
> >>> [07/14] z2ram: add error handling support for add_disk()
> >>> commit: 15733754ccf35c49d2f36a7ac51adc8b975c1c78
> >>> [08/14] block/sunvdc: add error handling support for add_disk()
> >>> commit: f583eaef0af39b792d74e39721b5ba4b6948a270
> >>> [09/14] mtd/ubi/block: add error handling support for add_disk()
> >>> commit: ed73919124b2e48490adbbe48ffe885a2a4c6fee
> >>> [10/14] ataflop: remove ataflop_probe_lock mutex
> >>> commit: 4ddb85d36613c45bde00d368bf9f357bd0708a0c
> >>> [11/14] block: update __register_blkdev() probe documentation
> >>> commit: 26e06f5b13671d194d67ae8e2b66f524ab174153
> >>> [12/14] ataflop: address add_disk() error handling on probe
> >>> commit: 46a7db492e7a27408bc164cbe6424683e79529b0
> >>> [13/14] floppy: address add_disk() error handling on probe
> >>> commit: ec28fcc6cfcd418d20038ad2c492e87bf3a9f026
> >>> [14/14] block: add __must_check for *add_disk*() callers
> >>> commit: 1698712d85ec2f128fc7e7c5dc2018b5ed2b7cf6
> >>
> >> rivers/scsi/sd.c: In function ‘sd_probe’:
> >> drivers/scsi/sd.c:3573:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> >> 3573 | device_add_disk(dev, gd, NULL);
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> drivers/scsi/sr.c: In function ‘sr_probe’:
> >> drivers/scsi/sr.c:731:9: warning: ignoring return value of ‘device_add_disk’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> >> 731 | device_add_disk(&sdev->sdev_gendev, disk, NULL);
> >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >>
> >> Dropping the last two patches...
> >
> > Martin K Peterson has the respective patches needed queued up on his tree
> > for v5.16:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=e9d658c2175b95a8f091b12ddefb271683aeacd9
> > https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/commit/?h=5.16/scsi-staging&id=2a7a891f4c406822801ecd676b076c64de072c9e
> >
> > Would the last patch be sent once that gets to Linus?
>
> But that dependency wasn't clear in the patches posted,
Sorry my mistake.
> and it leaves me
> wondering if there are others?
There should not be, becauase at least my patches on top of linux-next
compiles fine as per 0-day builds on tons of configs.
> I obviously can't queue up a patch that
> adds a must_check to a function, when we still have callers that don't
> properly check it.
Absolutely.
> That should have been made clear, and that last patch never should've
> been part of the series. Please send it once Linus's tree has all
> callers checking the result.
Indeed. Will track and will do.
> > Also curious why drop the last two patches instead just the last one for
> > now?
>
> Sorry, meant just the last one.
Ah ok!
Luis