2014-04-09 05:37:05

by jon ernst

[permalink] [raw]
Subject: xfstest-bld generic/018 fails due to e4defrag issue

Hi,
running latest xfstest-bld with latest ext4 kernel "dev"
branch(ad6599ab3a).I always get generic/018 failed.
Then I took closer look and found out this issue.

After run this:

fragfile="/vdf/testfile"
for I in `seq 9 -1 0`; do
dd if=/dev/zero of=$fragfile bs=4k count=1 conv=notrunc seek=$I oflag=sync
done


Even though the file does exist. e4defrag complains about:

(this output comes from kvm guest machine)
> e4defrag -v /vdf/testfile
Can't get super block info: Success
"/vdf/testfile"

Is this a known issue or something I did wrong.

Thank you!

Jon


2014-04-09 22:03:24

by Theodore Ts'o

[permalink] [raw]
Subject: Re: xfstest-bld generic/018 fails due to e4defrag issue

On Wed, Apr 09, 2014 at 01:37:04AM -0400, jon ernst wrote:
> running latest xfstest-bld with latest ext4 kernel "dev"
> branch(ad6599ab3a).I always get generic/018 failed.
> Then I took closer look and found out this issue.

That's a renamed tested; it was previously shared/218. It's a test
which is known to fail for ext4, since its idea of how a defrag
program should work is slightly different from how e4defrag works:

shared/218 7s ... [20:48:32] [20:48:39] - output mismatch (see /results/results-4k/shared/218.out.bad)
--- tests/shared/218.out 2014-04-01 18:46:39.000000000 +0000
+++ /results/results-4k/shared/218.out.bad 2014-04-03 20:48:39.795694518 +0000
@@ -10,7 +10,7 @@
After: 1
Write backwards sync, but contiguous - should defrag to 1 extent
Before: 10
-After: 1
+After: 10
Write backwards sync leaving holes - defrag should do nothing
Before: 16
...
(Run 'diff -u tests/shared/218.out /results/results-4k/shared/218.out.bad' to see the entire diff)

What you are seeing is something very different, though.

> Even though the file does exist. e4defrag complains about:
>
> (this output comes from kvm guest machine)
> > e4defrag -v /vdf/testfile
> Can't get super block info: Success
> "/vdf/testfile"
>
> Is this a known issue or something I did wrong.

Unfortunately, e4defrag has horrible error handling, so we can't see
the error code properly, so we can't see why it's failing, but this is
from an attempt to open the file system to get some low-level
information.

How is /etc/mtab set up on your test machine? It looks like it failed
to find block device for the file system in question.

- Ted

2014-04-10 04:13:49

by jon ernst

[permalink] [raw]
Subject: Re: xfstest-bld generic/018 fails due to e4defrag issue

On Wed, Apr 9, 2014 at 6:03 PM, Theodore Ts'o <[email protected]> wrote:
> On Wed, Apr 09, 2014 at 01:37:04AM -0400, jon ernst wrote:
>> running latest xfstest-bld with latest ext4 kernel "dev"
>> branch(ad6599ab3a).I always get generic/018 failed.
>> Then I took closer look and found out this issue.
>
> That's a renamed tested; it was previously shared/218. It's a test
> which is known to fail for ext4, since its idea of how a defrag
> program should work is slightly different from how e4defrag works:
>
> shared/218 7s ... [20:48:32] [20:48:39] - output mismatch (see /results/results-4k/shared/218.out.bad)
> --- tests/shared/218.out 2014-04-01 18:46:39.000000000 +0000
> +++ /results/results-4k/shared/218.out.bad 2014-04-03 20:48:39.795694518 +0000
> @@ -10,7 +10,7 @@
> After: 1
> Write backwards sync, but contiguous - should defrag to 1 extent
> Before: 10
> -After: 1
> +After: 10
> Write backwards sync leaving holes - defrag should do nothing
> Before: 16
> ...
> (Run 'diff -u tests/shared/218.out /results/results-4k/shared/218.out.bad' to see the entire diff)
>
> What you are seeing is something very different, though.
>
>> Even though the file does exist. e4defrag complains about:
>>
>> (this output comes from kvm guest machine)
>> > e4defrag -v /vdf/testfile
>> Can't get super block info: Success
>> "/vdf/testfile"
>>
>> Is this a known issue or something I did wrong.
>
> Unfortunately, e4defrag has horrible error handling, so we can't see
> the error code properly, so we can't see why it's failing, but this is
> from an attempt to open the file system to get some low-level
> information.
>
> How is /etc/mtab set up on your test machine? It looks like it failed
> to find block device for the file system in question.
>
> - Ted

I found the root cause of this failure.

The failure case happens on "bigalloc" testing option.
ext2fs_open failed due to EXT2_FLAG_64BITS is not being set in testing
rootfs image. So ext2fs_open in e4defrag.c returns err: 2133571465.

Because bigalloc requires cluster-aware bitfield operations, which
means we need EXT2_FLAG_64BITS.
I see e2image.c creates image always with EXT2_FLAG_64BITS flag. It is
safe to do same thing for e4defrag in my opinion. Please correct me if
I am wrong.




[PATCH] e4defrag: open fs with EXT2_FLAG_64BITS flag

Signed-off-by: Jon Ernst <[email protected]>
---
misc/e4defrag.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/misc/e4defrag.c b/misc/e4defrag.c
index 620f4e7..c5a2754 100644
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -1794,7 +1794,7 @@ int main(int argc, char *argv[])

if (current_uid == ROOT_UID) {
/* Get super block info */
- ret = ext2fs_open(dev_name, 0, 0, block_size,
+ ret = ext2fs_open(dev_name,EXT2_FLAG_64BITS,
0, block_size,
unix_io_manager, &fs);
if (ret) {
if (mode_flag & DETAIL) {
--
1.8.1.2

2014-04-10 13:56:41

by Theodore Ts'o

[permalink] [raw]
Subject: Re: xfstest-bld generic/018 fails due to e4defrag issue

On Thu, Apr 10, 2014 at 12:13:49AM -0400, jon ernst wrote:
>
> Because bigalloc requires cluster-aware bitfield operations, which
> means we need EXT2_FLAG_64BITS.
> I see e2image.c creates image always with EXT2_FLAG_64BITS flag. It is
> safe to do same thing for e4defrag in my opinion. Please correct me if
> I am wrong.

Um.... I *think* so. e4defrag is one of the less well
tested/maintained parts of e2fsprogs, as well as the kernel-side code
which supports e4defrag. I can't think of any reason why there would
be any 32-bit dependencies in the kernel side code, although someone
should probably do a quick audit of the e4defrag code to make sure
it's not using blk_t where it should be using blk64_t, or have other
32-bit dependencies.

- Ted

2014-04-10 18:42:22

by Darrick J. Wong

[permalink] [raw]
Subject: Re: xfstest-bld generic/018 fails due to e4defrag issue

On Thu, Apr 10, 2014 at 09:56:37AM -0400, Theodore Ts'o wrote:
> On Thu, Apr 10, 2014 at 12:13:49AM -0400, jon ernst wrote:
> >
> > Because bigalloc requires cluster-aware bitfield operations, which
> > means we need EXT2_FLAG_64BITS.
> > I see e2image.c creates image always with EXT2_FLAG_64BITS flag. It is
> > safe to do same thing for e4defrag in my opinion. Please correct me if
> > I am wrong.
>
> Um.... I *think* so. e4defrag is one of the less well
> tested/maintained parts of e2fsprogs, as well as the kernel-side code
> which supports e4defrag. I can't think of any reason why there would
> be any 32-bit dependencies in the kernel side code, although someone
> should probably do a quick audit of the e4defrag code to make sure
> it's not using blk_t where it should be using blk64_t, or have other
> 32-bit dependencies.

>From a quick visual inspection and a sparse bitwise check, e4defrag looks 64bit
clean.

--D
>
> - Ted
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-04-11 03:07:42

by Theodore Ts'o

[permalink] [raw]
Subject: Re: xfstest-bld generic/018 fails due to e4defrag issue

On Thu, Apr 10, 2014 at 11:42:15AM -0700, Darrick J. Wong wrote:
>
> From a quick visual inspection and a sparse bitwise check, e4defrag looks 64bit
> clean.

Thanks for checking! I've applied Jon's patch.

- Ted

2014-04-11 03:10:48

by jon ernst

[permalink] [raw]
Subject: Re: xfstest-bld generic/018 fails due to e4defrag issue

On Thu, Apr 10, 2014 at 11:07 PM, Theodore Ts'o <[email protected]> wrote:
> On Thu, Apr 10, 2014 at 11:42:15AM -0700, Darrick J. Wong wrote:
>>
>> From a quick visual inspection and a sparse bitwise check, e4defrag looks 64bit
>> clean.
>
> Thanks for checking! I've applied Jon's patch.
>
> - Ted
Thanks Ted, I have also checked e4defrag.c with eyeballs. All physical
blocks are represented by 64bit.

2014-04-11 03:18:07

by Theodore Ts'o

[permalink] [raw]
Subject: [PATCH] e4defrag: fix error reporting when ext2fs_open fails

Signed-off-by: "Theodore Ts'o" <[email protected]>
---
misc/e4defrag.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/misc/e4defrag.c b/misc/e4defrag.c
index a1224bb..0db5e4b 100644
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -1797,10 +1797,10 @@ int main(int argc, char *argv[])
ret = ext2fs_open(dev_name, EXT2_FLAG_64BITS, 0,
block_size, unix_io_manager, &fs);
if (ret) {
- if (mode_flag & DETAIL) {
- perror("Can't get super block info");
- PRINT_FILE_NAME(argv[i]);
- }
+ if (mode_flag & DETAIL)
+ com_err(argv[1], ret,
+ "while trying to open file system: %s",
+ dev_name);
continue;
}

--
1.9.0