Skip to content

Commit bb5faa9

Browse files
Mauricio Faria de Oliveiraaxboe
authored andcommitted
loop: do not enforce max_loop hard limit by (new) default
Problem: The max_loop parameter is used for 2 different purposes: 1) initial number of loop devices to pre-create on init 2) maximum number of loop devices to add on access/open() Historically, its default value (zero) caused 1) to create non-zero number of devices (CONFIG_BLK_DEV_LOOP_MIN_COUNT), and no hard limit on 2) to add devices with autoloading. However, the default value changed in commit 85c5019 ("loop: Fix the max_loop commandline argument treatment when it is set to 0") to CONFIG_BLK_DEV_LOOP_MIN_COUNT, for max_loop=0 not to pre-create devices. That does improve 1), but unfortunately it breaks 2), as the default behavior changed from no-limit to hard-limit. Example: For example, this userspace code broke for N >= CONFIG, if the user relied on the default value 0 for max_loop: mknod("/dev/loopN"); open("/dev/loopN"); // now fails with ENXIO Though affected users may "fix" it with (loop.)max_loop=0, this means to require a kernel parameter change on stable kernel update (that commit Fixes: an old commit in stable). Solution: The original semantics for the default value in 2) can be applied if the parameter is not set (ie, default behavior). This still keeps the intended function in 1) and 2) if set, and that commit's intended improvement in 1) if max_loop=0. Before 85c5019: - default: 1) CONFIG devices 2) no limit - max_loop=0: 1) CONFIG devices 2) no limit - max_loop=X: 1) X devices 2) X limit After 85c5019: - default: 1) CONFIG devices 2) CONFIG limit (*) - max_loop=0: 1) 0 devices (*) 2) no limit - max_loop=X: 1) X devices 2) X limit This commit: - default: 1) CONFIG devices 2) no limit (*) - max_loop=0: 1) 0 devices 2) no limit - max_loop=X: 1) X devices 2) X limit Future: The issue/regression from that commit only affects code under the CONFIG_BLOCK_LEGACY_AUTOLOAD deprecation guard, thus the fix too is contained under it. Once that deprecated functionality/code is removed, the purpose 2) of max_loop (hard limit) is no longer in use, so the module parameter description can be changed then. Tests: Linux 6.4-rc7 CONFIG_BLK_DEV_LOOP_MIN_COUNT=8 CONFIG_BLOCK_LEGACY_AUTOLOAD=y - default (original) # ls -1 /dev/loop* /dev/loop-control /dev/loop0 ... /dev/loop7 # ./test-loop open: /dev/loop8: No such device or address - default (patched) # ls -1 /dev/loop* /dev/loop-control /dev/loop0 ... /dev/loop7 # ./test-loop # - max_loop=0 (original & patched): # ls -1 /dev/loop* /dev/loop-control # ./test-loop # - max_loop=8 (original & patched): # ls -1 /dev/loop* /dev/loop-control /dev/loop0 ... /dev/loop7 # ./test-loop open: /dev/loop8: No such device or address - max_loop=0 (patched; CONFIG_BLOCK_LEGACY_AUTOLOAD is not set) # ls -1 /dev/loop* /dev/loop-control # ./test-loop open: /dev/loop8: No such device or address Fixes: 85c5019 ("loop: Fix the max_loop commandline argument treatment when it is set to 0") Signed-off-by: Mauricio Faria de Oliveira <mfo@canonical.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Link: https://lore.kernel.org/r/20230720143033.841001-3-mfo@canonical.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 23881ae commit bb5faa9

File tree

1 file changed

+34
-2
lines changed

1 file changed

+34
-2
lines changed

drivers/block/loop.c

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,14 +1775,43 @@ static const struct block_device_operations lo_fops = {
17751775
/*
17761776
* If max_loop is specified, create that many devices upfront.
17771777
* This also becomes a hard limit. If max_loop is not specified,
1778+
* the default isn't a hard limit (as before commit 85c50197716c
1779+
* changed the default value from 0 for max_loop=0 reasons), just
17781780
* create CONFIG_BLK_DEV_LOOP_MIN_COUNT loop devices at module
17791781
* init time. Loop devices can be requested on-demand with the
17801782
* /dev/loop-control interface, or be instantiated by accessing
17811783
* a 'dead' device node.
17821784
*/
17831785
static int max_loop = CONFIG_BLK_DEV_LOOP_MIN_COUNT;
1784-
module_param(max_loop, int, 0444);
1786+
1787+
#ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
1788+
static bool max_loop_specified;
1789+
1790+
static int max_loop_param_set_int(const char *val,
1791+
const struct kernel_param *kp)
1792+
{
1793+
int ret;
1794+
1795+
ret = param_set_int(val, kp);
1796+
if (ret < 0)
1797+
return ret;
1798+
1799+
max_loop_specified = true;
1800+
return 0;
1801+
}
1802+
1803+
static const struct kernel_param_ops max_loop_param_ops = {
1804+
.set = max_loop_param_set_int,
1805+
.get = param_get_int,
1806+
};
1807+
1808+
module_param_cb(max_loop, &max_loop_param_ops, &max_loop, 0444);
17851809
MODULE_PARM_DESC(max_loop, "Maximum number of loop devices");
1810+
#else
1811+
module_param(max_loop, int, 0444);
1812+
MODULE_PARM_DESC(max_loop, "Initial number of loop devices");
1813+
#endif
1814+
17861815
module_param(max_part, int, 0444);
17871816
MODULE_PARM_DESC(max_part, "Maximum number of partitions per loop device");
17881817

@@ -2098,7 +2127,7 @@ static void loop_probe(dev_t dev)
20982127
{
20992128
int idx = MINOR(dev) >> part_shift;
21002129

2101-
if (max_loop && idx >= max_loop)
2130+
if (max_loop_specified && max_loop && idx >= max_loop)
21022131
return;
21032132
loop_add(idx);
21042133
}
@@ -2285,6 +2314,9 @@ module_exit(loop_exit);
22852314
static int __init max_loop_setup(char *str)
22862315
{
22872316
max_loop = simple_strtol(str, NULL, 0);
2317+
#ifdef CONFIG_BLOCK_LEGACY_AUTOLOAD
2318+
max_loop_specified = true;
2319+
#endif
22882320
return 1;
22892321
}
22902322

0 commit comments

Comments
 (0)