Skip to contentSkip to conversation

fstests: generic/260: Make it handle btrfs more gracefully

PATCH In review Qu Wenruo 4 participants 6 messages Jun 3, 2019 – Jun 7, 2019 Btrfs file system

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

PATCH v2 1/1 fstests: generic/260: Make it handle btrfs more gracefully

If a filesystem doesn't map its logical address space (normally the bytenr/blocknr returned by fiemap) directly to its devices(s), the following assumptions used in the test case is no longer true:
- trim range start beyond the end of fs should fail
- trim range start beyond the end of fs with len set should fail

Under the following example, even with just one device, btrfs can still trim the fs correctly while breaking above assumption:

0 1G 1.25G

|---------------|///////////////|-----------------| <- btrfs logical
		   |				       address space
        ------------  mapped as SINGLE
        |

0 V 256M
|///////////////| <- device address space

Thus trim range start=1G len=256M will cause btrfs to trim the 256M block group, thus return correct result.

Furthermore, there is no cleared defined behavior for whether a fs should trim the unmapped space. (only for indirectly mapped fs)

Btrfs currently will always trim the unmapped space, but the behavior can change as large trim can be very expensive.

Despite the change to skip certain tests for btrfs, still run the following tests for btrfs:
- trim start=U64_MAX with lenght set
This will expose a bug that btrfs doesn't check overflow of the range. This bug will be fixed soon.

- trim beyond the end of the fs
This will expose a bug where btrfs could send trim command beyond the end of its device.
This bug is a regression, can be fixed by reverting c2d1b3aae336 ("btrfs:
Honour FITRIM range constraints during free space trim")

With proper fixes for btrfs, this test case should pass on btrfs, ext4, xfs.

---
changelog:
v2:
- Return 0/1 instead of echo "1"/"0" for _is_fs_directly_mapped Although it may be a little confusing, but make
"if _is_fs_directly_mapped; then" much cleaner.
- Comment change.

---
 common/rc             | 15 ++++++++
 tests/generic/260     | 79 +++++++++++++++++++++++++++----------------
 tests/generic/260.out |  9 +----
 3 files changed, 66 insertions(+), 37 deletions(-)
common/rc +15 -0
index 17b89d5d..117dcec2 100644
@@ -4005,6 +4005,21 @@ _require_fibmap()
 	rm -f $file
 }
 
+# Check if the logical address (returned by fiemap) can be larger than the
+# block device.
+# Currently only btrfs has such behavior even for single device usage.
+# Return 0 if it's directly mapped, return 1 if not.
+# This return value looks confusing but allows us to use
+# "if _is_fs_direct_mapped; then" directly
+_is_fs_direct_mapped()
+{
+	if [ "$FSTYP" == "btrfs" ]; then
+		return 1
+	else
+		return 0
+	fi
+}
+
 _try_wipe_scratch_devs()
 {
 	test -x "$WIPEFS_PROG" || return 0
tests/generic/260 +50 -29

133 lines not on this page · raw message

tests/generic/260.out +1 -8
index a16c4f74..f4ee2f72 100644
@@ -1,12 +1,5 @@
 QA output created by 260
-[+] Start beyond the end of fs (should fail)
-fstrim: SCRATCH_MNT: FITRIM ioctl failed: Invalid argument
-[+] Start beyond the end of fs with len set (should fail)
-fstrim: SCRATCH_MNT: FITRIM ioctl failed: Invalid argument
-[+] Start = 2^64-1 (should fail)
-fstrim: SCRATCH_MNT: FITRIM ioctl failed: Invalid argument
-[+] Start = 2^64-1 and len is set (should fail)
-fstrim: SCRATCH_MNT: FITRIM ioctl failed: Invalid argument
+[+] Optional trim range test (fs dependent)
 [+] Default length (should succeed)
 [+] Default length with start set (should succeed)
 [+] Length beyond the end of fs (should succeed)
Signature · 1 line
2.21.0
#1 lore raw
Quoted 42 lines · Qu Wenruo

If a filesystem doesn't map its logical address space (normally the bytenr/blocknr returned by fiemap) directly to its devices(s), the following assumptions used in the test case is no longer true:
- trim range start beyond the end of fs should fail
- trim range start beyond the end of fs with len set should fail

Under the following example, even with just one device, btrfs can still trim the fs correctly while breaking above assumption:

0 1G 1.25G

|---------------|///////////////|-----------------| <- btrfs logical
		   |				       address space
        ------------  mapped as SINGLE
        |

0 V 256M
|///////////////| <- device address space

Thus trim range start=1G len=256M will cause btrfs to trim the 256M block group, thus return correct result.

Furthermore, there is no cleared defined behavior for whether a fs should trim the unmapped space. (only for indirectly mapped fs)

Btrfs currently will always trim the unmapped space, but the behavior can change as large trim can be very expensive.

Despite the change to skip certain tests for btrfs, still run the following tests for btrfs:
- trim start=U64_MAX with lenght set
This will expose a bug that btrfs doesn't check overflow of the range. This bug will be fixed soon.

- trim beyond the end of the fs
This will expose a bug where btrfs could send trim command beyond the end of its device.
This bug is a regression, can be fixed by reverting c2d1b3aae336 ("btrfs:
Honour FITRIM range constraints during free space trim")

With proper fixes for btrfs, this test case should pass on btrfs, ext4, xfs.

Quoted 8 lines

---
changelog:
v2:
- Return 0/1 instead of echo "1"/"0" for _is_fs_directly_mapped Although it may be a little confusing, but make
"if _is_fs_directly_mapped; then" much cleaner.
- Comment change.
---

Nope, the output is rather unhelpful. Current misc-next of btrfs fails and the output is:

[+] Start = 2^64-1 and len is set (should fail)

[+] Trim an empty fs

13554941952 trimed

[+] Try to trim beyond the end of the fs

[+] Try to trim the fs with large enough len

15727198208 trimed

generic/260 [failed, exit status 1]

There is no 260.out file which is supposed to contain some of the error strings which in turn makes the test tedious to debug...

<snip>

#2 lore raw
Quoted 54 lines · Nikolay Borisov
Quoted 42 lines · Qu Wenruo

If a filesystem doesn't map its logical address space (normally the bytenr/blocknr returned by fiemap) directly to its devices(s), the following assumptions used in the test case is no longer true:
- trim range start beyond the end of fs should fail
- trim range start beyond the end of fs with len set should fail

Under the following example, even with just one device, btrfs can still trim the fs correctly while breaking above assumption:

0 1G 1.25G

|---------------|///////////////|-----------------| <- btrfs logical
		   |				       address space
        ------------  mapped as SINGLE
        |

0 V 256M
|///////////////| <- device address space

Thus trim range start=1G len=256M will cause btrfs to trim the 256M block group, thus return correct result.

Furthermore, there is no cleared defined behavior for whether a fs should trim the unmapped space. (only for indirectly mapped fs)

Btrfs currently will always trim the unmapped space, but the behavior can change as large trim can be very expensive.

Despite the change to skip certain tests for btrfs, still run the following tests for btrfs:
- trim start=U64_MAX with lenght set
This will expose a bug that btrfs doesn't check overflow of the range. This bug will be fixed soon.

- trim beyond the end of the fs
This will expose a bug where btrfs could send trim command beyond the end of its device.
This bug is a regression, can be fixed by reverting c2d1b3aae336 ("btrfs:
Honour FITRIM range constraints during free space trim")

With proper fixes for btrfs, this test case should pass on btrfs, ext4, xfs.

Quoted 8 lines

---
changelog:
v2:
- Return 0/1 instead of echo "1"/"0" for _is_fs_directly_mapped Although it may be a little confusing, but make
"if _is_fs_directly_mapped; then" much cleaner.
- Comment change.
---

Quoted 2 lines

Nope, the output is rather unhelpful. Current misc-next of btrfs fails and the output is:

This is not the output. This is seqres.full.

For output, using 5.2-rc2, btrfs would fail like:
[+] Optional trim range test (fs dependent)
[+] Default length (should succeed)
[+] Default length with start set (should succeed) [+] Length beyond the end of fs (should succeed)
[+] Length beyond the end of fs with start set (should succeed) Unexpected error happened during trim
Test done

Which is already good enough to show what's wrong.

Quoted 5 lines

[+] Start = 2^64-1 and len is set (should fail)

[+] Trim an empty fs

Quoted 7 lines

13554941952 trimed

[+] Try to trim beyond the end of the fs

[+] Try to trim the fs with large enough len

15727198208 trimed

For this full, try it on 5.2-rc2, then you would understand why it's here:

[+] Start = 2^64-1 and len is set (should fail) << It doesn't fail [+] Trim an empty fs
0 trimed << It trimmed 0 bytes, isn't it already a problem? [+] Try to trim beyond the end of the fs
fstrim: /mnt/scratch: FITRIM ioctl failed: Input/output error << Beyond device end bug
[+] Try to trim the fs with large enough len
5367267328 trimed << The only good result here.

Quoted 6 lines

generic/260 [failed, exit status 1]

There is no 260.out file which is supposed to contain some of the error strings which in turn makes the test tedious to debug...

I'm afraid you're checking the wrong file.

Thanks,
Qu

Quoted 4 lines

<snip>

#3 lore raw
Quoted 98 lines · Qu Wenruo
Quoted 54 lines · Nikolay Borisov
Quoted 42 lines · Qu Wenruo

If a filesystem doesn't map its logical address space (normally the bytenr/blocknr returned by fiemap) directly to its devices(s), the following assumptions used in the test case is no longer true:
- trim range start beyond the end of fs should fail
- trim range start beyond the end of fs with len set should fail

Under the following example, even with just one device, btrfs can still trim the fs correctly while breaking above assumption:

0 1G 1.25G

|---------------|///////////////|-----------------| <- btrfs logical
		   |				       address space
        ------------  mapped as SINGLE
        |

0 V 256M
|///////////////| <- device address space

Thus trim range start=1G len=256M will cause btrfs to trim the 256M block group, thus return correct result.

Furthermore, there is no cleared defined behavior for whether a fs should trim the unmapped space. (only for indirectly mapped fs)

Btrfs currently will always trim the unmapped space, but the behavior can change as large trim can be very expensive.

Despite the change to skip certain tests for btrfs, still run the following tests for btrfs:
- trim start=U64_MAX with lenght set
This will expose a bug that btrfs doesn't check overflow of the range. This bug will be fixed soon.

- trim beyond the end of the fs
This will expose a bug where btrfs could send trim command beyond the end of its device.
This bug is a regression, can be fixed by reverting c2d1b3aae336 ("btrfs:
Honour FITRIM range constraints during free space trim")

With proper fixes for btrfs, this test case should pass on btrfs, ext4, xfs.

Quoted 8 lines

---
changelog:
v2:
- Return 0/1 instead of echo "1"/"0" for _is_fs_directly_mapped Although it may be a little confusing, but make
"if _is_fs_directly_mapped; then" much cleaner.
- Comment change.
---

Quoted 2 lines

Nope, the output is rather unhelpful. Current misc-next of btrfs fails and the output is:

This is not the output. This is seqres.full.

For output, using 5.2-rc2, btrfs would fail like:
[+] Optional trim range test (fs dependent)
[+] Default length (should succeed)
[+] Default length with start set (should succeed) [+] Length beyond the end of fs (should succeed)
[+] Length beyond the end of fs with start set (should succeed) Unexpected error happened during trim
Test done

Which is already good enough to show what's wrong.

Quoted 5 lines

[+] Start = 2^64-1 and len is set (should fail)

[+] Trim an empty fs

Quoted 7 lines

13554941952 trimed

[+] Try to trim beyond the end of the fs

[+] Try to trim the fs with large enough len

15727198208 trimed

For this full, try it on 5.2-rc2, then you would understand why it's here:

[+] Start = 2^64-1 and len is set (should fail) << It doesn't fail [+] Trim an empty fs
0 trimed << It trimmed 0 bytes, isn't it already a problem? [+] Try to trim beyond the end of the fs
fstrim: /mnt/scratch: FITRIM ioctl failed: Input/output error << Beyond device end bug
[+] Try to trim the fs with large enough len
5367267328 trimed << The only good result here.

Quoted 1 line
Quoted 7 lines
Quoted 5 lines

generic/260 [failed, exit status 1]

There is no 260.out file which is supposed to contain some of the error strings which in turn makes the test tedious to debug...

I'm afraid you're checking the wrong file.

I don't have an .out file produced!

Quoted 9 lines

Thanks,
Qu

Quoted 4 lines

<snip>

#4 lore raw
Quoted 104 lines · Nikolay Borisov
Quoted 101 lines · Qu Wenruo
Quoted 54 lines · Nikolay Borisov
Quoted 42 lines · Qu Wenruo

If a filesystem doesn't map its logical address space (normally the bytenr/blocknr returned by fiemap) directly to its devices(s), the following assumptions used in the test case is no longer true:
- trim range start beyond the end of fs should fail
- trim range start beyond the end of fs with len set should fail

Under the following example, even with just one device, btrfs can still trim the fs correctly while breaking above assumption:

0 1G 1.25G

|---------------|///////////////|-----------------| <- btrfs logical
		   |				       address space
        ------------  mapped as SINGLE
        |

0 V 256M
|///////////////| <- device address space

Thus trim range start=1G len=256M will cause btrfs to trim the 256M block group, thus return correct result.

Furthermore, there is no cleared defined behavior for whether a fs should trim the unmapped space. (only for indirectly mapped fs)

Btrfs currently will always trim the unmapped space, but the behavior can change as large trim can be very expensive.

Despite the change to skip certain tests for btrfs, still run the following tests for btrfs:
- trim start=U64_MAX with lenght set
This will expose a bug that btrfs doesn't check overflow of the range. This bug will be fixed soon.

- trim beyond the end of the fs
This will expose a bug where btrfs could send trim command beyond the end of its device.
This bug is a regression, can be fixed by reverting c2d1b3aae336 ("btrfs:
Honour FITRIM range constraints during free space trim")

With proper fixes for btrfs, this test case should pass on btrfs, ext4, xfs.

Quoted 8 lines

---
changelog:
v2:
- Return 0/1 instead of echo "1"/"0" for _is_fs_directly_mapped Although it may be a little confusing, but make
"if _is_fs_directly_mapped; then" much cleaner.
- Comment change.
---

Quoted 2 lines

Nope, the output is rather unhelpful. Current misc-next of btrfs fails and the output is:

This is not the output. This is seqres.full.

For output, using 5.2-rc2, btrfs would fail like:
[+] Optional trim range test (fs dependent)
[+] Default length (should succeed)
[+] Default length with start set (should succeed) [+] Length beyond the end of fs (should succeed)
[+] Length beyond the end of fs with start set (should succeed) Unexpected error happened during trim
Test done

Which is already good enough to show what's wrong.

Quoted 5 lines

[+] Start = 2^64-1 and len is set (should fail)

[+] Trim an empty fs

Quoted 7 lines

13554941952 trimed

[+] Try to trim beyond the end of the fs

[+] Try to trim the fs with large enough len

15727198208 trimed

For this full, try it on 5.2-rc2, then you would understand why it's here:

[+] Start = 2^64-1 and len is set (should fail) << It doesn't fail [+] Trim an empty fs
0 trimed << It trimmed 0 bytes, isn't it already a problem? [+] Try to trim beyond the end of the fs
fstrim: /mnt/scratch: FITRIM ioctl failed: Input/output error << Beyond device end bug
[+] Try to trim the fs with large enough len
5367267328 trimed << The only good result here.

Quoted 4 lines

generic/260 [failed, exit status 1]

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

There is no 260.out file which is supposed to contain some of the error strings which in turn makes the test tedious to debug...

I'm afraid you're checking the wrong file.

I don't have an .out file produced!

Then it's because the output matches.
Only status is wrong.

For latest misc branch, the only test it will fail should be that start = -1 with len set one.
Since that subtest is inside the optional fs dependent test, thus it doesn't output into stdio but completely rely on seqres.full.

In that case, the seqres.full output should be enough to show the problem.

Quoted 1 line

[+] Start = 2^64-1 and len is set (should fail)

Should fail but no error message? Isn't that the reason why it fails?

Thanks,
Qu

Quoted 11 lines
Quoted 11 lines

Thanks,
Qu

Quoted 4 lines

<snip>

#5 lore raw
In reply to Qu Wenruo
Quoted 5 lines · Qu Wenruo

If a filesystem doesn't map its logical address space (normally the bytenr/blocknr returned by fiemap) directly to its devices(s), the following assumptions used in the test case is no longer true:
- trim range start beyond the end of fs should fail
- trim range start beyond the end of fs with len set should fail

All these assumptions seem generally bogus. I don't think we can just run this as a generic test.