2007-05-19 13:53:56

by Uwe Bugla

[permalink] [raw]
Subject: bug in 2.6.22-rc2: loop mount limited to one single iso image

Hi,

I am running Debian Etch 4.0 stable in connection with udev.

In kernel 2.6.22-rc2 the number of available loop mounts is reduced from 8 (conventional standard of preceding 2.6 kernels) to 1.

Symptom: If you try to mount more than one single iso-image with
the loop parameter you are receiving the following message during system boot:

"mount: could not find any free loop device"

Kernel 2.6.20.11 does not show that problem.

Question: Can someone reading this confirm and reproduce that problem?
If yes: Are there any fixes available?

Thanks and Regards

Uwe

--
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
Ideal f?r Modem und ISDN: http://www.gmx.net/de/go/smartsurfer


2007-05-19 18:33:40

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Hey there,

On 5/19/07, Uwe Bugla <[email protected]> wrote:
> I am running Debian Etch 4.0 stable in connection with udev.
>
> In kernel 2.6.22-rc2 the number of available loop mounts is reduced from
> 8 (conventional standard of preceding 2.6 kernels) to 1.
>
> Symptom: If you try to mount more than one single iso-image with
> the loop parameter you are receiving the following message during system boot:
>
> "mount: could not find any free loop device"
>
> Kernel 2.6.20.11 does not show that problem.
>
> Question: Can someone reading this confirm and reproduce that problem?

I can reproduce the problem pretty trivially with:

mkdir a m1 m2
touch a/1
genisoimage -o cd1.iso a || mkisofs -o cd1.iso a
cp cd1.iso cd2.iso
sudo mount -o loop cd1.iso m1
sudo mount -o loop cd2.iso m2

...and the last mount fails. That used to work, at least as of 2.6.15, which is
the only other 2.6 kernel I have running on a machine that I can log into at the
moment.

Looking at the changelogs between 2.6.20 and current tip shows at least three
significant patches to loop.c. The first was supposedly tested heavily, so
we'll leave that alone for now. (They all were applied after 2.6.21 was released,
so it's probably fine.)

Anyway, could you try reverting the two patches below (in order: the first, then
the second) and see if that fixes it? The second one looks iffy to me, but if
this fixes it then I'm sure Al can sort out why.

Ray



commit 705962ccc9d21a08b74b6b6e1d3cf10f98968a67
Author: Al Viro <[email protected]>
Date: Sun May 13 05:52:32 2007 -0400

fix deadlock in loop.c

... doh

Jeremy Fitzhardinge noted that the recent loop.c cleanups worked, but
cause lockdep to complain.

Ouch. OK, the deadlock is real and yes, I'm an idiot. Speaking of which,
we probably want to s/lock/pin/ in drivers/base/map.c to avoid such
brainos again. And yes, this stuff needs clear documentation. Will try
to put one together once I get some sleep...

Signed-off-by: Al Viro <[email protected]>
Cc: Jeremy Fitzhardinge <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index e2fc4b6..5526ead 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1399,6 +1399,11 @@ static struct loop_device *loop_init_one(int i)
struct loop_device *lo;
struct gendisk *disk;

+ list_for_each_entry(lo, &loop_devices, lo_list) {
+ if (lo->lo_number == i)
+ return lo;
+ }
+
lo = kzalloc(sizeof(*lo), GFP_KERNEL);
if (!lo)
goto out;
@@ -1443,17 +1448,13 @@ static void loop_del_one(struct loop_device *lo)
kfree(lo);
}

-static int loop_lock(dev_t dev, void *data)
-{
- mutex_lock(&loop_devices_mutex);
- return 0;
-}
-
static struct kobject *loop_probe(dev_t dev, int *part, void *data)
{
- struct loop_device *lo = loop_init_one(dev & MINORMASK);
+ struct loop_device *lo;
struct kobject *kobj;

+ mutex_lock(&loop_devices_mutex);
+ lo = loop_init_one(dev & MINORMASK);
kobj = lo ? get_disk(lo->lo_disk) : ERR_PTR(-ENOMEM);
mutex_unlock(&loop_devices_mutex);

@@ -1466,7 +1467,7 @@ 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, loop_lock, NULL);
+ THIS_MODULE, loop_probe, NULL, NULL);

if (max_loop) {
printk(KERN_INFO "loop: the max_loop option is obsolete "





commit 07002e995638b83a6987180f43722a0eb39d4932
Author: Al Viro <[email protected]>
Date: Sat May 12 16:23:15 2007 -0400

fix the dynamic allocation and probe in loop.c

Signed-off-by: Al Viro <[email protected]>
Acked-by: Ken Chen <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 18cdd8c..e2fc4b6 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1317,18 +1317,6 @@ static long lo_compat_ioctl(struct file *file, unsigned int cmd, unsigned long a
}
#endif

-static struct loop_device *loop_find_dev(int number)
-{
- struct loop_device *lo;
-
- list_for_each_entry(lo, &loop_devices, lo_list) {
- if (lo->lo_number == number)
- return lo;
- }
- return NULL;
-}
-
-static struct loop_device *loop_init_one(int i);
static int lo_open(struct inode *inode, struct file *file)
{
struct loop_device *lo = inode->i_bdev->bd_disk->private_data;
@@ -1337,11 +1325,6 @@ static int lo_open(struct inode *inode, struct file *file)
lo->lo_refcnt++;
mutex_unlock(&lo->lo_ctl_mutex);

- mutex_lock(&loop_devices_mutex);
- if (!loop_find_dev(lo->lo_number + 1))
- loop_init_one(lo->lo_number + 1);
- mutex_unlock(&loop_devices_mutex);
-
return 0;
}

@@ -1448,7 +1431,7 @@ out_free_queue:
out_free_dev:
kfree(lo);
out:
- return ERR_PTR(-ENOMEM);
+ return NULL;
}

static void loop_del_one(struct loop_device *lo)
@@ -1460,36 +1443,30 @@ static void loop_del_one(struct loop_device *lo)
kfree(lo);
}

+static int loop_lock(dev_t dev, void *data)
+{
+ mutex_lock(&loop_devices_mutex);
+ return 0;
+}
+
static struct kobject *loop_probe(dev_t dev, int *part, void *data)
{
- unsigned int number = dev & MINORMASK;
- struct loop_device *lo;
+ struct loop_device *lo = loop_init_one(dev & MINORMASK);
+ struct kobject *kobj;

- mutex_lock(&loop_devices_mutex);
- lo = loop_find_dev(number);
- if (lo == NULL)
- lo = loop_init_one(number);
+ kobj = lo ? get_disk(lo->lo_disk) : ERR_PTR(-ENOMEM);
mutex_unlock(&loop_devices_mutex);

*part = 0;
- if (IS_ERR(lo))
- return (void *)lo;
- else
- return &lo->lo_disk->kobj;
+ return kobj;
}

static int __init loop_init(void)
{
- struct loop_device *lo;
-
if (register_blkdev(LOOP_MAJOR, "loop"))
return -EIO;
blk_register_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS,
- THIS_MODULE, loop_probe, NULL, NULL);
-
- lo = loop_init_one(0);
- if (IS_ERR(lo))
- goto out;
+ THIS_MODULE, loop_probe, loop_lock, NULL);

if (max_loop) {
printk(KERN_INFO "loop: the max_loop option is obsolete "
@@ -1498,11 +1475,6 @@ static int __init loop_init(void)
}
printk(KERN_INFO "loop: module loaded\n");
return 0;
-
-out:
- unregister_blkdev(LOOP_MAJOR, "loop");
- printk(KERN_ERR "loop: ran out of memory\n");
- return -ENOMEM;
}

static void __exit loop_exit(void)

2007-05-19 19:18:17

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Ray Lee wrote:

> Hey there,
>
> On 5/19/07, Uwe Bugla <[email protected]> wrote:
> > I am running Debian Etch 4.0 stable in connection with udev.
> >
> > In kernel 2.6.22-rc2 the number of available loop mounts is reduced
> > from 8 (conventional standard of preceding 2.6 kernels) to 1.
> >
> > Symptom: If you try to mount more than one single iso-image with
> > the loop parameter you are receiving the following message during
> > system boot:
> >
> > "mount: could not find any free loop device"
> >
> > Kernel 2.6.20.11 does not show that problem.
> >
> > Question: Can someone reading this confirm and reproduce that problem?
>
> I can reproduce the problem pretty trivially with:
>
> mkdir a m1 m2
> touch a/1
> genisoimage -o cd1.iso a || mkisofs -o cd1.iso a
> cp cd1.iso cd2.iso
> sudo mount -o loop cd1.iso m1
> sudo mount -o loop cd2.iso m2
>
> ...and the last mount fails. That used to work, at least as of 2.6.15,
> which is the only other 2.6 kernel I have running on a machine that I can
> log into at the moment.
>


This is unfortunate case of user-space breakage.

Now when we do not have specific loop device limit, we also do not
pre-register loop devices:

{pts/0}% LC_ALL=C ll /dev/loop*
brw-rw---- 1 root disk 7, 0 May 19 20:33 /dev/loop0

(I wonder where loop0 comes from at all :) but losetup & Co unfortunately
need preexisting device node :(

Trivial workaround for those who need it now - wrapper that creates node and
calls real losetup. I am not sure this actually works with loop mount
though.

Real fix is to add special control node and change losetup to use it instead
(or in addition) to opening /dev/loop%d. But as it stands now I'd call it
big regression.

-andrey

> Looking at the changelogs between 2.6.20 and current tip shows at least
> three significant patches to loop.c. The first was supposedly tested
> heavily, so we'll leave that alone for now. (They all were applied after
> 2.6.21 was released, so it's probably fine.)
>
> Anyway, could you try reverting the two patches below (in order: the
> first, then the second) and see if that fixes it? The second one looks
> iffy to me, but if this fixes it then I'm sure Al can sort out why.
>
> Ray
>
>
>
> commit 705962ccc9d21a08b74b6b6e1d3cf10f98968a67
> Author: Al Viro <[email protected]>
> Date: Sun May 13 05:52:32 2007 -0400
>
> fix deadlock in loop.c
>
> ... doh
>
> Jeremy Fitzhardinge noted that the recent loop.c cleanups worked, but
> cause lockdep to complain.
>
> Ouch. OK, the deadlock is real and yes, I'm an idiot. Speaking of
> which, we probably want to s/lock/pin/ in drivers/base/map.c to avoid
> such
> brainos again. And yes, this stuff needs clear documentation. Will
> try to put one together once I get some sleep...
>
> Signed-off-by: Al Viro <[email protected]>
> Cc: Jeremy Fitzhardinge <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index e2fc4b6..5526ead 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1399,6 +1399,11 @@ static struct loop_device *loop_init_one(int i)
> struct loop_device *lo;
> struct gendisk *disk;
>
> + list_for_each_entry(lo, &loop_devices, lo_list) {
> + if (lo->lo_number == i)
> + return lo;
> + }
> +
> lo = kzalloc(sizeof(*lo), GFP_KERNEL);
> if (!lo)
> goto out;
> @@ -1443,17 +1448,13 @@ static void loop_del_one(struct loop_device *lo)
> kfree(lo);
> }
>
> -static int loop_lock(dev_t dev, void *data)
> -{
> - mutex_lock(&loop_devices_mutex);
> - return 0;
> -}
> -
> static struct kobject *loop_probe(dev_t dev, int *part, void *data)
> {
> - struct loop_device *lo = loop_init_one(dev & MINORMASK);
> + struct loop_device *lo;
> struct kobject *kobj;
>
> + mutex_lock(&loop_devices_mutex);
> + lo = loop_init_one(dev & MINORMASK);
> kobj = lo ? get_disk(lo->lo_disk) : ERR_PTR(-ENOMEM);
> mutex_unlock(&loop_devices_mutex);
>
> @@ -1466,7 +1467,7 @@ 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, loop_lock, NULL);
> + THIS_MODULE, loop_probe, NULL, NULL);
>
> if (max_loop) {
> printk(KERN_INFO "loop: the max_loop option is obsolete "
>
>
>
>
>
> commit 07002e995638b83a6987180f43722a0eb39d4932
> Author: Al Viro <[email protected]>
> Date: Sat May 12 16:23:15 2007 -0400
>
> fix the dynamic allocation and probe in loop.c
>
> Signed-off-by: Al Viro <[email protected]>
> Acked-by: Ken Chen <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 18cdd8c..e2fc4b6 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -1317,18 +1317,6 @@ static long lo_compat_ioctl(struct file *file,
> unsigned int cmd, unsigned long a
> }
> #endif
>
> -static struct loop_device *loop_find_dev(int number)
> -{
> - struct loop_device *lo;
> -
> - list_for_each_entry(lo, &loop_devices, lo_list) {
> - if (lo->lo_number == number)
> - return lo;
> - }
> - return NULL;
> -}
> -
> -static struct loop_device *loop_init_one(int i);
> static int lo_open(struct inode *inode, struct file *file)
> {
> struct loop_device *lo = inode->i_bdev->bd_disk->private_data;
> @@ -1337,11 +1325,6 @@ static int lo_open(struct inode *inode, struct file
> *file)
> lo->lo_refcnt++;
> mutex_unlock(&lo->lo_ctl_mutex);
>
> - mutex_lock(&loop_devices_mutex);
> - if (!loop_find_dev(lo->lo_number + 1))
> - loop_init_one(lo->lo_number + 1);
> - mutex_unlock(&loop_devices_mutex);
> -
> return 0;
> }
>
> @@ -1448,7 +1431,7 @@ out_free_queue:
> out_free_dev:
> kfree(lo);
> out:
> - return ERR_PTR(-ENOMEM);
> + return NULL;
> }
>
> static void loop_del_one(struct loop_device *lo)
> @@ -1460,36 +1443,30 @@ static void loop_del_one(struct loop_device *lo)
> kfree(lo);
> }
>
> +static int loop_lock(dev_t dev, void *data)
> +{
> + mutex_lock(&loop_devices_mutex);
> + return 0;
> +}
> +
> static struct kobject *loop_probe(dev_t dev, int *part, void *data)
> {
> - unsigned int number = dev & MINORMASK;
> - struct loop_device *lo;
> + struct loop_device *lo = loop_init_one(dev & MINORMASK);
> + struct kobject *kobj;
>
> - mutex_lock(&loop_devices_mutex);
> - lo = loop_find_dev(number);
> - if (lo == NULL)
> - lo = loop_init_one(number);
> + kobj = lo ? get_disk(lo->lo_disk) : ERR_PTR(-ENOMEM);
> mutex_unlock(&loop_devices_mutex);
>
> *part = 0;
> - if (IS_ERR(lo))
> - return (void *)lo;
> - else
> - return &lo->lo_disk->kobj;
> + return kobj;
> }
>
> static int __init loop_init(void)
> {
> - struct loop_device *lo;
> -
> if (register_blkdev(LOOP_MAJOR, "loop"))
> return -EIO;
> blk_register_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS,
> - THIS_MODULE, loop_probe, NULL, NULL);
> -
> - lo = loop_init_one(0);
> - if (IS_ERR(lo))
> - goto out;
> + THIS_MODULE, loop_probe, loop_lock, NULL);
>
> if (max_loop) {
> printk(KERN_INFO "loop: the max_loop option is obsolete "
> @@ -1498,11 +1475,6 @@ static int __init loop_init(void)
> }
> printk(KERN_INFO "loop: module loaded\n");
> return 0;
> -
> -out:
> - unregister_blkdev(LOOP_MAJOR, "loop");
> - printk(KERN_ERR "loop: ran out of memory\n");
> - return -ENOMEM;
> }
>
> static void __exit loop_exit(void)


2007-05-20 04:46:00

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Sunday 20 May 2007, Uwe Bugla wrote:
> Am Samstag, 19. Mai 2007 21:17 schrieben Sie:
> > Ray Lee wrote:
> > > Hey there,
> > >
> > > On 5/19/07, Uwe Bugla <[email protected]> wrote:
> > > > I am running Debian Etch 4.0 stable in connection with udev.
> > > >
> > > > In kernel 2.6.22-rc2 the number of available loop mounts is reduced
> > > > from 8 (conventional standard of preceding 2.6 kernels) to 1.
> > > >
> > > > Symptom: If you try to mount more than one single iso-image with
> > > > the loop parameter you are receiving the following message during
> > > > system boot:
> > > >
> > > > "mount: could not find any free loop device"
> > > >
> > > > Kernel 2.6.20.11 does not show that problem.
> > > >
> > > > Question: Can someone reading this confirm and reproduce that
> > > > problem?
> > >
> > > I can reproduce the problem pretty trivially with:
> > >
> > > mkdir a m1 m2
> > > touch a/1
> > > genisoimage -o cd1.iso a || mkisofs -o cd1.iso a
> > > cp cd1.iso cd2.iso
> > > sudo mount -o loop cd1.iso m1
> > > sudo mount -o loop cd2.iso m2
> > >
> > > ...and the last mount fails. That used to work, at least as of 2.6.15,
> > > which is the only other 2.6 kernel I have running on a machine that I
> > > can log into at the moment.
> >
> > This is unfortunate case of user-space breakage.
> >
> > Now when we do not have specific loop device limit, we also do not
> > pre-register loop devices:
> >
> > {pts/0}% LC_ALL=C ll /dev/loop*
> > brw-rw---- 1 root disk 7, 0 May 19 20:33 /dev/loop0
> >
> > (I wonder where loop0 comes from at all :) but losetup & Co unfortunately
> > need preexisting device node :(
> >
> > Trivial workaround for those who need it now - wrapper that creates node
> > and calls real losetup. I am not sure this actually works with loop mount
> > though.
> >
> > Real fix is to add special control node and change losetup to use it
> > instead (or in addition) to opening /dev/loop%d. But as it stands now I'd
> > call it big regression.
> >
> > -andrey
> >
> > > Looking at the changelogs between 2.6.20 and current tip shows at least
> > > three significant patches to loop.c. The first was supposedly tested
> > > heavily, so we'll leave that alone for now. (They all were applied
> > > after 2.6.21 was released, so it's probably fine.)
> > >
> > > Anyway, could you try reverting the two patches below (in order: the
> > > first, then the second) and see if that fixes it? The second one looks
> > > iffy to me, but if this fixes it then I'm sure Al can sort out why.
> > >
> > > Ray
> > >
> > >
> > >
> > > commit 705962ccc9d21a08b74b6b6e1d3cf10f98968a67
> > > Author: Al Viro <[email protected]>
> > > Date: Sun May 13 05:52:32 2007 -0400
> > >
> > > fix deadlock in loop.c
> > >
> > > ... doh
> > >
> > > Jeremy Fitzhardinge noted that the recent loop.c cleanups worked,
> > > but cause lockdep to complain.
> > >
> > > Ouch. OK, the deadlock is real and yes, I'm an idiot. Speaking
> > > of which, we probably want to s/lock/pin/ in drivers/base/map.c to
> > > avoid such
> > > brainos again. And yes, this stuff needs clear documentation.
> > > Will try to put one together once I get some sleep...
> > >
> > > Signed-off-by: Al Viro <[email protected]>
> > > Cc: Jeremy Fitzhardinge <[email protected]>
> > > Signed-off-by: Linus Torvalds <[email protected]>
> > >
> > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> > > index e2fc4b6..5526ead 100644
> > > --- a/drivers/block/loop.c
> > > +++ b/drivers/block/loop.c
> > > @@ -1399,6 +1399,11 @@ static struct loop_device *loop_init_one(int i)
> > > struct loop_device *lo;
> > > struct gendisk *disk;
> > >
> > > + list_for_each_entry(lo, &loop_devices, lo_list) {
> > > + if (lo->lo_number == i)
> > > + return lo;
> > > + }
> > > +
> > > lo = kzalloc(sizeof(*lo), GFP_KERNEL);
> > > if (!lo)
> > > goto out;
> > > @@ -1443,17 +1448,13 @@ static void loop_del_one(struct loop_device
> > > *lo) kfree(lo);
> > > }
> > >
> > > -static int loop_lock(dev_t dev, void *data)
> > > -{
> > > - mutex_lock(&loop_devices_mutex);
> > > - return 0;
> > > -}
> > > -
> > > static struct kobject *loop_probe(dev_t dev, int *part, void *data)
> > > {
> > > - struct loop_device *lo = loop_init_one(dev & MINORMASK);
> > > + struct loop_device *lo;
> > > struct kobject *kobj;
> > >
> > > + mutex_lock(&loop_devices_mutex);
> > > + lo = loop_init_one(dev & MINORMASK);
> > > kobj = lo ? get_disk(lo->lo_disk) : ERR_PTR(-ENOMEM);
> > > mutex_unlock(&loop_devices_mutex);
> > >
> > > @@ -1466,7 +1467,7 @@ 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, loop_lock,
> > > NULL); + THIS_MODULE, loop_probe, NULL,
> > > NULL);
> > >
> > > if (max_loop) {
> > > printk(KERN_INFO "loop: the max_loop option is obsolete "
> > >
> > >
> > >
> > >
> > >
> > > commit 07002e995638b83a6987180f43722a0eb39d4932
> > > Author: Al Viro <[email protected]>
> > > Date: Sat May 12 16:23:15 2007 -0400
> > >
> > > fix the dynamic allocation and probe in loop.c
> > >
> > > Signed-off-by: Al Viro <[email protected]>
> > > Acked-by: Ken Chen <[email protected]>
> > > Signed-off-by: Linus Torvalds <[email protected]>
> > >
> > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> > > index 18cdd8c..e2fc4b6 100644
> > > --- a/drivers/block/loop.c
> > > +++ b/drivers/block/loop.c
> > > @@ -1317,18 +1317,6 @@ static long lo_compat_ioctl(struct file *file,
> > > unsigned int cmd, unsigned long a
> > > }
> > > #endif
> > >
> > > -static struct loop_device *loop_find_dev(int number)
> > > -{
> > > - struct loop_device *lo;
> > > -
> > > - list_for_each_entry(lo, &loop_devices, lo_list) {
> > > - if (lo->lo_number == number)
> > > - return lo;
> > > - }
> > > - return NULL;
> > > -}
> > > -
> > > -static struct loop_device *loop_init_one(int i);
> > > static int lo_open(struct inode *inode, struct file *file)
> > > {
> > > struct loop_device *lo = inode->i_bdev->bd_disk->private_data;
> > > @@ -1337,11 +1325,6 @@ static int lo_open(struct inode *inode, struct
> > > file *file)
> > > lo->lo_refcnt++;
> > > mutex_unlock(&lo->lo_ctl_mutex);
> > >
> > > - mutex_lock(&loop_devices_mutex);
> > > - if (!loop_find_dev(lo->lo_number + 1))
> > > - loop_init_one(lo->lo_number + 1);
> > > - mutex_unlock(&loop_devices_mutex);
> > > -
> > > return 0;
> > > }
> > >
> > > @@ -1448,7 +1431,7 @@ out_free_queue:
> > > out_free_dev:
> > > kfree(lo);
> > > out:
> > > - return ERR_PTR(-ENOMEM);
> > > + return NULL;
> > > }
> > >
> > > static void loop_del_one(struct loop_device *lo)
> > > @@ -1460,36 +1443,30 @@ static void loop_del_one(struct loop_device
> > > *lo) kfree(lo);
> > > }
> > >
> > > +static int loop_lock(dev_t dev, void *data)
> > > +{
> > > + mutex_lock(&loop_devices_mutex);
> > > + return 0;
> > > +}
> > > +
> > > static struct kobject *loop_probe(dev_t dev, int *part, void *data)
> > > {
> > > - unsigned int number = dev & MINORMASK;
> > > - struct loop_device *lo;
> > > + struct loop_device *lo = loop_init_one(dev & MINORMASK);
> > > + struct kobject *kobj;
> > >
> > > - mutex_lock(&loop_devices_mutex);
> > > - lo = loop_find_dev(number);
> > > - if (lo == NULL)
> > > - lo = loop_init_one(number);
> > > + kobj = lo ? get_disk(lo->lo_disk) : ERR_PTR(-ENOMEM);
> > > mutex_unlock(&loop_devices_mutex);
> > >
> > > *part = 0;
> > > - if (IS_ERR(lo))
> > > - return (void *)lo;
> > > - else
> > > - return &lo->lo_disk->kobj;
> > > + return kobj;
> > > }
> > >
> > > static int __init loop_init(void)
> > > {
> > > - struct loop_device *lo;
> > > -
> > > if (register_blkdev(LOOP_MAJOR, "loop"))
> > > return -EIO;
> > > blk_register_region(MKDEV(LOOP_MAJOR, 0), 1UL << MINORBITS,
> > > - THIS_MODULE, loop_probe, NULL, NULL);
> > > -
> > > - lo = loop_init_one(0);
> > > - if (IS_ERR(lo))
> > > - goto out;
> > > + THIS_MODULE, loop_probe, loop_lock,
> > > NULL);
> > >
> > > if (max_loop) {
> > > printk(KERN_INFO "loop: the max_loop option is obsolete "
> > > @@ -1498,11 +1475,6 @@ static int __init loop_init(void)
> > > }
> > > printk(KERN_INFO "loop: module loaded\n");
> > > return 0;
> > > -
> > > -out:
> > > - unregister_blkdev(LOOP_MAJOR, "loop");
> > > - printk(KERN_ERR "loop: ran out of memory\n");
> > > - return -ENOMEM;
> > > }
> > >
> > > static void __exit loop_exit(void)
>
> Hello Ray, hello Andrey,
>
> First of all, thank you deeply for your contributions / reproduction /
> ideas!
>
> unfortunately it does not make any difference if I simply rip out the
> changes of patch-2.6.22-rc2 or patch-2.6.21 in connection with patch
> 2.6.22-rc2 regarding module loop.c.
>

because they are just followup to the real cause. This is causes by commit

commit 73285082745045bcd64333c1fbaa88f8490f2626
Author: Ken Chen <[email protected]>
Date: Tue May 8 00:28:20 2007 -0700

remove artificial software max_loop limit


look what it did (abridged):

-static int __init loop_init(void)
[...]
- for (i = 0; i < max_loop; i++) {
- disks[i] = alloc_disk(1);
- if (!disks[i])
- goto out_mem3;
}
-
- for (i = 0; i < max_loop; i++) {
- struct loop_device *lo = &loop_dev[i];
- struct gendisk *disk = disks[i];
-
- memset(lo, 0, sizeof(*lo));
- lo->lo_queue = blk_alloc_queue(GFP_KERNEL);
- if (!lo->lo_queue)
- goto out_mem4;
- mutex_init(&lo->lo_ctl_mutex);
- lo->lo_number = i;
- lo->lo_thread = NULL;
- init_waitqueue_head(&lo->lo_event);
- spin_lock_init(&lo->lo_lock);
- disk->major = LOOP_MAJOR;
- disk->first_minor = i;
- disk->fops = &lo_fops;
- sprintf(disk->disk_name, "loop%d", i);
- disk->private_data = lo;
- disk->queue = lo->lo_queue;
- }
-
- /* We cannot fail after we call this, so another loop!*/
- for (i = 0; i < max_loop; i++)
- add_disk(disks[i]);

So before this commit we got /dev/loop%d up to max_loop when loop was loaded.
After this commit we get nothing (I still wonder wheher lone loop0 comes from
after reboot, because reloading module leaves me without /dev/loop%n
alltogether).

This is a real regression because on udev-enabled system (probably 99% of
distributions now) losetup as available in current util-linux simply stops
working.

-andrey

> In all mentioned cases I get nothing but an incompilable kernel 2.6.22-rc2.
>
> As I stated already the mentioned loop-mount-problem does not exist in
> Kernel 2.6.20.11, who is, at least for the performance level on my machine,
> nothing but a bad compromise.
>
> I hope that Al Viro will supply a solution for this horrible kernel
> regression. And I do not think that Al Viro is an idiot - he is just a
> human, as all of us are.
>
> Yours sincerely
>
> Uwe



Attachments:
(No filename) (11.32 kB)
(No filename) (189.00 B)
Download all attachments

2007-05-20 06:17:16

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/19/07, Andrey Borzenkov <[email protected]> wrote:
> On Sunday 20 May 2007, Uwe Bugla wrote:
> > unfortunately it does not make any difference if I simply rip out the
> > changes of patch-2.6.22-rc2 or patch-2.6.21 in connection with patch
> > 2.6.22-rc2 regarding module loop.c.
>
> because they are just followup to the real cause. This is causes by commit
>
> commit 73285082745045bcd64333c1fbaa88f8490f2626
> Author: Ken Chen <[email protected]>
> Date: Tue May 8 00:28:20 2007 -0700
>
> remove artificial software max_loop limit
>

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

2007-05-20 06:28:30

by Al Viro

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> 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.

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?

2007-05-20 06:58:49

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Sunday 20 May 2007, Al Viro wrote:
> On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > 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.
>
> 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 right
now and without fixed limit for their number; which implies that nodes should
appear and go on as needed basis.

But right now there is no kernel mechanism that user level program could use
to request allocation of new loop node. I won't discuss whether it is
legitimate to mandate new version of util-linux for kernel 2.6.22; but it is
obvious that any kernel patch that adds such mechanism goes far beyond simple
bug fix and is not acceptable at this stage.

So let's revert this change and discuss it for post-2.6.22 timeframe.


Attachments:
(No filename) (1.02 kB)
(No filename) (189.00 B)
Download all attachments

2007-05-20 14:56:48

by Uwe Bugla

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Am Sonntag, 20. Mai 2007 08:58 schrieben Sie:
> On Sunday 20 May 2007, Al Viro wrote:
> > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > 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.
> >
> > 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
> right now and without fixed limit for their number; which implies that
> nodes should appear and go on as needed basis.
>
> But right now there is no kernel mechanism that user level program could
> use to request allocation of new loop node. I won't discuss whether it is
> legitimate to mandate new version of util-linux for kernel 2.6.22; but it
> is obvious that any kernel patch that adds such mechanism goes far beyond
> simple 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

2007-05-20 15:22:19

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/19/07, Al Viro <[email protected]> wrote:
> On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > 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.
>
> 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?

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?

2007-05-20 15:27:04

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/20/07, Uwe Bugla <[email protected]> wrote:
> 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

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.

2007-05-20 15:54:42

by Kay Sievers

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/20/07, Ray Lee <[email protected]> wrote:
> On 5/19/07, Al Viro <[email protected]> wrote:
> > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > 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.
> >
> > 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?
>
> 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?
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

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

2007-05-20 16:03:19

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Sunday 20 May 2007, Kay Sievers wrote:
> On 5/20/07, Ray Lee <[email protected]> wrote:
> > On 5/19/07, Al Viro <[email protected]> wrote:
> > > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > > 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.
> > >
> > > 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?
> >
> > 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?
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel"
> > in the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
>
> 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.
>

Won't these be removed after "losetup -d"?


Attachments:
(No filename) (1.55 kB)
(No filename) (189.00 B)
Download all attachments

2007-05-20 16:09:51

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Sunday 20 May 2007, Ray Lee wrote:
> On 5/19/07, Al Viro <[email protected]> wrote:
> > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > 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.
> >
> > 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?
>
> 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.
>

this seems to work with /dev/pts though. May be, by accident?


Attachments:
(No filename) (824.00 B)
(No filename) (189.00 B)
Download all attachments

2007-05-20 16:10:40

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/20/07, Kay Sievers <[email protected]> wrote:
> On 5/20/07, Ray Lee <[email protected]> wrote:
> > On 5/19/07, Al Viro <[email protected]> wrote:
> > > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > > 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.
> > >
> > > 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?
> >
> > 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.

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.

2007-05-20 16:15:00

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/20/07, Andrey Borzenkov <[email protected]> wrote:
> On Sunday 20 May 2007, Ray Lee wrote:
> > 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.
>
> 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 :-).

2007-05-20 16:18:51

by Kay Sievers

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Sun, 2007-05-20 at 09:10 -0700, Ray Lee wrote:
> On 5/20/07, Kay Sievers <[email protected]> wrote:
> > On 5/20/07, Ray Lee <[email protected]> wrote:
> > > On 5/19/07, Al Viro <[email protected]> wrote:
> > > > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > > > 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.
> > > >
> > > > 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?
> > >
> > > 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.
>
> 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

2007-05-20 16:24:17

by Andreas Schwab

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Andrey Borzenkov <[email protected]> writes:

> On Sunday 20 May 2007, Kay Sievers wrote:
>>
>> 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.
>>
>
> 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, [email protected]
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."

2007-05-20 16:34:16

by Uwe Bugla

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Am Sonntag, 20. Mai 2007 18:16 schrieben Sie:
> On Sun, 2007-05-20 at 09:10 -0700, Ray Lee wrote:
> > On 5/20/07, Kay Sievers <[email protected]> wrote:
> > > On 5/20/07, Ray Lee <[email protected]> wrote:
> > > > On 5/19/07, Al Viro <[email protected]> wrote:
> > > > > On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> > > > > > 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.
> > > > >
> > > > > 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?
> > > >
> > > > 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.
> >
> > 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 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 program
drivers/block/loop.c:1367: error: stray '\240' in program

Thanks to Ray! Well done!

Best regards

Uwe

2007-05-20 19:55:27

by Michael Mauch

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Uwe Bugla wrote:

> 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

That's the fault of one of your MUAs, that decided to convert a
normal space into a "no break space". With GNU sed you could use

sed -i 's/\xa0/\x20/g' loop.c

to replace these back.

Michael

2007-05-21 06:08:55

by Ken Chen

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/19/07, Ray Lee <[email protected]> wrote:
> 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.

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 <[email protected]>
Date: Sat May 12 16:23:15 2007 -0400

fix the dynamic allocation and probe in loop.c

Signed-off-by: Al Viro <[email protected]>
Acked-by: Ken Chen <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>

2007-05-21 06:40:47

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/20/07, Ken Chen <[email protected]> wrote:
> On 5/19/07, Ray Lee <[email protected]> wrote:
> > 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.
>
> The real solution is to have the user space tool to create these
> device nodes in advance.

Maybe. But requiring people upgrade their userspace tools or setup for
2.6.22 isn't a reasonable solution.

> 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.

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
compatible with how things are being done in 2.6.21 and earlier.

> However, the code was removed by Al in this commit:

> commit 07002e995638b83a6987180f43722a0eb39d4932
> Author: Al Viro <[email protected]>
> Date: Sat May 12 16:23:15 2007 -0400

> fix the dynamic allocation and probe in loop.c

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.

2007-05-21 08:05:09

by Uwe Bugla

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Am Montag, 21. Mai 2007 08:40 schrieben Sie:
> On 5/20/07, Ken Chen <[email protected]> wrote:
> > On 5/19/07, Ray Lee <[email protected]> wrote:
> > > 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.
> >
> > The real solution is to have the user space tool to create these
> > device nodes in advance.
>
> Maybe. But requiring people upgrade their userspace tools or setup for
> 2.6.22 isn't a reasonable solution.
>
> > 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.
>
> 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
> compatible with how things are being done in 2.6.21 and earlier.
>
> > However, the code was removed by Al in this commit:
> >
> > commit 07002e995638b83a6987180f43722a0eb39d4932
> > Author: Al Viro <[email protected]>
> > Date: Sat May 12 16:23:15 2007 -0400
> >
> > fix the dynamic allocation and probe in loop.c
>
> 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.

Yes. I reverted three in fact. Please see my latest entry in this thread to
read what I did to make this work for now.

>
> 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. 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

2007-05-21 16:19:01

by Linus Torvalds

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image



On Sun, 20 May 2007, Kay Sievers wrote:
>
> Right, providing "preallocated" devices, 8 or the number given in
> max_loop, sounds like the best option until the tools can handle that.

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

2007-05-21 16:28:26

by Ken Chen

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/21/07, Linus Torvalds <[email protected]> wrote:
> 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?

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.

2007-05-21 16:35:26

by Ray Lee

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/21/07, Ken Chen <[email protected]> wrote:
> On 5/21/07, Linus Torvalds <[email protected]> wrote:
> > I don't have much choice. I assume it is
> >
> > commit 73285082 "remove artificial software max_loop limit"
> >
> > that introduced the new behaviour. Ken?
>
> 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.

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
to fix the problem.

> Nevertheless, yes, I'm guilty of introducing the new behavior.

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.

2007-05-21 16:37:43

by Ken Chen

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/21/07, Ken Chen <[email protected]> wrote:
> 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.

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.

2007-05-21 16:56:08

by Uwe Bugla

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Am Montag, 21. Mai 2007 18:37 schrieben Sie:
> On 5/21/07, Ken Chen <[email protected]> wrote:
> > 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.
>
> 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.

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
with udev?

>
> Is this acceptable? Patch in a bit.

2007-05-21 17:10:01

by Linus Torvalds

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image



On Mon, 21 May 2007, Ken Chen wrote:
>
> 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.

That sounds perfect.

Linus

2007-05-21 17:13:24

by Kay Sievers

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Mon, 2007-05-21 at 18:50 +0200, Uwe Bugla wrote:
> Am Montag, 21. Mai 2007 18:37 schrieben Sie:
> > On 5/21/07, Ken Chen <[email protected]> wrote:
> > > 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.
> >
> > 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.
>
> 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
> with udev?

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

2007-05-21 17:52:04

by Andrey Borzenkov

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Monday 21 May 2007, Kay Sievers wrote:
> On Mon, 2007-05-21 at 18:50 +0200, Uwe Bugla wrote:
> > Am Montag, 21. Mai 2007 18:37 schrieben Sie:
> > > On 5/21/07, Ken Chen <[email protected]> wrote:
> > > > 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.
> > >
> > > 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.
> >
> > 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 with udev?
>
> 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().
>

Except as already mentioned it will introduce races between tool requesting
new device and udev creating new node. We already had this with raw devices.
My comparison with /dev/pts was incorrect because it is using internal
filesystem that creates devices synchronously.

May be all such cases has to be converted to use common file system.

mount -t nodefs -o device=pts none /dev/pts
mount -t nodefs -o device=loop none /dev/loop


Attachments:
(No filename) (2.21 kB)
(No filename) (189.00 B)
Download all attachments

2007-05-21 20:48:44

by Ken Chen

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On 5/21/07, Ken Chen <[email protected]> wrote:
> 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.
>

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 <[email protected]>

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 underlying device
+ * structure on-demand, provided that there is an access dev node.
+ * However, this will not work well with user space tool that doesn't
+ * know about such "feature". In order to not break any existing
+ * tool, we do the following:
+ *
+ * (1) if max_loop is specified, create that many upfront, and this
+ * also becomes a hard limit.
+ * (2) if max_loop is not specified, create 8 loop device on module
+ * load, user can further extend loop device by create dev node
+ * themselves and have kernel automatically instantiate actual
+ * device on-demand.
+ */
+ if (max_loop) {
+ nr = max_loop;
+ range = max_loop;
+ } else {
+ nr = 8;
+ range = 1UL << MINORBITS;
+ }
+
+ if (register_blkdev(LOOP_MAJOR, "loop"))
+ return -EIO;
+ blk_register_region(MKDEV(LOOP_MAJOR, 0), range,
+ THIS_MODULE, loop_probe, NULL, NULL);
+
+ for (i = 0; i < nr; i++) {
+ if (!loop_init_one(i))
+ goto err;
+ }
+
+ printk(KERN_INFO "loop: module loaded\n");
+ return 0;
+err:
+ loop_exit();
+ printk(KERN_INFO "loop: out of memory\n");
+ return -ENOMEM;
+}
+
module_init(loop_init);
module_exit(loop_exit);


Attachments:
(No filename) (3.73 kB)
loop_default_8.patch (2.61 kB)
Download all attachments

2007-05-21 21:26:00

by Uwe Bugla

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Am Montag, 21. Mai 2007 22:48 schrieben Sie:
> On 5/21/07, Ken Chen <[email protected]> wrote:
> > 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.
>
> 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 <[email protected]>
>
> 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 underlying device
> + * structure on-demand, provided that there is an access dev node.
> + * However, this will not work well with user space tool that doesn't
> + * know about such "feature". In order to not break any existing
> + * tool, we do the following:
> + *
> + * (1) if max_loop is specified, create that many upfront, and this
> + * also becomes a hard limit.
> + * (2) if max_loop is not specified, create 8 loop device on module
> + * load, user can further extend loop device by create dev node
> + * themselves and have kernel automatically instantiate actual
> + * device on-demand.
> + */
> + if (max_loop) {
> + nr = max_loop;
> + range = max_loop;
> + } else {
> + nr = 8;
> + range = 1UL << MINORBITS;
> + }
> +
> + if (register_blkdev(LOOP_MAJOR, "loop"))
> + return -EIO;
> + blk_register_region(MKDEV(LOOP_MAJOR, 0), range,
> + THIS_MODULE, loop_probe, NULL, NULL);
> +
> + for (i = 0; i < nr; i++) {
> + if (!loop_init_one(i))
> + goto err;
> + }
> +
> + printk(KERN_INFO "loop: module loaded\n");
> + return 0;
> +err:
> + loop_exit();
> + printk(KERN_INFO "loop: out of memory\n");
> + return -ENOMEM;
> +}
> +
> module_init(loop_init);
> module_exit(loop_exit);

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

2007-05-21 22:09:34

by Andrew Morton

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Mon, 21 May 2007 23:20:54 +0200
Uwe Bugla <[email protected]> wrote:

> 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.

Yes, please send out a fresh bug report for this.

2007-05-22 00:11:00

by Al Viro

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Mon, May 21, 2007 at 09:11:02AM -0700, Linus Torvalds wrote:
>
>
> On Sun, 20 May 2007, Kay Sievers wrote:
> >
> > Right, providing "preallocated" devices, 8 or the number given in
> > max_loop, sounds like the best option until the tools can handle that.
>
> Yes. Can somebody who actually _uses_ loop send a tested patch, please?

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.

2007-05-22 00:13:47

by Al Viro

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

On Tue, May 22, 2007 at 01:10:38AM +0100, Al Viro wrote:
> On Mon, May 21, 2007 at 09:11:02AM -0700, Linus Torvalds wrote:
> >
> >
> > On Sun, 20 May 2007, Kay Sievers wrote:
> > >
> > > Right, providing "preallocated" devices, 8 or the number given in
> > > max_loop, sounds like the best option until the tools can handle that.
> >
> > Yes. Can somebody who actually _uses_ loop send a tested patch, please?
>
> 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().

2007-05-22 20:27:12

by Jan Engelhardt

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image


On May 20 2007 07:28, Al Viro wrote:
>On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
>> 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.
>
>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?

"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
--

2007-05-22 21:40:42

by Uwe Bugla

[permalink] [raw]
Subject: Re: bug in 2.6.22-rc2: loop mount limited to one single iso image

Am Dienstag, 22. Mai 2007 22:18 schrieben Sie:
> On May 20 2007 07:28, Al Viro wrote:
> >On Sat, May 19, 2007 at 11:16:59PM -0700, Ray Lee wrote:
> >> 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.
> >
> >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?
>
> "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 / appreciate discussions on items that seem to be far away
from actual user's needs, without being proven by examples of real necessity.

Otherwise expressed: Who really needs actually to mount up to 256 iso images
using the loop device? I am ready to learn...... :)

3. I have seen this feature of "virtual DVD devices" under Windoze XP,
established for whatever nonsense reason. Question: Who actually needs to
mount parallely for instance up to 200 video DVDs wasting Ram like hell?

I would call that a kind of fetish, i. e. I do not see the deeper sense of it!

The base line for linux as an operating system is: "Small and effective is
Beautiful", isn't it?