* [PATCH 0/2] block: 7th -- last batch of add_disk() error handling conversions
@ 2021-09-04 1:39 Luis Chamberlain
2021-09-04 1:39 ` [PATCH 1/2] block: make __register_blkdev() return an error Luis Chamberlain
2021-09-04 1:39 ` [PATCH 2/2] block: add __must_check for *add_disk*() callers Luis Chamberlain
0 siblings, 2 replies; 3+ messages in thread
From: Luis Chamberlain @ 2021-09-04 1:39 UTC (permalink / raw)
To: axboe, hch, efremov, song, jejb, martin.petersen, viro, hare,
jack, ming.lei, tj
Cc: linux-raid, linux-scsi, linux-fsdevel, linux-block, linux-kernel,
Luis Chamberlain
This is the 7th and final set of changes to convert all drivers over to
use and finally enforce add_disk() error handling. This set deals with
those old drivers using __register_blkdev() which in turn have a
respective add_disk() call made. The last patch ensures we won't
add new drivers without a check for the add_disk() call.
You can find the full set of patches on my kernel.org linux
20210901-for-axboe-add-disk-error-handling branch [0] which is now 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 (2):
block: make __register_blkdev() return an error
block: add __must_check for *add_disk*() callers
block/genhd.c | 21 ++++++++++++---------
drivers/block/ataflop.c | 20 +++++++++++++++-----
drivers/block/brd.c | 7 +++++--
drivers/block/floppy.c | 14 ++++++++++----
drivers/block/loop.c | 6 +++---
drivers/md/md.c | 10 +++++++---
drivers/scsi/sd.c | 3 ++-
fs/block_dev.c | 5 ++++-
include/linux/genhd.h | 10 +++++-----
9 files changed, 63 insertions(+), 33 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/2] block: make __register_blkdev() return an error
2021-09-04 1:39 [PATCH 0/2] block: 7th -- last batch of add_disk() error handling conversions Luis Chamberlain
@ 2021-09-04 1:39 ` Luis Chamberlain
2021-09-04 1:39 ` [PATCH 2/2] block: add __must_check for *add_disk*() callers Luis Chamberlain
1 sibling, 0 replies; 3+ messages in thread
From: Luis Chamberlain @ 2021-09-04 1:39 UTC (permalink / raw)
To: axboe, hch, efremov, song, jejb, martin.petersen, viro, hare,
jack, ming.lei, tj
Cc: linux-raid, linux-scsi, linux-fsdevel, linux-block, linux-kernel,
Luis Chamberlain
This makes __register_blkdev() return an error, and
also changes the probe() call to return an error as well.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
block/genhd.c | 15 +++++++++------
drivers/block/ataflop.c | 20 +++++++++++++++-----
drivers/block/brd.c | 7 +++++--
drivers/block/floppy.c | 14 ++++++++++----
drivers/block/loop.c | 6 +++---
drivers/md/md.c | 10 +++++++---
drivers/scsi/sd.c | 3 ++-
fs/block_dev.c | 5 ++++-
include/linux/genhd.h | 4 ++--
9 files changed, 57 insertions(+), 27 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index 567549a011d1..c4836296a974 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -180,7 +180,7 @@ static struct blk_major_name {
struct blk_major_name *next;
int major;
char name[16];
- void (*probe)(dev_t devt);
+ int (*probe)(dev_t devt);
} *major_names[BLKDEV_MAJOR_HASH_SIZE];
static DEFINE_MUTEX(major_names_lock);
@@ -209,7 +209,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
* @major: the requested major device number [1..BLKDEV_MAJOR_MAX-1]. If
* @major = 0, try to allocate any unused major number.
* @name: the name of the new block device as a zero terminated string
- * @probe: allback that is called on access to any minor number of @major
+ * @probe: callback that is called on access to any minor number of @major
*
* The @name must be unique within the system.
*
@@ -227,7 +227,7 @@ void blkdev_show(struct seq_file *seqf, off_t offset)
* Use register_blkdev instead for any new code.
*/
int __register_blkdev(unsigned int major, const char *name,
- void (*probe)(dev_t devt))
+ int (*probe)(dev_t devt))
{
struct blk_major_name **n, *p;
int index, ret = 0;
@@ -621,17 +621,18 @@ static ssize_t disk_badblocks_store(struct device *dev,
return badblocks_store(disk->bb, page, len, 0);
}
-void blk_request_module(dev_t devt)
+int blk_request_module(dev_t devt)
{
unsigned int major = MAJOR(devt);
struct blk_major_name **n;
+ int err;
mutex_lock(&major_names_lock);
for (n = &major_names[major_to_index(major)]; *n; n = &(*n)->next) {
if ((*n)->major == major && (*n)->probe) {
- (*n)->probe(devt);
+ err = (*n)->probe(devt);
mutex_unlock(&major_names_lock);
- return;
+ return err;
}
}
mutex_unlock(&major_names_lock);
@@ -639,6 +640,8 @@ void blk_request_module(dev_t devt)
if (request_module("block-major-%d-%d", MAJOR(devt), MINOR(devt)) > 0)
/* Make old-style 2.4 aliases work */
request_module("block-major-%d", MAJOR(devt));
+
+ return 0;
}
/*
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 5dc9b3d32415..e55824e27188 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -1989,24 +1989,34 @@ static int ataflop_alloc_disk(unsigned int drive, unsigned int type)
static DEFINE_MUTEX(ataflop_probe_lock);
-static void ataflop_probe(dev_t dev)
+static int ataflop_probe(dev_t dev)
{
int drive = MINOR(dev) & 3;
int type = MINOR(dev) >> 2;
+ int err = -ENODEV;
if (type)
type--;
- if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS)
- return;
+ if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) {
+ err = -EINVAL;
+ goto out;
+ }
+
mutex_lock(&ataflop_probe_lock);
if (!unit[drive].disk[type]) {
- if (ataflop_alloc_disk(drive, type) == 0) {
- add_disk(unit[drive].disk[type]);
+ err = ataflop_alloc_disk(drive, type);
+ if (err == 0) {
+ err = add_disk(unit[drive].disk[type]);
+ if (err)
+ blk_cleanup_disk(unit[drive].disk[type]);
unit[drive].registered[type] = true;
}
}
mutex_unlock(&ataflop_probe_lock);
+
+out:
+ return err;
}
static void atari_cleanup_floppy_disk(struct atari_floppy_struct *fs)
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index c2bf4946f4e3..82a93044de95 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -426,10 +426,11 @@ static int brd_alloc(int i)
return err;
}
-static void brd_probe(dev_t dev)
+static int brd_probe(dev_t dev)
{
int i = MINOR(dev) / max_part;
struct brd_device *brd;
+ int err = 0;
mutex_lock(&brd_devices_mutex);
list_for_each_entry(brd, &brd_devices, brd_list) {
@@ -437,9 +438,11 @@ static void brd_probe(dev_t dev)
goto out_unlock;
}
- brd_alloc(i);
+ err = brd_alloc(i);
out_unlock:
mutex_unlock(&brd_devices_mutex);
+
+ return err;
}
static void brd_del_one(struct brd_device *brd)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 0434f28742e7..5ecffc7f6e22 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4517,21 +4517,27 @@ static int floppy_alloc_disk(unsigned int drive, unsigned int type)
static DEFINE_MUTEX(floppy_probe_lock);
-static void floppy_probe(dev_t dev)
+static int floppy_probe(dev_t dev)
{
unsigned int drive = (MINOR(dev) & 3) | ((MINOR(dev) & 0x80) >> 5);
unsigned int type = (MINOR(dev) >> 2) & 0x1f;
+ int err = -ENODEV;
if (drive >= N_DRIVE || !floppy_available(drive) ||
type >= ARRAY_SIZE(floppy_type))
- return;
+ return -EINVAL;
mutex_lock(&floppy_probe_lock);
if (!disks[drive][type]) {
- if (floppy_alloc_disk(drive, type) == 0)
- add_disk(disks[drive][type]);
+ if (floppy_alloc_disk(drive, type) == 0) {
+ err = add_disk(disks[drive][type]);
+ if (err)
+ blk_cleanup_disk(disks[drive][type]);
+ }
}
mutex_unlock(&floppy_probe_lock);
+
+ return err;
}
static int __init do_floppy_init(void)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index b8b9e2349e77..6bc1c075741e 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2425,13 +2425,13 @@ static void loop_remove(struct loop_device *lo)
kfree(lo);
}
-static void loop_probe(dev_t dev)
+static int loop_probe(dev_t dev)
{
int idx = MINOR(dev) >> part_shift;
if (max_loop && idx >= max_loop)
- return;
- loop_add(idx);
+ return -EINVAL;
+ return loop_add(idx);
}
static int loop_control_remove(int idx)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 5c0d3536d7c7..fc2790979686 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -5735,12 +5735,16 @@ static int md_alloc(dev_t dev, char *name)
return error;
}
-static void md_probe(dev_t dev)
+static int md_probe(dev_t dev)
{
+ int error = 0;
+
if (MAJOR(dev) == MD_MAJOR && MINOR(dev) >= 512)
- return;
+ return -EINVAL;
if (create_on_open)
- md_alloc(dev, NULL);
+ error = md_alloc(dev, NULL);
+
+ return error;
}
static int add_named_array(const char *val, const struct kernel_param *kp)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 8c1273fff23e..50872c6ce030 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -630,8 +630,9 @@ static struct scsi_driver sd_template = {
* Don't request a new module, as that could deadlock in multipath
* environment.
*/
-static void sd_default_probe(dev_t devt)
+static int sd_default_probe(dev_t devt)
{
+ return 0;
}
/*
diff --git a/fs/block_dev.c b/fs/block_dev.c
index 45df6cbccf12..81a4738910a8 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -1144,10 +1144,13 @@ struct block_device *blkdev_get_no_open(dev_t dev)
{
struct block_device *bdev;
struct inode *inode;
+ int ret;
inode = ilookup(blockdev_superblock, dev);
if (!inode) {
- blk_request_module(dev);
+ ret = blk_request_module(dev);
+ if (ret)
+ return NULL;
inode = ilookup(blockdev_superblock, dev);
if (!inode)
return NULL;
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index c68d83c87f83..5828ecda5c49 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -281,7 +281,7 @@ struct gendisk *__blk_alloc_disk(int node, struct lock_class_key *lkclass);
void blk_cleanup_disk(struct gendisk *disk);
int __register_blkdev(unsigned int major, const char *name,
- void (*probe)(dev_t devt));
+ int (*probe)(dev_t devt));
#define register_blkdev(major, name) \
__register_blkdev(major, name, NULL)
void unregister_blkdev(unsigned int major, const char *name);
@@ -317,7 +317,7 @@ static inline int bd_register_pending_holders(struct gendisk *disk)
dev_t part_devt(struct gendisk *disk, u8 partno);
void inc_diskseq(struct gendisk *disk);
dev_t blk_lookup_devt(const char *name, int partno);
-void blk_request_module(dev_t devt);
+int blk_request_module(dev_t devt);
#ifdef CONFIG_BLOCK
void printk_all_partitions(void);
#else /* CONFIG_BLOCK */
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/2] block: add __must_check for *add_disk*() callers
2021-09-04 1:39 [PATCH 0/2] block: 7th -- last batch of add_disk() error handling conversions Luis Chamberlain
2021-09-04 1:39 ` [PATCH 1/2] block: make __register_blkdev() return an error Luis Chamberlain
@ 2021-09-04 1:39 ` Luis Chamberlain
1 sibling, 0 replies; 3+ messages in thread
From: Luis Chamberlain @ 2021-09-04 1:39 UTC (permalink / raw)
To: axboe, hch, efremov, song, jejb, martin.petersen, viro, hare,
jack, ming.lei, tj
Cc: linux-raid, linux-scsi, linux-fsdevel, linux-block, linux-kernel,
Luis Chamberlain
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 c4836296a974..4c80e6c3d8dd 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -383,8 +383,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);
@@ -529,7 +529,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 5828ecda5c49..8d78d36c424e 100644
--- a/include/linux/genhd.h
+++ b/include/linux/genhd.h
@@ -214,9 +214,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.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-09-04 1:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-04 1:39 [PATCH 0/2] block: 7th -- last batch of add_disk() error handling conversions Luis Chamberlain
2021-09-04 1:39 ` [PATCH 1/2] block: make __register_blkdev() return an error Luis Chamberlain
2021-09-04 1:39 ` [PATCH 2/2] block: add __must_check for *add_disk*() callers 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).