2008-07-31 16:11:09

by Aneesh Kumar K.V

[permalink] [raw]
Subject: i_size error with the latest patchqueue

Hi Ted,

I haven't yet looked at whether it is kernel or e2fsck bug.
But I am having this with the latest patch queue when running
fsstress. The fsstress have falloc added as a supported operation.
So it is mostly to do with fallocate. Architecture is x86.

The tests performed are

chown 3/39 write op
creat 4/39 write op
dread 4/39
fdatasync 1/39 write op
fsync 1/39 write op
getdents 1/39
link 1/39 write op
mkdir 2/39 write op
mknod 2/39 write op
read 1/39
readlink 1/39
rename 2/39 write op
rmdir 1/39 write op
stat 1/39
symlink 2/39 write op
sync 1/39
truncate 2/39 write op
unlink 1/39 write op
write 4/39 write op
fallocate 4/39 write op

seed = 1218256896


/usr/local/e2fsprogs/sbin/e2fsck -fnv /dev/sdb1
e2fsck 1.41.0 (10-Jul-2008)
Pass 1: Checking inodes, blocks, and sizes
Inode 10093, i_size is 974599, should be 999424. Fix? no

Inode 22481, i_size is 363927, should be 909312. Fix? no

Inode 28621, i_size is 143236, should be 159744. Fix? no

Inode 28635, i_size is 1203683, should be 1830912. Fix? no

Inode 42764, i_size is 2328377, should be 2691072. Fix? no

Inode 54630, i_size is 204182, should be 331776. Fix? no

Inode 60249, i_size is 0, should be 700416. Fix? no

Inode 67588, i_size is 1621922, should be 1810432. Fix? no

Inode 71084, i_size is 0, should be 184320. Fix? no

Inode 78764, i_size is 0, should be 151552. Fix? no

Inode 92569, i_size is 651967, should be 806912. Fix? no


-aneesh


2008-07-31 16:47:30

by Theodore Ts'o

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

On Thu, Jul 31, 2008 at 09:40:59PM +0530, Aneesh Kumar K.V wrote:
> Hi Ted,
>
> I haven't yet looked at whether it is kernel or e2fsck bug.
> But I am having this with the latest patch queue when running
> fsstress. The fsstress have falloc added as a supported operation.
> So it is mostly to do with fallocate. Architecture is x86.

Part of this is definitely an e2fsck bug, because e2fsck isn't
currently checking the EXT2_EXTENT_FLAGS_UNINIT flag. In line 1715 of
e2fsck/pass1.c:

start_block = pb->last_block = extent.e_lblk + extent.e_len - 1;

We shouldn't be setting pb->last_block if this is an unitialized
extent.

I suspect this is also related though to the issue you raised in the
"e2fsprogs and blocks outside i_size" thread about two weeks ago. So
even after we fix this e2fsck bug, we may still have some fsstress
failures until we decide how to deal with that particular issue.

Do you have a quickie program that calls fallocate to preallocate
space for a particular program easily to hand? If not, I can write
one in about 5-10 minutes, but if you have one written already, it'll
save me the effort. Thanks!!

- Ted

2008-07-31 16:48:46

by Eric Sandeen

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

Aneesh Kumar K.V wrote:
> Hi Ted,
>
> I haven't yet looked at whether it is kernel or e2fsck bug.
> But I am having this with the latest patch queue when running
> fsstress. The fsstress have falloc added as a supported operation.
> So it is mostly to do with fallocate. Architecture is x86.
>
> The tests performed are
>
> chown 3/39 write op
> creat 4/39 write op
> dread 4/39
> fdatasync 1/39 write op
> fsync 1/39 write op
> getdents 1/39
> link 1/39 write op
> mkdir 2/39 write op
> mknod 2/39 write op
> read 1/39
> readlink 1/39
> rename 2/39 write op
> rmdir 1/39 write op
> stat 1/39
> symlink 2/39 write op
> sync 1/39
> truncate 2/39 write op
> unlink 1/39 write op
> write 4/39 write op
> fallocate 4/39 write op
>
> seed = 1218256896
>
>
> /usr/local/e2fsprogs/sbin/e2fsck -fnv /dev/sdb1
> e2fsck 1.41.0 (10-Jul-2008)
> Pass 1: Checking inodes, blocks, and sizes
> Inode 10093, i_size is 974599, should be 999424. Fix? no

are those due to blocks beyond EOF, I assume? How is fallocate being
called in fsstress?

-Eric

2008-07-31 16:54:41

by Eric Sandeen

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

Theodore Tso wrote:
> Do you have a quickie program that calls fallocate to preallocate
> space for a particular program easily to hand? If not, I can write
> one in about 5-10 minutes, but if you have one written already, it'll
> save me the effort. Thanks!!

Ted, I hacked up:
http://sandeen.fedorapeople.org/utilities/fallocate.c
a while ago, I think it still works, at least on x86/x86_64

-Eric

2008-07-31 17:00:03

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

On Thu, Jul 31, 2008 at 12:47:28PM -0400, Theodore Tso wrote:
> On Thu, Jul 31, 2008 at 09:40:59PM +0530, Aneesh Kumar K.V wrote:
> > Hi Ted,
> >
> > I haven't yet looked at whether it is kernel or e2fsck bug.
> > But I am having this with the latest patch queue when running
> > fsstress. The fsstress have falloc added as a supported operation.
> > So it is mostly to do with fallocate. Architecture is x86.
>
> Part of this is definitely an e2fsck bug, because e2fsck isn't
> currently checking the EXT2_EXTENT_FLAGS_UNINIT flag. In line 1715 of
> e2fsck/pass1.c:
>
> start_block = pb->last_block = extent.e_lblk + extent.e_len - 1;
>
> We shouldn't be setting pb->last_block if this is an unitialized
> extent.
>
> I suspect this is also related though to the issue you raised in the
> "e2fsprogs and blocks outside i_size" thread about two weeks ago. So
> even after we fix this e2fsck bug, we may still have some fsstress
> failures until we decide how to deal with that particular issue.
>
> Do you have a quickie program that calls fallocate to preallocate
> space for a particular program easily to hand? If not, I can write
> one in about 5-10 minutes, but if you have one written already, it'll
> save me the effort. Thanks!!

http://lkml.org/lkml/2007/7/13/175

-aneesh

2008-07-31 17:06:27

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

On Thu, Jul 31, 2008 at 11:41:39AM -0500, Eric Sandeen wrote:
> Aneesh Kumar K.V wrote:
> > Hi Ted,
> >
> > I haven't yet looked at whether it is kernel or e2fsck bug.
> > But I am having this with the latest patch queue when running
> > fsstress. The fsstress have falloc added as a supported operation.
> > So it is mostly to do with fallocate. Architecture is x86.
> >
> > The tests performed are
> >
> > chown 3/39 write op
> > creat 4/39 write op
> > dread 4/39
> > fdatasync 1/39 write op
> > fsync 1/39 write op
> > getdents 1/39
> > link 1/39 write op
> > mkdir 2/39 write op
> > mknod 2/39 write op
> > read 1/39
> > readlink 1/39
> > rename 2/39 write op
> > rmdir 1/39 write op
> > stat 1/39
> > symlink 2/39 write op
> > sync 1/39
> > truncate 2/39 write op
> > unlink 1/39 write op
> > write 4/39 write op
> > fallocate 4/39 write op
> >
> > seed = 1218256896
> >
> >
> > /usr/local/e2fsprogs/sbin/e2fsck -fnv /dev/sdb1
> > e2fsck 1.41.0 (10-Jul-2008)
> > Pass 1: Checking inodes, blocks, and sizes
> > Inode 10093, i_size is 974599, should be 999424. Fix? no
>
> are those due to blocks beyond EOF, I assume? How is fallocate being
> called in fsstress?
>

lr = ((__int64_t)random() << 32) + random();
off = (off64_t)(lr % MIN(stb.st_size + (1024 * 1024), MAXFSIZE));
off %= maxfsize;
len = (random() % (getpagesize() * 32)) + 1;
#ifdef __i386__
#define __NR_fallocate 324
e = syscall(__NR_fallocate, fd, 0, off, len) < 0 ? errno : 0;
#elif defined (__powerpc__)
#define __NR_fallocate 309
e = syscall(__NR_fallocate, fd, 0, 0, (unsigned int)off, 0, (unsigned int) len) < 0 ? errno : 0;
#elif defined (__x86_64__)
#define __NR_fallocate 285
e = syscall(__NR_fallocate, fd, 0, off, len) < 0 ? errno : 0;
#endif


2008-07-31 17:09:17

by Aneesh Kumar K.V

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

On Thu, Jul 31, 2008 at 12:47:28PM -0400, Theodore Tso wrote:
> On Thu, Jul 31, 2008 at 09:40:59PM +0530, Aneesh Kumar K.V wrote:
> > Hi Ted,
> >
> > I haven't yet looked at whether it is kernel or e2fsck bug.
> > But I am having this with the latest patch queue when running
> > fsstress. The fsstress have falloc added as a supported operation.
> > So it is mostly to do with fallocate. Architecture is x86.
>
> Part of this is definitely an e2fsck bug, because e2fsck isn't
> currently checking the EXT2_EXTENT_FLAGS_UNINIT flag. In line 1715 of
> e2fsck/pass1.c:
>
> start_block = pb->last_block = extent.e_lblk + extent.e_len - 1;
>
> We shouldn't be setting pb->last_block if this is an unitialized
> extent.


What I am wondering is that why we didn't observe these errors before.
I used to run the same test before and I haven't seen the errors before.
I will check the logs again to verify this.

Also note that fallocate is called with update i_size option.


>
> I suspect this is also related though to the issue you raised in the
> "e2fsprogs and blocks outside i_size" thread about two weeks ago. So
> even after we fix this e2fsck bug, we may still have some fsstress
> failures until we decide how to deal with that particular issue.


fallocate is called in the test with update i_size. So it should not
happen.

>
> Do you have a quickie program that calls fallocate to preallocate
> space for a particular program easily to hand? If not, I can write
> one in about 5-10 minutes, but if you have one written already, it'll
> save me the effort. Thanks!!


-aneesh

2008-07-31 18:25:56

by Theodore Ts'o

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

On Thu, Jul 31, 2008 at 10:38:56PM +0530, Aneesh Kumar K.V wrote:
>
> Also note that fallocate is called with update i_size option.
>

Are you sure about this? I can replicate the problem using Eric's
fallocate problem if i_size is not updated:

<tytso.root@closure> {/tmp}, level 2
509# touch /mnt/foo
<tytso.root@closure> {/tmp}, level 2
510# ./fallocate -n -l 10m /mnt/foo
<tytso.root@closure> {/tmp}, level 2
511# ls -al /mnt/foo
10240 -rw-r--r-- 1 root root 0 2008-07-31 14:24 /mnt/foo
<tytso.root@closure> {/tmp}, level 2
512# umount /mnt
<tytso.root@closure> {/tmp}, level 2
513# e2fsck -fy /dev/thunk/testext4
e2fsck 1.41.0 (10-Jul-2008)
Pass 1: Checking inodes, blocks, and sizes
Inode 12, i_size is 0, should be 10485760. Fix? yes

Pass 2: Checking directory structure
Pass 3: Checking directory connectivity
Pass 4: Checking reference counts
Pass 5: Checking group summary information

/dev/thunk/testext4: ***** FILE SYSTEM WAS MODIFIED *****
/dev/thunk/testext4: 12/65536 files (16.7% non-contiguous), 15205/262144 blocks

- Ted

2008-07-31 18:47:45

by Theodore Ts'o

[permalink] [raw]
Subject: Re: i_size error with the latest patchqueue

So I think what I need to do is to have scan_extent_node update two
variables; pb->last_block, and pb->last_block_used, where
pb->last_block_used doesn't get updated if the extent is not
initialized.

However, what we'll have to do is that if the two are different,
i_size must be at least pb->last_block_used*blocksize, but less than or
equal to pb->last_block*blocksize + (blocksize-1).

- Ted