login
Search
because they are just followup to the real cause. This is causes by commit
commit 73285082745045bcd64333c1fbaa88f8490f2626
Author: Ken Chen <kenchen@google.com>
Date: Tue May 8 00:28:20 2007 -0700
remove artificial software max_loop limit
look what it did (abridged):
=2Dstatic int __init loop_init(void)
[...]
=2D for (i =3D 0; i < max_loop; i++) {
=2D disks[i] =3D alloc_disk(1);
=2D if (!disks[i])
=2D goto out_mem3;
}
=2D
=2D for (i =3D 0; i < max_loop; i++) {
=2D struct loop_device *lo =3D &loop_dev[i];
=2D struct gendisk *disk =3D disks[i];
=2D
=2D memset(lo, 0, sizeof(*lo));
=2D lo->lo_queue =3D blk_alloc_queue(GFP_KERNEL);
=2D if (!lo->lo_queue)
=2D goto out_mem4;
=2D mutex_init(&lo->lo_ctl_mutex);
=2D lo->lo_number =3D i;
=2D lo->lo_thread =3D NULL;
=2D init_waitqueue_head(&lo->lo_event);
=2D spin_lock_init(&lo->lo_lock);
=2D disk->major =3D LOOP_MAJOR;
=2D disk->first_minor =3D i;
=2D disk->fops =3D &lo_fops;
=2D sprintf(disk->disk_name, "loop%d", i);
=2D disk->private_data =3D lo;
=2D disk->queue =3D lo->lo_queue;
=2D }
=2D
=2D /* We cannot fail after we call this, so another loop!*/
=2D for (i =3D 0; i < max_loop; i++)
=2D add_disk(disks[i]);
So before this commit we got /dev/loop%d up to max_loop when loop was loade=
d.=20
After this commit we get nothing (I still wonder wheher lone loop0 comes fr=
om=20
after reboot, because reloading module leaves me without /dev/loop%n=20
alltogether).
This is a real regression because on udev-enabled system (probably 99% of=20
distributions now) losetup as available in current util-linux simply stops=
=20
working.
Yeah, that's the only one left. I was hoping it wasn't that one, as it claimed to have been tested extensively. Guess it wasn't tested with udev. Ken? Ball's in your court. As the patch isn't providing a killer feature for 2.6.22, I'd suggest just reverting it for now until the issues are ironed out. Ray -
Hold it. The real question here is which logics do we want there. IOW, and how many device nodes do we want to appear and _when_ do we want them to appear? -
of course we'd like to use exactly as many (or few) nodes as are in use rig= ht=20 now and without fixed limit for their number; which implies that nodes shou= ld=20 appear and go on as needed basis. But right now there is no kernel mechanism that user level program could us= e=20 to request allocation of new loop node. I won't discuss whether it is=20 legitimate to mandate new version of util-linux for kernel 2.6.22; but it i= s=20 obvious that any kernel patch that adds such mechanism goes far beyond simp= le=20 bug fix and is not acceptable at this stage. So let's revert this change and discuss it for post-2.6.22 timeframe.
Hi, I am of course not a fan of limiting the maximum of available loops to 8. My question / proposal for now would be: Could anybody of you please be kind enough and write / provide me a counter patch supplying me: a. a compilable 2.6.22-rc2 kernel b. a loop device that can mount up to 8 iso-images I would prefer this thing as outline attachment due to Email client wordwrapping problems. Looking happily forward to a functionable counter patch to resolve the current issue as a compromise solution, Best regards and thanks Uwe -
If you revert all three patches in this thread, you should be okay. If you're having trouble with that, you can revert all the way back to the version in 2.6.21 (by just copying it), and then do a search/replace on loop.c to change any invalidate_bdev(bdev, 0) you find to invalidate_bdev(bdev) and that should work as well. -
The when part is what looks to make it racy. I'm guessing that we're relying on udev to create those loop nodes. If so, I think any scheme that creates more on demand would give transient mount errors while it's waiting on udev to create more nodes. Perhaps if we were to start with 8 loop nodes at init (as we have in 2.6.21), and then always maintain a margin of 8 (or 4, or...) when they start being used or detached? -
Until the tools can request dynamic loop device allocation from the kernel before they want to use the device, you can create as many as needed "static" loop* nodes in /lib/udev/devices/, which will be copied to /dev/ early on every bootup. Kay -
Won't these be removed after "losetup -d"?
No, only when you unload the loop module (and then only those that were ever used). Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany PGP key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -
Except that's different than current behavior presented to userspace. IOW, we broke userspace for anyone using udev. Which is, y'know, a lot of us. We're at -rc2 right now. Given that, it looks like we have two options. First is to revert all this for now and try again when the patch has had more testing and agreement (as this isn't a major feature we're talking about here; it's effectively just a cleanup that happened to have unfortunate side-effects). The second option is that we could have the loop device start with 8 nodes populated, which would match current behavior. A third option of requiring new userspace for 2.6.22 is a non-starter. -
Right, providing "preallocated" devices, 8 or the number given in max_loop, sounds like the best option until the tools can handle that. Thanks, Kay -
OK people, this is what I did just to resolve the issue for now: 1. copied loop.c from 2.6.21 into the 2.6.22-rc2 tree 2. changed exactly two entries from "invalidate_bdev(bdev, 0)" to "invalidate_bdev(bdev)" Output is: a. a compilable kernel b. all four iso images are mounted as expected Andrey's path however (i. e. copying his attached version of loop.c into the 2.6.22-rc2 kernel tree) led to: a. an incompilable kernel b. endless messages trying to compile loop.c going like this (just a part of them - not complete anyway!): drivers/block/loop.c:1350: error: stray '\240' in program drivers/block/loop.c:1350: error: stray '\240' in program drivers/block/loop.c:1350: error: stray '\240' in program drivers/block/loop.c:1350: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c:1351: error: stray '\240' in program drivers/block/loop.c: In function 'loop_register_transfer': drivers/block/loop.c:1367: error: stray '\240' in program drivers/block/loop.c:1367: error: stray '\240' in program drivers/block/loop.c:1367: error: stray '\240' in program drivers/block/loop.c:1367: error: stray '\240' in ...
Yes. Can somebody who actually _uses_ loop send a tested patch, please? We're not going to break existing user space over something idiotic like this. Not acceptable. The alternative is to just revert all the loop.c patches. Which I'll do unless somebody sends me a tested fix - because as a non-loop user myself, I don't have much choice. I assume it is commit 73285082 "remove artificial software max_loop limit" that introduced the new behaviour. Ken? Linus -
yes and no. in that commit, I automatically create n+1 device when loop device n is created, allergically was tested to be fine with casual usage of "losetup" and "mount -o loop". However, there is a bug in that commit when loop.c was compiled as a module. And when Al fixed it, he also removed that magic "n+1" trick. Nevertheless, yes, I'm guilty of introducing the new behavior. -
As I said before, the reporter *tested* with Al's two patches reverted, AND IT STILL FAILED. Your commit had to be reverted as well It's not a behavior, it's a bug. Whether you reintroduce the n+1 inductive trick is immaterial to the problem at hand. loop.c needs to populate 8 or max_loop devices upon init to maintain current behavior. -
The easiest way is to reinstate max_loop and create "max_loop" device up front at module load time. However, that will lose all the "fancy on-demand device instantiation feature". So I propose we do the following: 1. have the module honor "max_loop" parameter and create that many device upfront on module load (max_loop will also be a hard max) iff user specify the parameter. 2. if max_loop is not specified, default create 8 loop device. User can extent more loop device by create device node themselves and have kernel automatically instantiate loop device on-demand. Is this acceptable? Patch in a bit. -
Sorry, Ken: My question on point 2 would be: Does "User can extent more loop device by create device node themselves and......." correspond or conflict to working -
Udev shouldn't care if the kernel tells udev about the new device, and the node with the correct dev_t is already there, it will leave it as it is, and only apply the configured user,group,mode values. The loop tools should probably extended to be able to request new devices from the kernel in a different way than open(). Kay -
Except as already mentioned it will introduce races between tool requesting= =20 new device and udev creating new node. We already had this with raw devices= =2E=20 My comparison with /dev/pts was incorrect because it is using internal=20 filesystem that creates devices synchronously. May be all such cases has to be converted to use common file system. mount -t nodefs -o device=3Dpts none /dev/pts mount -t nodefs -o device=3Dloop none /dev/loop
Could people who has problem with loop device please test this? I
tested it on my Ubuntu feisty distribution and it works fine. Though I
typically don't use loop device at all.
---
The kernel on-demand loop device instantiation breaks several user
space tools as the tools are not ready to cope with the "on-demand
feature". Fix it by instantiate default 8 loop devices and also
reinstate max_loop module parameter.
Signed-off-by: Ken Chen <kenchen@google.com>
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 5526ead..0aae8d8 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1354,7 +1354,7 @@ #endif
*/
static int max_loop;
module_param(max_loop, int, 0);
-MODULE_PARM_DESC(max_loop, "obsolete, loop device is created on-demand");
+MODULE_PARM_DESC(max_loop, "Maximum number of loop devices");
MODULE_LICENSE("GPL");
MODULE_ALIAS_BLOCKDEV_MAJOR(LOOP_MAJOR);
@@ -1462,34 +1462,66 @@ static struct kobject *loop_probe(dev_t
return kobj;
}
-static int __init loop_init(void)
-{
- if (register_blkdev(LOOP_MAJOR, "loop"))
- return -EIO;
- blk_register_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS,
- THIS_MODULE, loop_probe, NULL, NULL);
-
- if (max_loop) {
- printk(KERN_INFO "loop: the max_loop option is obsolete "
- "and will be removed in March 2008\n");
-
- }
- printk(KERN_INFO "loop: module loaded\n");
- return 0;
-}
-
static void __exit loop_exit(void)
{
+ unsigned long range;
struct loop_device *lo, *next;
+ range = max_loop ? max_loop : 1UL << MINORBITS;
+
list_for_each_entry_safe(lo, next, &loop_devices, lo_list)
loop_del_one(lo);
- blk_unregister_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS);
+ blk_unregister_region(MKDEV(LOOP_MAJOR, 0), range);
if (unregister_blkdev(LOOP_MAJOR, "loop"))
printk(KERN_WARNING "loop: cannot unregister blkdev\n");
}
+static int __init loop_init(void)
+{
+ int i, nr;
+ unsigned long range;
+
+ /*
+ * loop module now has a feature to instantiate ...Thank you, Ken :) Excellent work, everything runs as expected. Starting compilation of 2.6.22-rc2 I get the following nasty messages: scripts/kconfig/conf -s arch/i386/Kconfig drivers/macintosh/Kconfig:116:warning: 'select' used by config symbol 'PMAC_APM_EMU' refers to undefined symbol 'SYS_SUPPORTS_APM_EMULATION' drivers/net/Kconfig:2283:warning: 'select' used by config symbol 'UCC_GETH' refers to undefined symbol 'UCC_FAST' drivers/input/keyboard/Kconfig:170:warning: 'select' used by config symbol 'KEYBOARD_ATARI' refers to undefined symbol 'ATARI_KBD_CORE' drivers/input/mouse/Kconfig:182:warning: 'select' used by config symbol 'MOUSE_ATARI' refers to undefined symbol 'ATARI_KBD_CORE' But, please note, that has nothing to do with the loop issue, which is solved with this patch. Other sidenote: I meanwhile tried to find out why my AMD K7 machine oopses with 2.6.21.1. I first suspected sis5513 ide module, reverted all its dependencies, even changed a Makefile, but the Oops is still there after 10 modules reverted. I am standing in the dark, although I really would like to help to close down 2.6.21.x "cleanly". On Intel P 4 machines this Oops does not happen at all, only on the AMD K7 machine. Already planned to start a new thread but instead of wild guessing around I do not have any idea wht the reason for the kernel Oops could be. Best Regards and Thanks Uwe -
On Mon, 21 May 2007 23:20:54 +0200 Yes, please send out a fresh bug report for this. -
FWIW, I do and I have tested it; what I did *not* do is using a testbox with dynamic /dev for testing. Mea culpa... AFAICS, patch that went to akpm (preallocate max_loop instances) is OK. -
... except that it needs to do cleanup on failure in loop_init(). -
this seems to work with /dev/pts though. May be, by accident?
Or maybe the kernel puts the requesting process to sleep until the udev userspace helper returns? I don't know. I've run out of useful contributions to this thread, so I think I'll shut up now :-). -
"min_loop" (max_loop?) nodes should appear (but without a backing gendisk), and when they are opened, they should get their gendisk allocated and assigned. Jan -- -
Jan, Please stick to the latest revised patch residing in Andrew's mm-tree now. All that I wanted was a possibility to mount up to eight 8 iso images to be mounted parallely at boot time using udev and AVOID fuzzing around with additional nodes in /dev/loop. This desire isn't nonsense at all: For instance, if you run Debian "Lenny" testing, you need to mount 4 DVD iso images parallely at boot time (at least I do need that) if you do not want to waste time with opening and closing your DVD device at all. Then there may be other desires: a. For instance mounting Christian Marillat's unofficial Debian iso image at boot time (multimedia stuff). b. For instance to mount and run all sampled libraries for helping the wine package to be as compatible as possible (i. e. winetools by Joachim von Thadden) and so on..... That means: At least for my personal desire, Ken Chen's latest patch was OK so far. Now, if there is a specific need for mounting up to 256 iso images including whatever technical basis or userspace tools requiries or changings of whatever kind then at least I do not care about the what and why at all. But the facts for now at least go like this: 1. Ken Chen's latest patch, criticised and enhanced by Andrew Morton, acked by Al Viro, is now part of the latest mm-tree. 2. The three patches leading to the disaster of breaking userspace are not yet ripped out of Linus's 2.6.22-rc2-tree. In so far I would deeply appreciate Linus Torvalds to rip put the mentioned patches mentioned, just to avoid future troubles. Yours sincerely Uwe P. S.: 1. The first approach, a common work by Ken Chen and Al Viro, was an approach promising up to 256 iso images to be mounted dynamically on user's demand while really offering only one iso image to be mounted and, after the desire to mount some more at boot time, stop working. I would evaluate that approach as stuff for cabaret laughter, jokin', or simply a terrible bug. 2. I do not like / ...
The real solution is to have the user space tool to create these
device nodes in advance.
The original loop patch was coded such that when we open a loop device
N, the immediate adjacent device "N + 1" is created. This will keep
"mount -o loop" happy because it typically does a linear scan to find
a free device. This might be somewhat hackary, but certainly will be
backward compatible before user space solution is deployed.
However, the code was removed by Al in this commit:
commit 07002e995638b83a6987180f43722a0eb39d4932
Author: Al Viro <viro@zeniv.linux.org.uk>
Date: Sat May 12 16:23:15 2007 -0400
fix the dynamic allocation and probe in loop.c
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Acked-by: Ken Chen <kenchen@google.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-
Maybe. But requiring people upgrade their userspace tools or setup for Except userspace is currently expecting 8 loop nodes upon bootup. Creating n+1 when n is opened is good, but racy if userspace tries to mount serveral loop devices in parallel. If the loop device instantiates 8 (or max_loop) upon init, then we're No, backing that code out wasn't good enough -- the reporter tested reverting both of Al's patches out and was still getting errors on boot. It took reverting your original one out as well to make it work. So, a compromise? Let's start with 8 (or max_loop) populated, and then we can move forward separately with teaching userspace new loop tricks. -
Yes. I reverted three in fact. Please see my latest entry in this thread to Yes. Good idea! The reason why I need 2.6.22-rc2 and why I cannot work with 2.6.21.1 is that I get a kernel oops (hangup) with 2.6.21.1 about 20 seconds after login. The machine is an AMD K7 with but SiS chipsets / controllers onboard. Responsible for that kernel oops with 2.6.21.1 is supposedly the whole ide layer plus sis5513.c. In so far, pulling all ide layer changes plus the sis5513.c patch from 2.6.22-rc2 into mainline of 2.6.21 would definitely be a very good idea. Thanks for the quick help, especially to Ray. Best Regards Uwe -
