| Message ID | 20220104071647.164918-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] block: deprecate autoloading based on dev_t | expand |
On Tue, Jan 04, 2022 at 08:16:47AM +0100, Christoph Hellwig wrote: > Make the legacy dev_t based autoloading optional and add a deprecation > warning. This kind of autoloading has ceased to be useful about 20 years > ago. ping?
On 1/17/22 1:43 AM, Christoph Hellwig wrote: > On Tue, Jan 04, 2022 at 08:16:47AM +0100, Christoph Hellwig wrote: >> Make the legacy dev_t based autoloading optional and add a deprecation >> warning. This kind of autoloading has ceased to be useful about 20 years >> ago. > > ping? Looks fine to me, just slightly deferred until the 5.18 block branch opens up.
On Tue, 4 Jan 2022 08:16:47 +0100, Christoph Hellwig wrote: > Make the legacy dev_t based autoloading optional and add a deprecation > warning. This kind of autoloading has ceased to be useful about 20 years > ago. > > Applied, thanks! [1/1] block: deprecate autoloading based on dev_t commit: cee57556d6b7521f584f6dd989546ea17d170346 Best regards,
I believe this change may break some mdraid setups. It looks to me like mdadm still depends on this legacy autoload behavior. There are some situations where mdadm does mknod to create a temporary device node and then attempts to open that temporary node. On my system, after disabling legacy autoloading, mdadm fails (ENODEV) when it attempts the open of the temporary node (block 9:127). It seems this is an issue when mdadm attempts to assemble an array that it sees as "foreign". And it makes the determination of whether an array is foreign or not based on whether the hostname recorded in the array superblock(s) matches the current system's hostname. It's my observation that some init systems do not set the system hostname early enough, so arrays that should look local will be treated as foreign arrays by mdadm, which makes it go down this path where it will depend on the legacy autoloading behavior. As an aside, this happens even though udev is in use. mdadm ships with udev rules that cause mdadm to be invoked to assemble arrays when uevents fire that announce the appearance of "file systems" of type linux_raid_member. So while udev is supposed to supplant this legacy autoloading behavior, it appears there are at least some current real-world cases where udev itself ultimately ends up itself depending (in an indirect way) on this legacy behavior :) Cheers, -- Dan
diff --git a/block/Kconfig b/block/Kconfig index d5d4197b7ed2d..205f8d01c6952 100644 --- a/block/Kconfig +++ b/block/Kconfig @@ -26,6 +26,18 @@ menuconfig BLOCK if BLOCK +config BLOCK_LEGACY_AUTOLOAD + bool "Legacy autoloading support" + help + Enable loading modules and creating block device instances based on + accesses through their device special file. This is a historic Linux + feature and makes no sense in a udev world where device files are + created on demand. + + Say N here unless booting or other functionality broke without it, in + which case you should also send a report to your distribution and + linux-block@vger.kernel.org. + config BLK_RQ_ALLOC_TIME bool diff --git a/block/bdev.c b/block/bdev.c index 8bf93a19041b7..51e41b2da2d9f 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -738,12 +738,15 @@ struct block_device *blkdev_get_no_open(dev_t dev) struct inode *inode; inode = ilookup(blockdev_superblock, dev); - if (!inode) { + if (!inode && IS_ENABLED(CONFIG_BLOCK_LEGACY_AUTOLOAD)) { blk_request_module(dev); inode = ilookup(blockdev_superblock, dev); - if (!inode) - return NULL; + if (inode) + pr_warn_ratelimited( +"block device autoloading is deprecated. It will be removed in Linux 5.19\n"); } + if (!inode) + return NULL; /* switch from the inode reference to a device mode one: */ bdev = &BDEV_I(inode)->bdev; diff --git a/block/genhd.c b/block/genhd.c index 626c8406f21a6..6ae990ff02660 100644 --- a/block/genhd.c +++ b/block/genhd.c @@ -185,7 +185,9 @@ static struct blk_major_name { struct blk_major_name *next; int major; char name[16]; +#ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD void (*probe)(dev_t devt); +#endif } *major_names[BLKDEV_MAJOR_HASH_SIZE]; static DEFINE_MUTEX(major_names_lock); static DEFINE_SPINLOCK(major_names_spinlock); @@ -275,7 +277,9 @@ int __register_blkdev(unsigned int major, const char *name, } p->major = major; +#ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD p->probe = probe; +#endif strlcpy(p->name, name, sizeof(p->name)); p->next = NULL; index = major_to_index(major); @@ -679,6 +683,7 @@ static ssize_t disk_badblocks_store(struct device *dev, return badblocks_store(disk->bb, page, len, 0); } +#ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD void blk_request_module(dev_t devt) { unsigned int major = MAJOR(devt); @@ -698,6 +703,7 @@ void blk_request_module(dev_t devt) /* Make old-style 2.4 aliases work */ request_module("block-major-%d", MAJOR(devt)); } +#endif /* CONFIG_BLOCK_LEGACY_AUTOLOAD */ /* * print a full list of all partitions - intended for places where the root
Make the legacy dev_t based autoloading optional and add a deprecation warning. This kind of autoloading has ceased to be useful about 20 years ago. Signed-off-by: Christoph Hellwig <hch@lst.de> --- Changes since v1: - improve the Kconfig help text and deprecation printk block/Kconfig | 12 ++++++++++++ block/bdev.c | 9 ++++++--- block/genhd.c | 6 ++++++ 3 files changed, 24 insertions(+), 3 deletions(-)