2007-07-24 11:04:03

by Girish Shilamkar

[permalink] [raw]
Subject: [Patch 4/13] Allow regular files to be preallocated on-disk.

Allow regular files to be preallocated on-disk up to the next multiple
of the system PAGE_SIZE without complaining about extra blocks.

Signed-off-by: Andreas Dilger <[email protected]>
Signed-off-by: Girish Shilamkar <[email protected]>
Signed-off-by: Kalpak Shah <[email protected]>

Index: e2fsprogs-1.40.1/e2fsck/pass1.c
===================================================================
--- e2fsprogs-1.40.1.orig/e2fsck/pass1.c
+++ e2fsprogs-1.40.1/e2fsck/pass1.c
@@ -1591,9 +1591,14 @@ static void check_blocks(e2fsck_t ctx, s
bad_size = 2;
}
} else {
+ e2_blkcnt_t blkpg = getpagesize() / fs->blocksize;
+
size = EXT2_I_SIZE(inode);
if ((pb.last_block >= 0) &&
- (size < (__u64) pb.last_block * fs->blocksize))
+ /* allow allocated blocks to end of PAGE_SIZE */
+ (size < (__u64)pb.last_block * fs->blocksize) &&
+ (pb.last_block / blkpg * blkpg != pb.last_block ||
+ size < (__u64)(pb.last_block & ~(blkpg-1)) *fs->blocksize))
bad_size = 3;
else if (size > ext2_max_sizes[fs->super->s_log_block_size])
bad_size = 4;


2007-08-04 06:02:20

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [Patch 4/13] Allow regular files to be preallocated on-disk.

On Tue, Jul 24, 2007 at 04:34:48PM +0530, Girish Shilamkar wrote:
> Allow regular files to be preallocated on-disk up to the next multiple
> of the system PAGE_SIZE without complaining about extra blocks.

Applied, with some fixups.

> } else {
> + e2_blkcnt_t blkpg = getpagesize() / fs->blocksize;

The getpagesize() call was marked LEGACY in SuSv2, and it was dropped
in POSIX.1-2001. It's much better to use sysconf(_SC_PAGESIZE).

Also, the patch will cause e2fsck to core dump if it is asked to check
a filesystem where fs->blocksize is > page_size. So in that case, we
have to set blkpg to 1.

Finally, it's a bad idea to call getpagesize() for every single inode,
since potentially getpagesize() or sysconf(_SC_PAGESIZE) could
potentially require a system call. So this should be factored out and
stashed away in the e2fsck context structure.

I'm also a little concerned this patch doesn't solve the problem where
a filesystem that had been previously mounted on an IA64 machine gets
connected to an x86 machine and then e2fsck is run. This patch
doesn't make the problem any more worse, and to fix this for real
would require stashing the largest blocksize ever used by the
filesystem in the superblock, and I'm not entirely convinced it's
worth it.

- Ted

2007-08-04 16:19:44

by Andreas Dilger

[permalink] [raw]
Subject: Re: [Patch 4/13] Allow regular files to be preallocated on-disk.

On Aug 03, 2007 20:40 -0400, Theodore Tso wrote:
> I'm also a little concerned this patch doesn't solve the problem where
> a filesystem that had been previously mounted on an IA64 machine gets
> connected to an x86 machine and then e2fsck is run. This patch
> doesn't make the problem any more worse, and to fix this for real
> would require stashing the largest blocksize ever used by the
> filesystem in the superblock, and I'm not entirely convinced it's
> worth it.

At worst we get some warnings in that case and the extra blocks beyond
EOF are removed.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

2007-08-04 20:38:59

by Theodore Ts'o

[permalink] [raw]
Subject: Re: [Patch 4/13] Allow regular files to be preallocated on-disk.

On Sat, Aug 04, 2007 at 10:19:43AM -0600, Andreas Dilger wrote:
> On Aug 03, 2007 20:40 -0400, Theodore Tso wrote:
> > I'm also a little concerned this patch doesn't solve the problem where
> > a filesystem that had been previously mounted on an IA64 machine gets
> > connected to an x86 machine and then e2fsck is run. This patch
> > doesn't make the problem any more worse, and to fix this for real
> > would require stashing the largest blocksize ever used by the
> > filesystem in the superblock, and I'm not entirely convinced it's
> > worth it.
>
> At worst we get some warnings in that case and the extra blocks beyond
> EOF are removed.

Yep, that's why I said it's probably not worth it to worry about that
case. It still bothers me a little, but that's the perfectionist in
me struggling to get out. :-)

- Ted