Skip to contentSkip to conversation

loop device broken in 2.4.6-pre5

DISCUSSION [email protected] 4 participants 5 messages Jun 26, 2001 – Jun 27, 2001

lore
Share & tools
lore
#0 lore raw
    From [email protected] Tue Jun 26 10:20:51 2001

    This patch fixes the problem. Please consider applying.

    --- linux-2.4.6-pre5/drivers/block/loop.c    Sat Jun 23 07:52:39 2001
    +++ linux/drivers/block/loop.c    Tue Jun 26 09:21:47 2001
    @@ -653,7 +653,7 @@
         bs = 0;
         if (blksize_size[MAJOR(lo_device)])
             bs = blksize_size[MAJOR(lo_device)][MINOR(lo_device)];
    -    if (!bs)
    +    if (!bs || S_ISREG(inode->i_mode))
             bs = BLOCK_SIZE;
     
         set_blocksize(dev, bs);

But why 1024? Next week your neighbour comes and has a file-backed loop device with an odd number of 512-byte sectors.
If you want a guarantee, then I suppose one should pick 512. (Or make the set blocksize ioctl also work on loop devices.)

Andries

#1 lore raw
Quoted 16 lines · [email protected] wrote:
    From [email protected] Tue Jun 26 10:20:51 2001

    This patch fixes the problem. Please consider applying.

    --- linux-2.4.6-pre5/drivers/block/loop.c    Sat Jun 23 07:52:39 2001
    +++ linux/drivers/block/loop.c    Tue Jun 26 09:21:47 2001
    @@ -653,7 +653,7 @@
         bs = 0;
         if (blksize_size[MAJOR(lo_device)])
             bs = blksize_size[MAJOR(lo_device)][MINOR(lo_device)];
    -    if (!bs)
    +    if (!bs || S_ISREG(inode->i_mode))
             bs = BLOCK_SIZE;
     
         set_blocksize(dev, bs);
Quoted 4 lines

But why 1024? Next week your neighbour comes and has a file-backed loop device with an odd number of 512-byte sectors.
If you want a guarantee, then I suppose one should pick 512. (Or make the set blocksize ioctl also work on loop devices.)

I thought the change was a "quick hack" that would make stuff work (page cache?) near the end of the file. That would mean that this kind of "quick hack" won't work.

But if it does anyway, then indeed 512 would be a more appropriate choice.

I thought I had to convince people, so I chose a sitation that I hoped people would understand to be likely/possible, to prevent reactions:
"No filesystem will use the last odd numbered 512bytes of a partition".

Roger.

Signature · 4 lines
** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2137555 **
*-- BitWizard writes Linux device drivers for any device you may have! --*
* There are old pilots, and there are bold pilots. 
* There are also old, bald pilots. 
#2 lore raw
In reply to [email protected]
Quoted 16 lines · [email protected] wrote:
    From [email protected] Tue Jun 26 10:20:51 2001

    This patch fixes the problem. Please consider applying.

    --- linux-2.4.6-pre5/drivers/block/loop.c    Sat Jun 23 07:52:39 2001
    +++ linux/drivers/block/loop.c    Tue Jun 26 09:21:47 2001
    @@ -653,7 +653,7 @@
         bs = 0;
         if (blksize_size[MAJOR(lo_device)])
             bs = blksize_size[MAJOR(lo_device)][MINOR(lo_device)];
    -    if (!bs)
    +    if (!bs || S_ISREG(inode->i_mode))
             bs = BLOCK_SIZE;

         set_blocksize(dev, bs);
Quoted 4 lines

But why 1024? Next week your neighbour comes and has a file-backed loop device with an odd number of 512-byte sectors.
If you want a guarantee, then I suppose one should pick 512. (Or make the set blocksize ioctl also work on loop devices.)

Because 1024 was the previous default. Keeping the same default that was used before offers least surprises to users and tools.

Regards,
Jari Ruusu <[email protected]>

#3 lore raw
Quoted 17 lines · Jari Ruusu
Quoted 16 lines · [email protected] wrote:
    From [email protected] Tue Jun 26 10:20:51 2001

    This patch fixes the problem. Please consider applying.

    --- linux-2.4.6-pre5/drivers/block/loop.c    Sat Jun 23 07:52:39 2001
    +++ linux/drivers/block/loop.c    Tue Jun 26 09:21:47 2001
    @@ -653,7 +653,7 @@
         bs = 0;
         if (blksize_size[MAJOR(lo_device)])
             bs = blksize_size[MAJOR(lo_device)][MINOR(lo_device)];
    -    if (!bs)
    +    if (!bs || S_ISREG(inode->i_mode))
             bs = BLOCK_SIZE;

         set_blocksize(dev, bs);
Quoted 7 lines
Quoted 4 lines

But why 1024? Next week your neighbour comes and has a file-backed loop device with an odd number of 512-byte sectors.
If you want a guarantee, then I suppose one should pick 512. (Or make the set blocksize ioctl also work on loop devices.)

Because 1024 was the previous default. Keeping the same default that was used before offers least surprises to users and tools.

But also makes it not work for odd number of sectors which is much worse IMHO.

Also it is far more surprising to find that the last sector is lost silently than to have a difference in behaviour, incorrect behaviour needs to be corrected not kept for backwards compatibility till the end of time. And the sooner that happens, the better. Both people and utilities will get used to it. Due to that 1024, mkntfs has to mark the disk dirty because it can never be sure just how many sectors there really are and hence can't be sure whether the backup boot sector was written to the correct place or not...

Also, the loop device is currently not consistent with the rest of the kernel:

* kernel physical block device: get_nr_sectors_sys_call returns the real number of 512 byte sectors
* file mounted on loop device: sam sys_call returns the number of sectors & ~1.

This difference in behaviour causes a much bigger surprise than anything else if we are talking about surprises!

Best regards,

Anton

Signature · 5 lines
  "Nothing succeeds like success." - Alexandre Dumas
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Linux NTFS Maintainer / WWW: http://sf.net/projects/linux-ntfs/
ICQ: 8561279 / WWW: http://www-stu.christs.cam.ac.uk/~aia21/
#4 lore raw
In reply to [email protected]
Quoted 36 lines · Anton Altaparmakov
Quoted 17 lines · Jari Ruusu
Quoted 16 lines · [email protected] wrote:
    From [email protected] Tue Jun 26 10:20:51 2001

    This patch fixes the problem. Please consider applying.

    --- linux-2.4.6-pre5/drivers/block/loop.c    Sat Jun 23 07:52:39 2001
    +++ linux/drivers/block/loop.c    Tue Jun 26 09:21:47 2001
    @@ -653,7 +653,7 @@
         bs = 0;
         if (blksize_size[MAJOR(lo_device)])
             bs = blksize_size[MAJOR(lo_device)][MINOR(lo_device)];
    -    if (!bs)
    +    if (!bs || S_ISREG(inode->i_mode))
             bs = BLOCK_SIZE;

         set_blocksize(dev, bs);
Quoted 7 lines
Quoted 4 lines

But why 1024? Next week your neighbour comes and has a file-backed loop device with an odd number of 512-byte sectors.
If you want a guarantee, then I suppose one should pick 512. (Or make the set blocksize ioctl also work on loop devices.)

Because 1024 was the previous default. Keeping the same default that was used before offers least surprises to users and tools.

But also makes it not work for odd number of sectors which is much worse IMHO.

Also it is far more surprising to find that the last sector is lost silently than to have a difference in behaviour, incorrect behaviour needs to be corrected not kept for backwards compatibility till the end of time. And the sooner that happens, the better. Both people and utilities will get used to it. Due to that 1024, mkntfs has to mark the disk dirty because it can never be sure just how many sectors there really are and hence can't be sure whether the backup boot sector was written to the correct place or not...

Quoted 8 lines

Also, the loop device is currently not consistent with the rest of the kernel:

* kernel physical block device: get_nr_sectors_sys_call returns the real number of 512 byte sectors
* file mounted on loop device: sam sys_call returns the number of sectors & ~1.

This difference in behaviour causes a much bigger surprise than anything else if we are talking about surprises!

OK. My original complaint was that the default could be larger than 1024. I am happy with 512.

Regards,
Jari Ruusu <[email protected]>