| Message ID | 20210904013932.3182778-2-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: 7th -- last batch of add_disk() error handling conversions | expand |
On 2021/09/04 10:39, Luis Chamberlain wrote: > 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; Since e.g. loop_add() from loop_probe() returns -EEXIST when /dev/loop$num already exists (e.g. raced with ioctl(LOOP_CTL_ADD)), isn't unconditionally failing an over-failing? > inode = ilookup(blockdev_superblock, dev); > if (!inode) > return NULL; By the way, Jens, will you pick up https://lkml.kernel.org/r/adb1e792-fc0e-ee81-7ea0-0906fc36419d@i-love.sakura.ne.jp before these "add error handling" changes?
On 9/3/21 8:49 PM, Tetsuo Handa wrote: > By the way, Jens, will you pick up > https://lkml.kernel.org/r/adb1e792-fc0e-ee81-7ea0-0906fc36419d@i-love.sakura.ne.jp > before these "add error handling" changes? Yes, that one is 5.15 material, the rest of the error handling changes are not. Hence the ordering follows naturally from that.
On Sat, Sep 04, 2021 at 11:49:06AM +0900, Tetsuo Handa wrote: > On 2021/09/04 10:39, Luis Chamberlain wrote: > > 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; > > Since e.g. loop_add() from loop_probe() returns -EEXIST when /dev/loop$num already > exists (e.g. raced with ioctl(LOOP_CTL_ADD)), isn't unconditionally failing an over-failing? It's not clear to me how. What do we loose by capturing the failure on blk_request_module()? Luis
On 2021/09/06 3:11, Luis Chamberlain wrote: > On Sat, Sep 04, 2021 at 11:49:06AM +0900, Tetsuo Handa wrote: >> On 2021/09/04 10:39, Luis Chamberlain wrote: >>> 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; >> >> Since e.g. loop_add() from loop_probe() returns -EEXIST when /dev/loop$num already >> exists (e.g. raced with ioctl(LOOP_CTL_ADD)), isn't unconditionally failing an over-failing? > > It's not clear to me how. What do we loose by capturing the failure on > blk_request_module()? > We loose ability to handle concurrent request. diff --git a/fs/block_dev.c b/fs/block_dev.c index 45df6cbccf12..9559c8aabc88 100644 --- a/fs/block_dev.c +++ b/fs/block_dev.c @@ -36,6 +36,7 @@ #include <linux/suspend.h> #include "internal.h" #include "../block/blk.h" +#include <linux/debug_locks.h> struct bdev_inode { struct block_device bdev; @@ -1147,7 +1148,11 @@ struct block_device *blkdev_get_no_open(dev_t dev) inode = ilookup(blockdev_superblock, dev); if (!inode) { + dump_stack(); + debug_show_held_locks(current); blk_request_module(dev); + if (!strcmp(current->comm, "cat")) + schedule_timeout_killable(10 * HZ); inode = ilookup(blockdev_superblock, dev); if (!inode) return NULL; Try "mknod /dev/loop$num b 7 $num" followed by concurrent "cat /dev/loop$num" requests. [ 535.847823] [ T5261] CPU: 1 PID: 5261 Comm: cat Tainted: G E 5.14.0-next-20210903+ #3 [ 535.851419] [ T5261] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020 [ 535.855563] [ T5261] Call Trace: [ 535.856746] [ T5261] dump_stack_lvl+0x79/0xbf [ 535.858332] [ T5261] blkdev_get_no_open+0x86/0xe0 [ 535.860036] [ T5261] blkdev_get_by_dev+0x5f/0x540 [ 535.861747] [ T5261] ? block_ioctl+0x40/0x40 [ 535.863305] [ T5261] blkdev_open+0x58/0x90 [ 535.864865] [ T5261] do_dentry_open+0x144/0x3a0 [ 535.866538] [ T5261] path_openat+0xa57/0xda0 [ 535.868139] [ T5261] do_filp_open+0x9f/0x140 [ 535.869933] [ T5261] ? _raw_spin_unlock+0x1a/0x30 [ 535.871570] [ T5261] do_sys_openat2+0x71/0x150 [ 535.873195] [ T5261] __x64_sys_openat+0x78/0xa0 [ 535.874740] [ T5261] do_syscall_64+0x3d/0xb0 [ 535.876234] [ T5261] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 535.878428] [ T5261] RIP: 0033:0x7f6b997d28db [ 535.879927] [ T5261] Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 4c 24 28 64 48 2b 0c 25 [ 535.886778] [ T5261] RSP: 002b:00007ffe8bdad880 EFLAGS: 00000246 ORIG_RAX: 0000000000000101 [ 535.889727] [ T5261] RAX: ffffffffffffffda RBX: 000055f741e5b634 RCX: 00007f6b997d28db [ 535.892466] [ T5261] RDX: 0000000000000000 RSI: 00007ffe8bdaf764 RDI: 00000000ffffff9c [ 535.895226] [ T5261] RBP: 00007ffe8bdaf764 R08: 0000000000000001 R09: 0000000000000000 [ 535.898162] [ T5261] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [ 535.900858] [ T5261] R13: 00007ffe8bdadb68 R14: 0000000000000000 R15: 0000000000020000 [ 535.903752] [ T5262] CPU: 2 PID: 5262 Comm: cat Tainted: G E 5.14.0-next-20210903+ #3 [ 535.904759] [ T5261] no locks held by cat/5261. [ 535.906905] [ T5262] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020 [ 535.906908] [ T5262] Call Trace: [ 535.906912] [ T5262] dump_stack_lvl+0x79/0xbf [ 535.915279] [ T5262] blkdev_get_no_open+0x86/0xe0 [ 535.916942] [ T5262] blkdev_get_by_dev+0x5f/0x540 [ 535.918608] [ T5262] ? block_ioctl+0x40/0x40 [ 535.920250] [ T5262] blkdev_open+0x58/0x90 [ 535.921696] [ T5262] do_dentry_open+0x144/0x3a0 [ 535.923291] [ T5262] path_openat+0xa57/0xda0 [ 535.924816] [ T5262] do_filp_open+0x9f/0x140 [ 535.926359] [ T5262] ? _raw_spin_unlock+0x1a/0x30 [ 535.928030] [ T5262] do_sys_openat2+0x71/0x150 [ 535.929600] [ T5262] __x64_sys_openat+0x78/0xa0 [ 535.931215] [ T5262] do_syscall_64+0x3d/0xb0 [ 535.932768] [ T5262] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 535.934817] [ T5262] RIP: 0033:0x7efe3a0aa8db [ 535.936354] [ T5262] Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 4c 24 28 64 48 2b 0c 25 [ 535.943021] [ T5262] RSP: 002b:00007ffe1697dfb0 EFLAGS: 00000246 ORIG_RAX: 0000000000000101 [ 535.945883] [ T5262] RAX: ffffffffffffffda RBX: 000055982a4e1634 RCX: 00007efe3a0aa8db [ 535.948580] [ T5262] RDX: 0000000000000000 RSI: 00007ffe1697e764 RDI: 00000000ffffff9c [ 535.951298] [ T5262] RBP: 00007ffe1697e764 R08: 0000000000000001 R09: 0000000000000000 [ 535.954064] [ T5262] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [ 535.956789] [ T5262] R13: 00007ffe1697e298 R14: 0000000000000000 R15: 0000000000020000 [ 535.959522] [ T5263] CPU: 0 PID: 5263 Comm: cat Tainted: G E 5.14.0-next-20210903+ #3 [ 535.962995] [ T5263] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 02/27/2020 [ 535.965787] [ T5262] no locks held by cat/5262. [ 535.966998] [ T5263] Call Trace: [ 535.970068] [ T5263] dump_stack_lvl+0x79/0xbf [ 535.971572] [ T5263] blkdev_get_no_open+0x86/0xe0 [ 535.973199] [ T5263] blkdev_get_by_dev+0x5f/0x540 [ 535.974893] [ T5263] ? block_ioctl+0x40/0x40 [ 535.976364] [ T5263] blkdev_open+0x58/0x90 [ 535.977781] [ T5263] do_dentry_open+0x144/0x3a0 [ 535.979378] [ T5263] path_openat+0xa57/0xda0 [ 535.980882] [ T5263] do_filp_open+0x9f/0x140 [ 535.982333] [ T5263] ? _raw_spin_unlock+0x1a/0x30 [ 535.984002] [ T5263] do_sys_openat2+0x71/0x150 [ 535.985554] [ T5263] __x64_sys_openat+0x78/0xa0 [ 535.987232] [ T5263] do_syscall_64+0x3d/0xb0 [ 535.988790] [ T5263] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 535.990804] [ T5263] RIP: 0033:0x7ff84ab408db [ 535.992275] [ T5263] Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 4c 24 28 64 48 2b 0c 25 [ 535.998888] [ T5263] RSP: 002b:00007ffda9b89a70 EFLAGS: 00000246 ORIG_RAX: 0000000000000101 [ 536.001628] [ T5263] RAX: ffffffffffffffda RBX: 000055626f376634 RCX: 00007ff84ab408db [ 536.004350] [ T5263] RDX: 0000000000000000 RSI: 00007ffda9b8a764 RDI: 00000000ffffff9c [ 536.007009] [ T5263] RBP: 00007ffda9b8a764 R08: 0000000000000001 R09: 0000000000000000 [ 536.009753] [ T5263] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 [ 536.012493] [ T5263] R13: 00007ffda9b89d58 R14: 0000000000000000 R15: 0000000000020000 [ 536.015854] [ T5263] no locks held by cat/5263. If blk_request_module() does not ignore -EEXIST error, only the first open() request would succeed because loop_add() returns 0 and all other concurrent requests would fail because loop_add() returns -EEXIST. Actually, blk_request_module() failures should be ignored, for subsequent ilookup() will fail if blk_request_module() failed to create the requested block device.
On Mon, Sep 06, 2021 at 02:59:38PM +0900, Tetsuo Handa wrote: > On 2021/09/06 3:11, Luis Chamberlain wrote: > > On Sat, Sep 04, 2021 at 11:49:06AM +0900, Tetsuo Handa wrote: > >> On 2021/09/04 10:39, Luis Chamberlain wrote: > >>> 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; > >> > >> Since e.g. loop_add() from loop_probe() returns -EEXIST when /dev/loop$num already > >> exists (e.g. raced with ioctl(LOOP_CTL_ADD)), isn't unconditionally failing an over-failing? > > > > It's not clear to me how. What do we loose by capturing the failure on > > blk_request_module()? > > > > We loose ability to handle concurrent request. > If blk_request_module() does not ignore -EEXIST error, only the first > open() request would succeed because loop_add() returns 0 and all > other concurrent requests would fail because loop_add() returns > -EEXIST. Yes I see that now thanks! > Actually, blk_request_module() failures should be ignored, for > subsequent ilookup() will fail if blk_request_module() failed to > create the requested block device. Then how about this: Since we would like to use __must_check for add_disk() we proceed with the change to capture the errors and propagate them and we just document on fs/block_dev.c's use of blk_request_module() about the above issue and how we prefer the errror that ilookup() would return. Luis
On 2021/09/07 23:57, Luis Chamberlain wrote: >> Actually, blk_request_module() failures should be ignored, for >> subsequent ilookup() will fail if blk_request_module() failed to >> create the requested block device. > > Then how about this: > > Since we would like to use __must_check for add_disk() we proceed with > the change to capture the errors and propagate them and we just document on > fs/block_dev.c's use of blk_request_module() about the above issue and > how we prefer the errror that ilookup() would return. Marking add_disk() as __must_check makes it possible to enforce "don't leave partially initialized devices". That's already an enough improvement. Probe functions can remain "void", and hence blk_request_module() can remain "void". That is, I would drop "[PATCH 1/2] block: make __register_blkdev() return an error".
On Wed, Sep 08, 2021 at 12:23:02AM +0900, Tetsuo Handa wrote: > On 2021/09/07 23:57, Luis Chamberlain wrote: > >> Actually, blk_request_module() failures should be ignored, for > >> subsequent ilookup() will fail if blk_request_module() failed to > >> create the requested block device. > > > > Then how about this: > > > > Since we would like to use __must_check for add_disk() we proceed with > > the change to capture the errors and propagate them and we just document on > > fs/block_dev.c's use of blk_request_module() about the above issue and > > how we prefer the errror that ilookup() would return. > > Marking add_disk() as __must_check makes it possible to enforce "don't leave > partially initialized devices". That's already an enough improvement. > > Probe functions can remain "void", and hence blk_request_module() can remain "void". > That is, I would drop "[PATCH 1/2] block: make __register_blkdev() return an error". Probe calls can be left voide, but because of the new __must_check we'd still have to modify all probe calls as they use add_disk() and it would seem odd to just capture the error to ignore it without documenting any of this. Luis
On Tue, Sep 07, 2021 at 08:28:06AM -0700, Luis Chamberlain wrote: > On Wed, Sep 08, 2021 at 12:23:02AM +0900, Tetsuo Handa wrote: > > On 2021/09/07 23:57, Luis Chamberlain wrote: > > >> Actually, blk_request_module() failures should be ignored, for > > >> subsequent ilookup() will fail if blk_request_module() failed to > > >> create the requested block device. > > > > > > Then how about this: > > > > > > Since we would like to use __must_check for add_disk() we proceed with > > > the change to capture the errors and propagate them and we just document on > > > fs/block_dev.c's use of blk_request_module() about the above issue and > > > how we prefer the errror that ilookup() would return. > > > > Marking add_disk() as __must_check makes it possible to enforce "don't leave > > partially initialized devices". That's already an enough improvement. > > > > Probe functions can remain "void", and hence blk_request_module() can remain "void". > > That is, I would drop "[PATCH 1/2] block: make __register_blkdev() return an error". > > Probe calls can be left voide, but because of the new __must_check we'd > still have to modify all probe calls as they use add_disk() and it would > seem odd to just capture the error to ignore it without documenting > any of this. So indeed, all probe() routines would need to be modified anyway because of the added final __must_check. Because of this I still think that if we want to *ignore* the error, we should just document and ignore it on the caller side. That is, still modify __register_blkdev() to propagate the error, and the caller can decide to ignore or not. In this case we can document on fs/block_dev.c *why* we are ignoring the error. Thoughts? Luis
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 */
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(-)