All of lore.kernel.org
 help / color / mirror / Atom feed
* bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year !
@ 2007-01-21 17:16 devzero
  2007-01-22 10:00 ` Bryn M. Reeves
  0 siblings, 1 reply; 6+ messages in thread
From: devzero @ 2007-01-21 17:16 UTC (permalink / raw)
  To: agk, breeves; +Cc: dm-devel

hello !

if these is the most recent versions of dm-loop patch (don`t actually know for sure, see my last mail):

http://kernel.org/pub/linux/kernel/people/agk/patches/2.6/editing/dm-loop.patch
http://kernel.org/pub/linux/kernel/people/agk/patches/2.6/editing/dm-add-loop.patch

then i may have found some bug, i`d like to report.

i was doing some testing of dm-loop with latest dmsetup on a 2.6.20-rc5 system.

i did some basic test, which seems to work ok.

i can now map a file to become /dev/mapper/loop0

dmlosetup loop0 test.img 

-> device-mapper: loop: Finalized extent map of 1232 bytes, 44 entries.

root@localhost ~ # ls -la /dev/mapper/loop0
brw------- 1 root root 252, 0 21. Jan 17:25 /dev/mapper/loop0

works !

hooray - great work !


while fiddling around a little bit, i accidentally tried to map that file a second time - anyway, i would expect this is quite ok and should work.

dmlosetup loop1 test.img 

this bails out with the following error, leaving trace in dmesg:  

device-mapper: loop: file is already in use: /root/device-mapper/dmsetup/test.dat
BUG: unable to handle kernel NULL pointer dereference at virtual address 00000000
 printing eip:
d097589c
*pde = 00000000
Oops: 0000 [#1]
PREEMPT SMP 
Modules linked in: dm_loop ipv6 snd_ens1371 gameport snd_rawmidi snd_ac97_codec ac97_bus pcnet32 i2c_piix4 i2c_core agpgart
CPU:    0
EIP:    0060:[<d097589c>]    Not tainted VLI
EFLAGS: 00010292   (2.6.20-rc5 #1)
EIP is at loop_setup_size+0x1c/0x1f0 [dm_loop]
eax: 00000000   ebx: cb3289c0   ecx: d097105c   edx: d0971020
esi: ffffffea   edi: 00000001   ebp: d0971020   esp: cad47e58
ds: 007b   es: 007b   ss: 0068
Process dmlosetup (pid: 1787, ti=cad46000 task=cfe13aa0 task.ti=cad46000)
Stack: c018ab46 00000008 cd08fbc0 c017a806 00000000 00000000 cfec9ee8 cfec9ec0 
       c018fcd3 cd4bc180 cacbe118 d097105c d0971020 cb3289c0 cb3289c0 ffffffea 
       00000001 d0971020 d0975bd3 d0978181 d09761b5 cb3289f8 cb3280c0 d0971020 
Call Trace:
 [<c018ab46>] dput+0x86/0x140
 [<c017a806>] __fput+0xf6/0x170
 [<c018fcd3>] mntput_no_expire+0x13/0x70
 [<d0975bd3>] loop_ctr+0xd3/0x170 [dm_loop]
 [<c049016b>] dm_table_add_target+0x12b/0x1e0
 [<c04928c0>] populate_table+0x80/0xe0
 [<c049297e>] table_load+0x5e/0x110
 [<c0492920>] table_load+0x0/0x110
 [<c04930db>] ctl_ioctl+0xcb/0x130
 [<c0534ebc>] __up+0x1c/0x20
 [<c018595a>] do_ioctl+0x6a/0xa0
 [<c0185b1e>] vfs_ioctl+0x5e/0x1d0
 [<c0185ccd>] sys_ioctl+0x3d/0x70
 [<c0103252>] sysenter_past_esp+0x5f/0x85
 =======================
Code: d0 e8 f9 19 7b ef eb b3 8d b4 26 00 00 00 00 55 57 56 53 83 ec 38 89 44 24 34 89 54 24 30 89 4c 24 2c 8b 40 04 8b 80 8c 00 00 00 <8b> 18 8b 4b 44 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90 90 89 
EIP: [<d097589c>] loop_setup_size+0x1c/0x1f0 [dm_loop] SS:ESP 0068:cad47e58
 
after this, if i do 
dmlosetup -d loop1
dmlosetup -d loop0

i cannot "rmmod dm-loop" , now telling me "device is in use", but it isn`t ! 

so, this looks like a bug to me, leaving dm-loop in an inconsitent state.


furthermore, one question about naming conventions :

dmlosetup testloop1 test.dat
dmlosetup: Could not parse loop_device testloop1
Usage:

losetup [-d|-a] [-e encryption] [-o offset] [-f|loop_device] [file]

Couldn't process command line.


so, it`s mandatory that loop_device needs to begin with "loop...." !?

regards
roland



> -----Ursprüngliche Nachricht-----
> Von: devzero@web.de
> Gesendet: 20.01.07 15:57:23
> An: breeves@redhat.com
> CC: dm-devel@redhat.com
> Betreff: Re: Re: device mapper integrated loops - and one more year !


> Hello Bryn, 
> 
> 
> >I'll see if we can get a revised version of the current patch for wider
> >testing in the next couple of weeks - let me know if you would be
> >interested in trying this out.
> 
> not sure if i sent a reply to this or have missed this -  but, yes - i`m still interested very much in using dm-loop !
> 
> is there anything new with this?
> do i have to expect issues with files >2gb ? (i need to loopback mount dvd iso-images)
> 
> is there a download url for the latest dm-loop patch so i can do some tests and report feedback ?
> 
> i couldn`t find it at http://sources.redhat.com/cgi-bin/cvsweb.cgi/device-mapper/?cvsroot=dm  and i`m not sure if i should take that one from http://kernel.org/pub/linux/kernel/people/agk/patches/2.6/
> 
> TIA
> roland
> 
> 
> 
> > Hello !
> > 
> > after desparately seeking for a solution to have more than 256 loop
> > devices (for a huge cd-rom server), i have recently come across
> > dm-loop, which looks very promising (to replace loop.c)
> > 
> > can i have more than 256 devices which this?
> 
> Hi Roland,
> 
> Yes, dm-loop will support as many loop devices as device-mapper can allow.
> 
> > our cd-rom server is going to have more than 256 iso-images soon and
> > i need to upgrade to a more recent OS, anyway, so this won`t be a 
> > problem that dm-loop is so brand new.
> > 
> > but - any timeline for dm-loop mainline inclusion ?
> > 
> > should i wait for 2.6.19 or 2.6.20 ?
> 
> There isn't a fixed timeline. We are currently working to improve the
> lookup code - the prototype patch uses a linear search which doesn't
> scale well for large/fragmented image files. I have some tests running
> at the moment and we hope to have something soon - the results so far
> are good, it's just a case of selecting between a couple of different
> methods.
> 
> The changes dm-loop requires in device-mapper core are pretty minor and
> have already been merged in 2.6.19, so it's just the dm-loop patch
> itself remaining.
> 
> > is it already useable with less recent kernels, i.e. can i download
> > dm-loop patch and recent dmsetup tool and use with older kernel ?
> 
> This should work fine - there are a couple of known issues with the
> existing patch, but unless you are using large images (>2G), or have a
> severely fragmented filesystem you shouldn't run into these.
> 
> I'll see if we can get a revised version of the current patch for wider
> testing in the next couple of weeks - let me know if you would be
> interested in trying this out.
> 
> Thanks,
> 
> Bryn.


_____________________________________________________________________
Der WEB.DE SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
http://smartsurfer.web.de/?mc=100071&distributionid=000000000066

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year !
  2007-01-21 17:16 bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year ! devzero
@ 2007-01-22 10:00 ` Bryn M. Reeves
  2007-01-22 22:00   ` roland
  0 siblings, 1 reply; 6+ messages in thread
From: Bryn M. Reeves @ 2007-01-22 10:00 UTC (permalink / raw)
  To: devzero; +Cc: dm-devel, agk

[-- Attachment #1: Type: text/plain, Size: 2756 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

devzero@web.de wrote:
> 
> i was doing some testing of dm-loop with latest dmsetup on a 2.6.20-rc5 system.
> 
> i did some basic test, which seems to work ok.
> 
> i can now map a file to become /dev/mapper/loop0
> 
> dmlosetup loop0 test.img 
> 
> -> device-mapper: loop: Finalized extent map of 1232 bytes, 44 entries.
> 
> root@localhost ~ # ls -la /dev/mapper/loop0
> brw------- 1 root root 252, 0 21. Jan 17:25 /dev/mapper/loop0

Great! Am glad this much is working for you!

> 
> while fiddling around a little bit, i accidentally tried to map that file a second time - anyway, i would expect this is quite ok and should work.
> 
> dmlosetup loop1 test.img 
> 
> this bails out with the following error, leaving trace in dmesg:  
> 
> device-mapper: loop: file is already in use: /root/device-mapper/dmsetup/test.dat
> BUG: unable to handle kernel NULL pointer dereference at virtual address 00000000
>  printing eip:
> d097589c
> *pde = 00000000
> Oops: 0000 [#1]

The dm-loop target only allows a file to be mapped once. That said, it
should just return the error, not oops the kernel!

This is fixed in my CVS copy of this patch - I'm attaching a patch that
you can apply on top of the one that you already have that includes this
fix plus a couple of other minor fixes/enhancements.

I'll also get the version on kernel.org updated - thanks for catching this!

> after this, if i do 
> dmlosetup -d loop1
> dmlosetup -d loop0
> 
> i cannot "rmmod dm-loop" , now telling me "device is in use", but it isn`t ! 
> 
> so, this looks like a bug to me, leaving dm-loop in an inconsitent state.

Unfortunately, after an oops in a kernel module you're pretty much
forced to reboot - the kernel terminates the code path that caused the
fault, leaving a hanging reference count on the module.

> furthermore, one question about naming conventions :
> 
> dmlosetup testloop1 test.dat
> dmlosetup: Could not parse loop_device testloop1
> Usage:
> 
> losetup [-d|-a] [-e encryption] [-o offset] [-f|loop_device] [file]
> 
> Couldn't process command line.
> 
> 
> so, it`s mandatory that loop_device needs to begin with "loop...." !?
> 

It is at the moment :) The current command line parameters try to be as
faithful as possible to the nomrmal losetup, where devices are normally
named as (/dev/)loopN. We can't exactly match that without disrupting
users of the regular loop driver though, so this should probably be relaxed.

Thanks for testing!

Bryn.





-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFFtItQ6YSQoMYUY94RAijWAJ47OJL21+0W3yiMqbq51k3EHAw0WACfaefN
PjSa8djCMZuyZ/gGyUAikp0=
=3bv3
-----END PGP SIGNATURE-----

[-- Attachment #2: dm-loop.fix_busy_image.patch --]
[-- Type: text/x-patch, Size: 2834 bytes --]

--- drivers/md/dm-loop.c.rel	2007-01-22 09:45:04.000000000 +0000
+++ drivers/md/dm-loop.c	2007-01-22 09:45:10.000000000 +0000
@@ -22,7 +22,7 @@
 #include "dm-bio-record.h"
 
 #define DM_MSG_PREFIX "loop"
-#define LOOP_MAX_EXTENTS 1024
+#define LOOP_MAX_EXTENTS 2048
 
 #define	DMLOOP_READONLY	0x01
 #define	DMLOOP_SYNC	0x02
@@ -69,7 +69,7 @@ struct loop_c {
 };
 
 #ifdef CONFIG_DM_DEBUG
-static void dump_extent(struct extent *e)
+static void dump_extent(unsigned i, struct extent *e)
 {
 	const char types[] = { 'f', 'd' };
 
@@ -81,8 +81,8 @@ static void dump_extent(struct extent *e
 		return;
 	}
 
-	DMDEBUG("start: %8llu len: %4llu %4c.rstart: %8llu",
-				e->start, e->len, types[e->type],
+	DMDEBUG(" [%u] start: %8llu len: %4llu %4c.rstart: %8llu",
+				i, e->start, e->len, types[e->type],
 				(sector_t)e->data );
 }
 
@@ -97,7 +97,7 @@ static void dump_extent_map(struct exten
 			map->nr_extents, map->cur_extent);
 
 	for (i = 0; i < map->nr_extents; i++)
-		dump_extent(DMLOOP_EXTENT(i));
+		dump_extent(i, DMLOOP_EXTENT(i));
 }
 
 #else /* CONFIG_DM_DEBUG */
@@ -228,16 +228,25 @@ reprobe:
 		continue;
 	}
 
+	if (nr_extents == LOOP_MAX_EXTENTS && probe_block < last_block){
+		DMERR("Loopfile contains more than LOOP_MAX_EXTENTS (%d) fragments\n",
+					LOOP_MAX_EXTENTS);
+		goto out_free;
+	}
+
 	map->nr_extents = nr_extents;
 	map->cur_extent = 0;
 
 	DMDEBUG("created initial extent map, finalizing.");
 	map = finalize_map(map);
+	if (!map)
+		return -ENOMEM;
+
+	//dump_extent_map(map);
 	DMINFO("Finalized extent map of %u bytes, %d entries.",
 			(map->nr_extents * sizeof(struct extent)),
 			map->nr_extents);
 
-	dump_extent_map(map);
 	lc->blkbits = blkbits;
 	lc->map = map;
 
@@ -408,7 +417,7 @@ static struct file *loop_get_file(char *
 {
 	struct file *filp;
 	struct inode *inode;
-	int r;
+	int r = 0;
 
 	filp = filp_open(loop_path,
 			 ((*flags & DMLOOP_READONLY) ? O_RDONLY : O_RDWR) |
@@ -434,6 +443,7 @@ static struct file *loop_get_file(char *
 
 	if (IS_SWAPFILE(inode)) {
 		DMERR("file is already in use: %s", loop_path);
+		r = -EBUSY;
 		goto out;
 	}
 
@@ -461,7 +471,7 @@ static int loop_setup_size(struct loop_c
 	lc->size = i_size_read(inode);
 	lc->blkbits = inode->i_blkbits;
 
-	if (lc->offset & (1 << lc->blkbits - 1)) {
+	if (lc->offset & ((1 << lc->blkbits) - 1)) {
 		DMERR("Backing file offset of %lld bytes not a multiple of "
 			"filesystem blocksize (%d)", lc->offset,
 			1 << lc->blkbits);
@@ -565,8 +575,11 @@ static int loop_ctr(struct dm_target *ti
 		goto out_putf;
 
 	r = setup_loop_extents(lc);
-	if (r) {
-		ti->error = "Could not create extent map";
+	if (r == -EINVAL) {
+		ti->error = "Could not create extent map - invalid argument";
+		goto out_putf;
+	} else if (r == -ENOMEM) {
+		ti->error = "Could not allocate extent map";
 		goto out_putf;
 	}
 

[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year !
  2007-01-22 10:00 ` Bryn M. Reeves
@ 2007-01-22 22:00   ` roland
  2007-01-22 22:10     ` Bryn M. Reeves
  0 siblings, 1 reply; 6+ messages in thread
From: roland @ 2007-01-22 22:00 UTC (permalink / raw)
  To: Bryn M. Reeves; +Cc: dm-devel, agk

Hi Bryn,

> I'll also get the version on kernel.org updated - thanks for catching 
> this!

thanks very much for that quick patch - it works as expected and the kernel 
oops went away!

since i want to use dm-loop for having a large number of loop-devices, i did 
some more "hardcore testing". ;)

so, i wrote a little test script which created a lot of dummy-files and 
turning them into a dm-loop target.

this worked very well - must have been around 3000 loop-targets when i got 
this one:

# device-mapper: reload ioctl failed: not enough memory available

and in dmesg:

Jan 22 22:42:06 localhost device-mapper: loop: Finalized extent map of 56 
bytes, 2 entries.
Jan 22 22:42:08 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:08 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:09 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:09 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:10 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:10 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:10 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:10 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:10 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:10 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:12 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:13 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:13 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:13 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:13 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0
Jan 22 22:42:13 localhost dmlosetup: page allocation failure. order:4, 
mode:0xd0
Jan 22 22:42:13 localhost  [<c015e1a2>] __alloc_pages+0x272/0x2f0


this was on a system with 256 MB of ram.

i assume, that i ran out of some kernel-buffer memory.

ok, not a real problem for me since i don`t expect having that much number 
of iso images on my cd-roms server to be loopback mounted, but - anyway - 
could you probably give a comment on this ?

is this some "natural limitation" due to dm-loop or device mapper consuming 
kernel-memory, or could this be another bug ?

regards
roland





----- Original Message ----- 
From: "Bryn M. Reeves" <breeves@redhat.com>
To: <devzero@web.de>
Cc: <agk@redhat.com>; <dm-devel@redhat.com>
Sent: Monday, January 22, 2007 11:00 AM
Subject: Re: bug in dm-loop? - was:Re: Re: device mapper integrated loops - 
and one more year !


> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> devzero@web.de wrote:
>>
>> i was doing some testing of dm-loop with latest dmsetup on a 2.6.20-rc5 
>> system.
>>
>> i did some basic test, which seems to work ok.
>>
>> i can now map a file to become /dev/mapper/loop0
>>
>> dmlosetup loop0 test.img
>>
>> -> device-mapper: loop: Finalized extent map of 1232 bytes, 44 entries.
>>
>> root@localhost ~ # ls -la /dev/mapper/loop0
>> brw------- 1 root root 252, 0 21. Jan 17:25 /dev/mapper/loop0
>
> Great! Am glad this much is working for you!
>
>>
>> while fiddling around a little bit, i accidentally tried to map that file 
>> a second time - anyway, i would expect this is quite ok and should work.
>>
>> dmlosetup loop1 test.img
>>
>> this bails out with the following error, leaving trace in dmesg:
>>
>> device-mapper: loop: file is already in use: 
>> /root/device-mapper/dmsetup/test.dat
>> BUG: unable to handle kernel NULL pointer dereference at virtual address 
>> 00000000
>>  printing eip:
>> d097589c
>> *pde = 00000000
>> Oops: 0000 [#1]
>
> The dm-loop target only allows a file to be mapped once. That said, it
> should just return the error, not oops the kernel!
>
> This is fixed in my CVS copy of this patch - I'm attaching a patch that
> you can apply on top of the one that you already have that includes this
> fix plus a couple of other minor fixes/enhancements.
>
> I'll also get the version on kernel.org updated - thanks for catching 
> this!
>
>> after this, if i do
>> dmlosetup -d loop1
>> dmlosetup -d loop0
>>
>> i cannot "rmmod dm-loop" , now telling me "device is in use", but it 
>> isn`t !
>>
>> so, this looks like a bug to me, leaving dm-loop in an inconsitent state.
>
> Unfortunately, after an oops in a kernel module you're pretty much
> forced to reboot - the kernel terminates the code path that caused the
> fault, leaving a hanging reference count on the module.
>
>> furthermore, one question about naming conventions :
>>
>> dmlosetup testloop1 test.dat
>> dmlosetup: Could not parse loop_device testloop1
>> Usage:
>>
>> losetup [-d|-a] [-e encryption] [-o offset] [-f|loop_device] [file]
>>
>> Couldn't process command line.
>>
>>
>> so, it`s mandatory that loop_device needs to begin with "loop...." !?
>>
>
> It is at the moment :) The current command line parameters try to be as
> faithful as possible to the nomrmal losetup, where devices are normally
> named as (/dev/)loopN. We can't exactly match that without disrupting
> users of the regular loop driver though, so this should probably be 
> relaxed.
>
> Thanks for testing!
>
> Bryn.
>
>
>
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.5 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org
>
> iD8DBQFFtItQ6YSQoMYUY94RAijWAJ47OJL21+0W3yiMqbq51k3EHAw0WACfaefN
> PjSa8djCMZuyZ/gGyUAikp0=
> =3bv3
> -----END PGP SIGNATURE-----
>


--------------------------------------------------------------------------------


> --- drivers/md/dm-loop.c.rel 2007-01-22 09:45:04.000000000 +0000
> +++ drivers/md/dm-loop.c 2007-01-22 09:45:10.000000000 +0000
> @@ -22,7 +22,7 @@
> #include "dm-bio-record.h"
>
> #define DM_MSG_PREFIX "loop"
> -#define LOOP_MAX_EXTENTS 1024
> +#define LOOP_MAX_EXTENTS 2048
>
> #define DMLOOP_READONLY 0x01
> #define DMLOOP_SYNC 0x02
> @@ -69,7 +69,7 @@ struct loop_c {
> };
>
> #ifdef CONFIG_DM_DEBUG
> -static void dump_extent(struct extent *e)
> +static void dump_extent(unsigned i, struct extent *e)
> {
>  const char types[] = { 'f', 'd' };
>
> @@ -81,8 +81,8 @@ static void dump_extent(struct extent *e
>  return;
>  }
>
> - DMDEBUG("start: %8llu len: %4llu %4c.rstart: %8llu",
> - e->start, e->len, types[e->type],
> + DMDEBUG(" [%u] start: %8llu len: %4llu %4c.rstart: %8llu",
> + i, e->start, e->len, types[e->type],
>  (sector_t)e->data );
> }
>
> @@ -97,7 +97,7 @@ static void dump_extent_map(struct exten
>  map->nr_extents, map->cur_extent);
>
>  for (i = 0; i < map->nr_extents; i++)
> - dump_extent(DMLOOP_EXTENT(i));
> + dump_extent(i, DMLOOP_EXTENT(i));
> }
>
> #else /* CONFIG_DM_DEBUG */
> @@ -228,16 +228,25 @@ reprobe:
>  continue;
>  }
>
> + if (nr_extents == LOOP_MAX_EXTENTS && probe_block < last_block){
> + DMERR("Loopfile contains more than LOOP_MAX_EXTENTS (%d) fragments\n",
> + LOOP_MAX_EXTENTS);
> + goto out_free;
> + }
> +
>  map->nr_extents = nr_extents;
>  map->cur_extent = 0;
>
>  DMDEBUG("created initial extent map, finalizing.");
>  map = finalize_map(map);
> + if (!map)
> + return -ENOMEM;
> +
> + //dump_extent_map(map);
>  DMINFO("Finalized extent map of %u bytes, %d entries.",
>  (map->nr_extents * sizeof(struct extent)),
>  map->nr_extents);
>
> - dump_extent_map(map);
>  lc->blkbits = blkbits;
>  lc->map = map;
>
> @@ -408,7 +417,7 @@ static struct file *loop_get_file(char *
> {
>  struct file *filp;
>  struct inode *inode;
> - int r;
> + int r = 0;
>
>  filp = filp_open(loop_path,
>  ((*flags & DMLOOP_READONLY) ? O_RDONLY : O_RDWR) |
> @@ -434,6 +443,7 @@ static struct file *loop_get_file(char *
>
>  if (IS_SWAPFILE(inode)) {
>  DMERR("file is already in use: %s", loop_path);
> + r = -EBUSY;
>  goto out;
>  }
>
> @@ -461,7 +471,7 @@ static int loop_setup_size(struct loop_c
>  lc->size = i_size_read(inode);
>  lc->blkbits = inode->i_blkbits;
>
> - if (lc->offset & (1 << lc->blkbits - 1)) {
> + if (lc->offset & ((1 << lc->blkbits) - 1)) {
>  DMERR("Backing file offset of %lld bytes not a multiple of "
>  "filesystem blocksize (%d)", lc->offset,
>  1 << lc->blkbits);
> @@ -565,8 +575,11 @@ static int loop_ctr(struct dm_target *ti
>  goto out_putf;
>
>  r = setup_loop_extents(lc);
> - if (r) {
> - ti->error = "Could not create extent map";
> + if (r == -EINVAL) {
> + ti->error = "Could not create extent map - invalid argument";
> + goto out_putf;
> + } else if (r == -ENOMEM) {
> + ti->error = "Could not allocate extent map";
>  goto out_putf;
>  }
>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year !
  2007-01-22 22:00   ` roland
@ 2007-01-22 22:10     ` Bryn M. Reeves
  2007-01-22 22:42       ` roland
  0 siblings, 1 reply; 6+ messages in thread
From: Bryn M. Reeves @ 2007-01-22 22:10 UTC (permalink / raw)
  To: roland; +Cc: device-mapper development

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

roland wrote:
> Hi Bryn,
> 
>> I'll also get the version on kernel.org updated - thanks for catching
>> this!
> 
> thanks very much for that quick patch - it works as expected and the
> kernel oops went away!

Your very welcome! I'm glad it addressed the problem & it's good to get
some feedback on dm-loop.

> since i want to use dm-loop for having a large number of loop-devices, i
> did some more "hardcore testing". ;)
> 
> so, i wrote a little test script which created a lot of dummy-files and
> turning them into a dm-loop target.
> 
> this worked very well - must have been around 3000 loop-targets when i
> got this one:

That's useful to know - this is a known limitation of the current code.

It simply allocates one big area of memory to store the lookup table for
the device. These kind of large, contiguous allocations are a "bad
thing" in the kernel, as you get failures of the kind you noted when
memory gets tight/fragmented.

> this was on a system with 256 MB of ram.
> 
> i assume, that i ran out of some kernel-buffer memory.
> 
> ok, not a real problem for me since i don`t expect having that much
> number of iso images on my cd-roms server to be loopback mounted, but -
> anyway - could you probably give a comment on this ?
> 
> is this some "natural limitation" due to dm-loop or device mapper
> consuming kernel-memory, or could this be another bug ?
> 

The current development version allocates these structures piecemeal, so
there isn't a single large allocation, rather many small allocations -
this will make problems like this go away (unless you really are out of
memory!).

You're particularly likely to see these problems with large images such
as DVD ISOs, as the number of entries in the lookup table is
proportional to the size of the image and the degree of fragmentation of
the filesystem on which it is stored.

Thanks again for testing!

Bryn.




-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFFtTZV6YSQoMYUY94RAhpAAJ46Bqw+qBEVzapdKZh2V2nbx4AzHACePfsV
ttYf/s/ed3z/cBeCHEE09FY=
=ds+L
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year !
  2007-01-22 22:10     ` Bryn M. Reeves
@ 2007-01-22 22:42       ` roland
  2007-01-22 23:20         ` Bryn M. Reeves
  0 siblings, 1 reply; 6+ messages in thread
From: roland @ 2007-01-22 22:42 UTC (permalink / raw)
  To: Bryn M. Reeves; +Cc: device-mapper development

> The current development version allocates these structures piecemeal, so
> there isn't a single large allocation, rather many small allocations -
> this will make problems like this go away (unless you really are out of
> memory!).
ok, i`ll wait for this.

> You're particularly likely to see these problems with large images such
> as DVD ISOs, as the number of entries in the lookup table is
> proportional to the size of the image and the degree of fragmentation of
> the filesystem on which it is stored.
uuuhhhhh - mhhhh - this sounds bad, since the testfiles i created were all 
sized 100k each.
i think i need to attach a big disk and create some more big files to test 
with....

> Thanks again for testing!
you`re welcome!

thanks for making dm-loop!
:)

roland

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year !
  2007-01-22 22:42       ` roland
@ 2007-01-22 23:20         ` Bryn M. Reeves
  0 siblings, 0 replies; 6+ messages in thread
From: Bryn M. Reeves @ 2007-01-22 23:20 UTC (permalink / raw)
  To: roland; +Cc: device-mapper development

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

roland wrote:
>> You're particularly likely to see these problems with large images such
>> as DVD ISOs, as the number of entries in the lookup table is
>> proportional to the size of the image and the degree of fragmentation of
>> the filesystem on which it is stored.
> uuuhhhhh - mhhhh - this sounds bad, since the testfiles i created were
> all sized 100k each.

With ~3000 of them active concurrently on a machine with 256M of RAM,
I'm not that surprised that you hit this limit!

Again, to keep things simple in the prototype, we grab the biggest
possible table size when we create a new device and then re-allocate it
to the correct size when we finish building the map - that's what the
"Finalized extent map of 56 bytes, 2 entries." messages refer to.

If debugging was enabled when the module was built, you should also see
messages like:

 loop: [DEBUG] setup_loop_extents:Allocated initial extent map of 57352
bytes, 2048 entries.

It's these initial large allocations that are causing the errors you see.

The code's pretty dumb in this area at the moment - improving memory
allocation and the time spent looking up entries in the table are our
current goals for dm-loop.

> i think i need to attach a big disk and create some more big files to
> test with....

The code will fail if it needs >2048 table entries to describe the file
- - depending on the filesystem used and how fragmented it is this will
limit you to files of a few gigabytes. This is another arbitrary limit
in this version that will go away in later dm-loop releases.

Kind regards,

Bryn.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFFtUam6YSQoMYUY94RAlMnAJ9C3fivcmwgcWiA8G5UIULB1BwnPgCgyrIO
n1QfreVTGUNfMHA6bRWdzwI=
=AV8G
-----END PGP SIGNATURE-----

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-01-22 23:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-21 17:16 bug in dm-loop? - was:Re: Re: device mapper integrated loops - and one more year ! devzero
2007-01-22 10:00 ` Bryn M. Reeves
2007-01-22 22:00   ` roland
2007-01-22 22:10     ` Bryn M. Reeves
2007-01-22 22:42       ` roland
2007-01-22 23:20         ` Bryn M. Reeves

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.