Re: [PATCH v2] block: genhd: don't call probe function with major_names_lock held
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Date: 2021-06-20 13:54:42
Also in:
linux-block, lkml
Subsystem:
block layer, floppy driver, ramdisk ram block device driver, scsi subsystem, software raid (multiple disks) support, the rest · Maintainers:
Jens Axboe, Denis Efremov, "James E.J. Bottomley", "Martin K. Petersen", Song Liu, Yu Kuai, Linus Torvalds
On 2021/06/20 11:44, Hillf Danton wrote:
Good craft in regard to triggering the ABBA deadlock, but curious why not move unregister_blkdev out of and before loop_ctl_mutex, given it will also serialise with the prober.
Well, something like this untested diff? Call unregister_blkdev() as soon as __exit function starts, for calling probe function after cleanup started will be unexpected for __exit function. Keep probe function no-op until __init function ends, for probe function might be called as soon as __register_blkdev() succeeded but calling probe function before setup completes will be unexpected for __init function. drivers/block/ataflop.c | 6 +++++- drivers/block/brd.c | 8 ++++++-- drivers/block/floppy.c | 4 ++++ drivers/block/loop.c | 4 ++-- drivers/ide/ide-probe.c | 8 +++++++- drivers/md/md.c | 5 +++++ drivers/scsi/sd.c | 10 +--------- 7 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index d601e49f80e0..3681e8c493b1 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c@@ -1995,6 +1995,7 @@ static int ataflop_alloc_disk(unsigned int drive, unsigned int type) } static DEFINE_MUTEX(ataflop_probe_lock); +static bool module_initialize_completed; static void ataflop_probe(dev_t dev) {
@@ -2006,6 +2007,8 @@ static void ataflop_probe(dev_t dev) if (drive >= FD_MAX_UNITS || type >= NUM_DISK_MINORS) return; + if (!module_initialize_completed) + return; mutex_lock(&ataflop_probe_lock); if (!unit[drive].disk[type]) { if (ataflop_alloc_disk(drive, type) == 0)
@@ -2080,6 +2083,7 @@ static int __init atari_floppy_init (void) UseTrackbuffer ? "" : "no "); config_types(); + module_initialize_completed = true; return 0; err:
@@ -2138,6 +2142,7 @@ static void __exit atari_floppy_exit(void) { int i, type; + unregister_blkdev(FLOPPY_MAJOR, "fd"); for (i = 0; i < FD_MAX_UNITS; i++) { for (type = 0; type < NUM_DISK_MINORS; type++) { if (!unit[i].disk[type])
@@ -2148,7 +2153,6 @@ static void __exit atari_floppy_exit(void) } blk_mq_free_tag_set(&unit[i].tag_set); } - unregister_blkdev(FLOPPY_MAJOR, "fd"); del_timer_sync(&fd_timer); atari_stram_free( DMABuffer );
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 7562cf30b14e..91a10c938e65 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c@@ -371,6 +371,7 @@ __setup("ramdisk_size=", ramdisk_size); static LIST_HEAD(brd_devices); static DEFINE_MUTEX(brd_devices_mutex); static struct dentry *brd_debugfs_dir; +static bool module_initialize_completed; static struct brd_device *brd_alloc(int i) {
@@ -439,6 +440,8 @@ static void brd_probe(dev_t dev) struct brd_device *brd; int i = MINOR(dev) / max_part; + if (!module_initialize_completed) + return; mutex_lock(&brd_devices_mutex); list_for_each_entry(brd, &brd_devices, brd_list) { if (brd->brd_number == i)
@@ -530,6 +533,7 @@ static int __init brd_init(void) mutex_unlock(&brd_devices_mutex); pr_info("brd: module loaded\n"); + module_initialize_completed = true; return 0; out_free:
@@ -550,13 +554,13 @@ static void __exit brd_exit(void) { struct brd_device *brd, *next; + unregister_blkdev(RAMDISK_MAJOR, "ramdisk"); + debugfs_remove_recursive(brd_debugfs_dir); list_for_each_entry_safe(brd, next, &brd_devices, brd_list) brd_del_one(brd); - unregister_blkdev(RAMDISK_MAJOR, "ramdisk"); - pr_info("brd: module unloaded\n"); }
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 8a9d22207c59..37b8e53c183d 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c@@ -4523,6 +4523,7 @@ static int floppy_alloc_disk(unsigned int drive, unsigned int type) } static DEFINE_MUTEX(floppy_probe_lock); +static bool module_initialize_completed; static void floppy_probe(dev_t dev) {
@@ -4533,6 +4534,8 @@ static void floppy_probe(dev_t dev) type >= ARRAY_SIZE(floppy_type)) return; + if (!module_initialize_completed) + return; mutex_lock(&floppy_probe_lock); if (!disks[drive][type]) { if (floppy_alloc_disk(drive, type) == 0)
@@ -4705,6 +4708,7 @@ static int __init do_floppy_init(void) NULL); } + module_initialize_completed = true; return 0; out_remove_drives:
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 76e12f3482a9..08aef61ab791 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c@@ -2386,13 +2386,13 @@ static int loop_exit_cb(int id, void *ptr, void *data) static void __exit loop_exit(void) { + unregister_blkdev(LOOP_MAJOR, "loop"); + mutex_lock(&loop_ctl_mutex); idr_for_each(&loop_index_idr, &loop_exit_cb, NULL); idr_destroy(&loop_index_idr); - unregister_blkdev(LOOP_MAJOR, "loop"); - misc_deregister(&loop_misc); mutex_unlock(&loop_ctl_mutex);
diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index aefd74c0d862..8c71356cdcfe 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c@@ -40,6 +40,8 @@ #include <linux/uaccess.h> #include <asm/io.h> +static bool module_initialize_completed; + /** * generic_id - add a generic drive id * @drive: drive to make an ID block for
@@ -904,6 +906,8 @@ static int init_irq (ide_hwif_t *hwif) static void ata_probe(dev_t dev) { + if (!module_initialize_completed) + return; request_module("ide-disk"); request_module("ide-cd"); request_module("ide-tape");
@@ -1475,6 +1479,8 @@ int ide_host_register(struct ide_host *host, const struct ide_port_info *d, } } + if (j) + module_initialize_completed = true; return j ? 0 : -1; } EXPORT_SYMBOL_GPL(ide_host_register);
@@ -1539,6 +1545,7 @@ EXPORT_SYMBOL_GPL(ide_port_unregister_devices); static void ide_unregister(ide_hwif_t *hwif) { + unregister_blkdev(hwif->major, hwif->name); mutex_lock(&ide_cfg_mtx); if (hwif->present) {
@@ -1559,7 +1566,6 @@ static void ide_unregister(ide_hwif_t *hwif) * Remove us from the kernel's knowledge */ kfree(hwif->sg_table); - unregister_blkdev(hwif->major, hwif->name); ide_release_dma_engine(hwif);
diff --git a/drivers/md/md.c b/drivers/md/md.c
index 49f897fbb89b..6603900062bc 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c@@ -68,6 +68,8 @@ #include "md-bitmap.h" #include "md-cluster.h" +static bool module_initialize_completed; + /* pers_list is a list of registered personalities protected * by pers_lock. * pers_lock does extra service to protect accesses to
@@ -5776,6 +5778,8 @@ static void md_probe(dev_t dev) { if (MAJOR(dev) == MD_MAJOR && MINOR(dev) >= 512) return; + if (!module_initialize_completed) + return; if (create_on_open) md_alloc(dev, NULL); }
@@ -9590,6 +9594,7 @@ static int __init md_init(void) raid_table_header = register_sysctl_table(raid_root_table); md_geninit(); + module_initialize_completed = true; return 0; err_mdp:
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index cb3c37d1e009..4fc8f4db2ccf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c@@ -629,14 +629,6 @@ static struct scsi_driver sd_template = { .eh_reset = sd_eh_reset, }; -/* - * Don't request a new module, as that could deadlock in multipath - * environment. - */ -static void sd_default_probe(dev_t devt) -{ -} - /* * Device no to disk mapping: *
@@ -3715,7 +3707,7 @@ static int __init init_sd(void) SCSI_LOG_HLQUEUE(3, printk("init_sd: sd driver entry point\n")); for (i = 0; i < SD_MAJORS; i++) { - if (__register_blkdev(sd_major(i), "sd", sd_default_probe)) + if (register_blkdev(sd_major(i), "sd")) continue; majors++; }
_______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees