* Commit 94968e74 breaks f_lotsbad @ 2011-06-17 6:28 Andreas Dilger 2011-06-20 1:58 ` Ted Ts'o 2011-08-10 19:07 ` [PATCH] libext2fs: copy cluster_bits in ext2fs_copy_generic_bmap Eric Sandeen 0 siblings, 2 replies; 7+ messages in thread From: Andreas Dilger @ 2011-06-17 6:28 UTC (permalink / raw) To: Ted Ts'o; +Cc: ext4 development Ted, I was going to send some more fixes, but it looks like your commit 94968e74 "libext2fs: teach bitmap functions about bigalloc/cluster" breaks the "f_lotsbad" test, which I found via git bisect and verified is the offending commit: --- ./f_lotsbad/expect.1 2011-06-01 20:25:00.871615457 +0000 +++ f_lotsbad.1.log 2011-06-17 05:33:37.398073505 +0000 @@ -26,6 +26,7 @@ Restarting e2fsck from the beginning... Pass 1: Checking inodes, blocks, and sizes +Illegal block number passed to ext2fs_test_block_bitmap #0 for in-use block map Pass 2: Checking directory structure Entry 'termcap' in / (2) has deleted/unused inode 12. Clear? yes I couldn't see the obvious correlation between the failure and the patch, though it is clear the patch is changing ext2fs_test_generic_bitmap(), but all of the changes should be no-ops because cluster_bits should be 0 unless bigalloc is enabled. My tree is up to b2e6c86d6, "mke2fs, e2fsck: fix i_blocks handling for bigalloc file systems", which AFAICS is the latest. Cheers, Andreas -- Andreas Dilger Principal Engineer Whamcloud, Inc. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Commit 94968e74 breaks f_lotsbad 2011-06-17 6:28 Commit 94968e74 breaks f_lotsbad Andreas Dilger @ 2011-06-20 1:58 ` Ted Ts'o 2011-06-28 5:16 ` Andreas Dilger 2011-08-10 19:07 ` [PATCH] libext2fs: copy cluster_bits in ext2fs_copy_generic_bmap Eric Sandeen 1 sibling, 1 reply; 7+ messages in thread From: Ted Ts'o @ 2011-06-20 1:58 UTC (permalink / raw) To: Andreas Dilger; +Cc: ext4 development On Fri, Jun 17, 2011 at 12:28:01AM -0600, Andreas Dilger wrote: > Ted, > I was going to send some more fixes, but it looks like your commit > 94968e74 "libext2fs: teach bitmap functions about bigalloc/cluster" > breaks the "f_lotsbad" test, which I found via git bisect and verified > is the offending commit: The problem was a bug in lib/ext2fs/Makefile.in; the $(SRCS) macro included "$(srcdir)/blkmap64_ba.o" (note ".o" instead of ".o"; Oops.) As a result we didn't have a dependency for blkmap64_ba.c in the Makefile, and so blkmap64_ba.o wasn't getting regenerated when bmap64.h changed. You can fix it by rm'ing blkmap64_ba.o and then rebuilding. I'll fixed the Makefile.in, rerun "make depend", and then check in a fix. Regards, - Ted ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Commit 94968e74 breaks f_lotsbad 2011-06-20 1:58 ` Ted Ts'o @ 2011-06-28 5:16 ` Andreas Dilger 2011-06-28 14:23 ` Ted Ts'o 0 siblings, 1 reply; 7+ messages in thread From: Andreas Dilger @ 2011-06-28 5:16 UTC (permalink / raw) To: Ted Ts'o; +Cc: ext4 development On 2011-06-19, at 7:58 PM, Ted Ts'o wrote: > On Fri, Jun 17, 2011 at 12:28:01AM -0600, Andreas Dilger wrote: >> >> I was going to send some more fixes, but it looks like your commit >> 94968e74 "libext2fs: teach bitmap functions about bigalloc/cluster" >> breaks the "f_lotsbad" test, which I found via git bisect and verified >> is the offending commit: > > The problem was a bug in lib/ext2fs/Makefile.in; the $(SRCS) macro > included "$(srcdir)/blkmap64_ba.o" (note ".o" instead of ".o"; Oops.) > > As a result we didn't have a dependency for blkmap64_ba.c in the > Makefile, and so blkmap64_ba.o wasn't getting regenerated when > bmap64.h changed. You can fix it by rm'ing blkmap64_ba.o and then > rebuilding. I'll fixed the Makefile.in, rerun "make depend", and then > check in a fix. I retested after pulling your commit 1ca87790b914a1958f3cbd8b5e1e7037f2cf30ed "libext2fs: fix makefile dependency problem", but even when I do "make clean" before each test I still get a failure on the "f_lotsbad" test: --- ./f_lotsbad/expect.1 2011-06-01 20:25:00.871615457 +0000 +++ f_lotsbad.1.log 2011-06-28 05:09:16.484198659 +0000 @@ -26,6 +26,7 @@ Restarting e2fsck from the beginning... Pass 1: Checking inodes, blocks, and sizes +Illegal block number passed to ext2fs_test_block_bitmap #0 for in-use block map Pass 2: Checking directory structure Entry 'termcap' in / (2) has deleted/unused inode 12. Clear? yes I verified that no "*.o" files remained before rebuilding, but it didn't fix the problem. I also tested on both the "maint" and "next" branches in case they were different, but it failed on both. Cheers, Andreas -- Andreas Dilger Principal Engineer Whamcloud, Inc. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Commit 94968e74 breaks f_lotsbad 2011-06-28 5:16 ` Andreas Dilger @ 2011-06-28 14:23 ` Ted Ts'o 2011-06-29 7:09 ` Andreas Dilger 0 siblings, 1 reply; 7+ messages in thread From: Ted Ts'o @ 2011-06-28 14:23 UTC (permalink / raw) To: Andreas Dilger; +Cc: ext4 development On Mon, Jun 27, 2011 at 11:16:14PM -0600, Andreas Dilger wrote: > > I retested after pulling your commit 1ca87790b914a1958f3cbd8b5e1e7037f2cf30ed > "libext2fs: fix makefile dependency problem", but even when I do "make clean" > before each test I still get a failure on the "f_lotsbad" test: I can't replicate this problem on my end. What gcc version are you using, and what are your configure options? This is what I'm using % ./config.status --version config.status configured by ../configure, generated by GNU Autoconf 2.65, with options "'--enable-elf-shlibs' '--enable-maintainer-mode' 'CFLAGS=-g' '--enable-symlink-build' '--enable-symlink-install' '--disable-libuuid' '--disable-libblkid'" Copyright (C) 2009 Free Software Foundation, Inc. This config.status script is free software; the Free Software Foundation gives unlimited permission to copy, distribute and modify it. % gcc --version gcc (Ubuntu 4.4.3-4ubuntu5) 4.4.3 - Ted ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Commit 94968e74 breaks f_lotsbad 2011-06-28 14:23 ` Ted Ts'o @ 2011-06-29 7:09 ` Andreas Dilger 0 siblings, 0 replies; 7+ messages in thread From: Andreas Dilger @ 2011-06-29 7:09 UTC (permalink / raw) To: Ted Ts'o; +Cc: ext4 development On 2011-06-28, at 8:23 AM, Ted Ts'o wrote: > On Mon, Jun 27, 2011 at 11:16:14PM -0600, Andreas Dilger wrote: >> >> I retested after pulling your commit 1ca87790b914a1958f3cbd8b5e1e7037f2cf30ed >> "libext2fs: fix makefile dependency problem", but even when I do "make clean" >> before each test I still get a failure on the "f_lotsbad" test: > > I can't replicate this problem on my end. What gcc version are you > using, and what are your configure options? > > This is what I'm using > > % ./config.status --version > config.status > configured by ../configure, generated by GNU Autoconf 2.65, > with options "'--enable-elf-shlibs' '--enable-maintainer-mode' 'CFLAGS=-g' '--enable-symlink-build' '--enable-symlink-install' '--disable-libuuid' '--disable-libblkid'" > > Copyright (C) 2009 Free Software Foundation, Inc. > This config.status script is free software; the Free Software Foundation > gives unlimited permission to copy, distribute and modify it. > > % gcc --version > gcc (Ubuntu 4.4.3-4ubuntu5) 4.4.3 I'm testing on an FC13 system, with a downgraded 2.6.32 FC12 kernel: [root@sookie e2fsprogs-git]# git describe v1.41.14-228-g1ca8779 [root@sookie e2fsprogs-git]# git diff [root@sookie e2fsprogs-git]# ./config.status --version config.status configured by ./configure, generated by GNU Autoconf 2.65, with options "" Copyright (C) 2009 Free Software Foundation, Inc. This config.status script is free software; the Free Software Foundation gives unlimited permission to copy, distribute and modify it. [root@sookie e2fsprogs-git]# gcc --version gcc (GCC) 4.4.5 20101112 (Red Hat 4.4.5-2) Copyright (C) 2010 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. [root@sookie e2fsprogs-git]# cd tests [root@sookie tests]# make test_script /bin/cp ./mke2fs.conf.in mke2fs.conf Creating test_script... [root@sookie tests]# ./test_script f_lotsbad f_lotsbad: too many illegal blocks in inode: failed 0 tests succeeded 1 tests failed Very strangely, the test passes on another node (older FC3 with many updates) when run as a non-root user, though this isn't true on the original test node: [adilger@mookie tests]$ ./test_script f_lotsbad f_lotsbad: too many illegal blocks in inode: ok 1 tests succeeded 0 tests failed [adilger@mookie tests]$ logout [root@mookie e2fsprogs-git]# cd tests/ [root@mookie tests]# ./test_script f_lotsbad f_lotsbad: too many illegal blocks in inode: failed 0 tests succeeded 1 tests failed root@mookie tests]# su adilger Agent pid 29339 [adilger@mookie tests]$ ./test_script f_lotsbad f_lotsbad: too many illegal blocks in inode: ok 1 tests succeeded 0 tests failed [adilger@mookie tests]$ ./config.status --version config.status configured by ./configure, generated by GNU Autoconf 2.65, with options "'CFLAGS=-Wall -g'" Copyright (C) 2009 Free Software Foundation, Inc. This config.status script is free software; the Free Software Foundation gives unlimited permission to copy, distribute and modify it. [adilger@mookie tests]$ gcc -V gcc: `-V' option must have argument [adilger@mookie tests]$ gcc --version gcc (GCC) 3.4.4 20050721 (Red Hat 3.4.4-2) Copyright (C) 2004 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Comparing the environments between user and root don't show any (apparently) significant differences (PATH is different, but I don't think this test uses anything outside the local e2fsck) and even running strace the only noticeable differences are the getuid/getgid calls, and umask (which impacts the mode on the test.img file, but I don't think that should be relevant either). Cheers, Andreas -- Andreas Dilger Principal Engineer Whamcloud, Inc. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] libext2fs: copy cluster_bits in ext2fs_copy_generic_bmap 2011-06-17 6:28 Commit 94968e74 breaks f_lotsbad Andreas Dilger 2011-06-20 1:58 ` Ted Ts'o @ 2011-08-10 19:07 ` Eric Sandeen 2011-08-11 1:50 ` Ted Ts'o 1 sibling, 1 reply; 7+ messages in thread From: Eric Sandeen @ 2011-08-10 19:07 UTC (permalink / raw) To: Andreas Dilger; +Cc: Ted Ts'o, ext4 development The f_lotsbad regression test was failing on some systems with: @@ -26,6 +61,7 @@ Restarting e2fsck from the beginning... Pass 1: Checking inodes, blocks, and sizes +Illegal block number passed to ext2fs_test_block_bitmap #0 for in-use block map Pass 2: Checking directory structure Entry 'termcap' in / (2) has deleted/unused inode 12. Clear? yes Running with valgrind (./test_script --valgrind f_lotsbad) we see: +==31409== Conditional jump or move depends on uninitialised value(s) +==31409== at 0x42927A: ext2fs_test_generic_bmap (gen_bitmap64.c:378) among others. Looking at gen_bitmap64.c: 376: arg >>= bitmap->cluster_bits; 377: 378: if ((arg < bitmap->start) || (arg > bitmap->end)) { A little more debugging showed that it was actually bitmap->cluster_bits which was uninitialized, because it never gets copied over in ext2fs_copy_generic_bmap() Patch below resolves the issue. Reported-by: Andreas Dilger <adilger@whamcloud.com> Signed-off-by: Eric Sandeen <sandeen@redhat.com> --- diff --git a/lib/ext2fs/gen_bitmap64.c b/lib/ext2fs/gen_bitmap64.c index f75876d..80c9f7a 100644 --- a/lib/ext2fs/gen_bitmap64.c +++ b/lib/ext2fs/gen_bitmap64.c @@ -193,6 +193,7 @@ errcode_t ext2fs_copy_generic_bmap(ext2fs_generic_bitmap src, new_bmap->real_end = src->real_end; new_bmap->bitmap_ops = src->bitmap_ops; new_bmap->base_error_code = src->base_error_code; + new_bmap->cluster_bits = src->cluster_bits; descr = src->description; if (descr) { ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] libext2fs: copy cluster_bits in ext2fs_copy_generic_bmap 2011-08-10 19:07 ` [PATCH] libext2fs: copy cluster_bits in ext2fs_copy_generic_bmap Eric Sandeen @ 2011-08-11 1:50 ` Ted Ts'o 0 siblings, 0 replies; 7+ messages in thread From: Ted Ts'o @ 2011-08-11 1:50 UTC (permalink / raw) To: Eric Sandeen; +Cc: Andreas Dilger, ext4 development Oops, thanks for catching this! Applied to the e2fsprogs next branch. - Ted ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-08-11 1:50 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-06-17 6:28 Commit 94968e74 breaks f_lotsbad Andreas Dilger 2011-06-20 1:58 ` Ted Ts'o 2011-06-28 5:16 ` Andreas Dilger 2011-06-28 14:23 ` Ted Ts'o 2011-06-29 7:09 ` Andreas Dilger 2011-08-10 19:07 ` [PATCH] libext2fs: copy cluster_bits in ext2fs_copy_generic_bmap Eric Sandeen 2011-08-11 1:50 ` Ted Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).