LTP test df01.sh detected different size of loop device in v5.19

DISCUSSION Petr Vorel 6 participants 15 messages Aug 12, 2022 – Aug 19, 2022 Block layer

Share & tools

Petr Vorel

author

2022-08-12 13:21 UTC #0 lore raw

Hi all,

LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:

grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]

How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit

df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...

tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint

=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.

Also 'df -T -P' fails.

It might be a false positive / bug in the test, but it's at least a changed behavior. I was able to reproduce it on v5.19 distro kernels (openSUSE, Debian). I haven't bisected (yet), nor checked Jens' git tree (maybe it has been fixed).

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/blob/f42f6f3b4671f447b743afe8612917ba4362b8a6/testcases/commands/df/df01.sh#L103-L110

Petr Vorel

author

2022-08-12 13:24 UTC #1 lore raw

Quoted 20 lines

Hi all,
LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:
grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]
How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit
df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint

Quoted 8 lines

=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.
Also 'df -T -P' fails.
It might be a false positive / bug in the test, but it's at least a changed behavior. I was able to reproduce it on v5.19 distro kernels (openSUSE, Debian). I haven't bisected (yet), nor checked Jens' git tree (maybe it has been fixed).

Forget to note dmesg "operation not supported error" warning on *each* run (even successful) on affected v5.19:

[ 5097.594021] loop0: detected capacity change from 0 to 524288
[ 5097.658201] operation not supported error, dev loop0, sector 262192 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
[ 5097.675670] XFS (loop0): Mounting V5 Filesystem
[ 5097.681668] XFS (loop0): Ending clean mount
[ 5097.956445] XFS (loop0): Unmounting Filesystem

Kind regards,
Petr

Quoted 4 lines

Kind regards,
Petr
[1] https://github.com/linux-test-project/ltp/blob/f42f6f3b4671f447b743afe8612917ba4362b8a6/testcases/commands/df/df01.sh#L103-L110

Petr Vorel

author

2022-08-12 14:00 UTC #2 lore raw

[ Cc LTP ML, sorry for the noise ]
Petr

Quoted 44 lines

Quoted 20 lines
Hi all,
LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:
grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]
How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit
df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
Quoted 8 lines
=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.
Also 'df -T -P' fails.
It might be a false positive / bug in the test, but it's at least a changed behavior. I was able to reproduce it on v5.19 distro kernels (openSUSE, Debian). I haven't bisected (yet), nor checked Jens' git tree (maybe it has been fixed).
Forget to note dmesg "operation not supported error" warning on *each* run (even successful) on affected v5.19:
[ 5097.594021] loop0: detected capacity change from 0 to 524288
[ 5097.658201] operation not supported error, dev loop0, sector 262192 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
[ 5097.675670] XFS (loop0): Mounting V5 Filesystem
[ 5097.681668] XFS (loop0): Ending clean mount
[ 5097.956445] XFS (loop0): Unmounting Filesystem
Kind regards,
Petr
Quoted 4 lines
Kind regards,
Petr
[1] https://github.com/linux-test-project/ltp/blob/f42f6f3b4671f447b743afe8612917ba4362b8a6/testcases/commands/df/df01.sh#L103-L110

Dave Chinner

2022-08-14 22:44 UTC #3 lore raw

Quoted 18 lines · Petr Vorel

Hi all,
LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:
grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]
How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit
df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...

Quoted 4 lines

tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint

=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.

Yup, most likely because we changed something in XFS related to internal block reservation spaces. That is, the test is making fundamentally flawed assumptions about filesystem used space accounting.

It is wrong to assuming that the available capacity of a given empty filesystem will never change. Assumptions like this have been invalid for decades because the available space can change based on the underlying configuration or the filesystem. e.g. different versions of mkfs.xfs set different default parameters and so simply changing the version of xfsprogs you use between the two comparision tests will make it fail....

And, well, XFS also has XFS_IOC_{GS}ET_RESBLKS ioctls that allow userspace to change the amount of reserved blocks. They were introduced in 1997, and since then we've changed the default reservation the filesystem takes at least a dozen times.

Quoted 1 line

Quoted 1 line
It might be a false positive / bug in the test, but it's at least a changed behavior.

Yup, any test that assumes "available space" does not change from kernel version to kernel version is flawed. There is no guarantee that this ever stays the same, nor that it needs to stay the same.

Quoted 10 lines

Quoted 2 lines
I was able to reproduce it on v5.19 distro kernels (openSUSE, Debian). I haven't bisected (yet), nor checked Jens' git tree (maybe it has been fixed).
Forget to note dmesg "operation not supported error" warning on *each* run (even successful) on affected v5.19:
[ 5097.594021] loop0: detected capacity change from 0 to 524288
[ 5097.658201] operation not supported error, dev loop0, sector 262192 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
[ 5097.675670] XFS (loop0): Mounting V5 Filesystem
[ 5097.681668] XFS (loop0): Ending clean mount
[ 5097.956445] XFS (loop0): Unmounting Filesystem

That warning is from mkfs attempting to use fallocate(ZERO_RANGE) to offload the zeroing of the journal to the block device. It would seem that the loop device image file is being hosted on a filesystem that does not support the fallocate() ZERO_RANGE (or maybe PUNCH_HOLE) operation. That warning should simply be removed - it serves no useful purpose to a user...

CHeers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Petr Vorel

author

2022-08-15 09:31 UTC #4 lore raw

Hi Dave,

Quoted 37 lines · Petr Vorel

Quoted 18 lines
Hi all,
LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:
grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]
How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit
df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
Quoted 4 lines
tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.
Yup, most likely because we changed something in XFS related to internal block reservation spaces. That is, the test is making fundamentally flawed assumptions about filesystem used space accounting.
It is wrong to assuming that the available capacity of a given empty filesystem will never change. Assumptions like this have been invalid for decades because the available space can change based on the underlying configuration or the filesystem. e.g. different versions of mkfs.xfs set different default parameters and so simply changing the version of xfsprogs you use between the two comparision tests will make it fail....

Quoted 4 lines

And, well, XFS also has XFS_IOC_{GS}ET_RESBLKS ioctls that allow userspace to change the amount of reserved blocks. They were introduced in 1997, and since then we've changed the default reservation the filesystem takes at least a dozen times.

Thanks a lot for valuable info.

Quoted 5 lines

Quoted 1 line
Quoted 1 line
It might be a false positive / bug in the test, but it's at least a changed behavior.
Yup, any test that assumes "available space" does not change from kernel version to kernel version is flawed. There is no guarantee that this ever stays the same, nor that it needs to stay the same.

I'm sorry I was not clear. Test [1] does not measure "available space" between kernel releases. It just run df command with parameters, saves it's output and compares "1024-blocks" and "Used" columns of df output with stat output:

		local total=$(stat -f mntpoint --printf=%b)
		local free=$(stat -f mntpoint --printf=%f)
		local used=$((total-free))

		local bsize=$(stat -f mntpoint --printf=%s)
		total=$((($total * $bsize + 512)/ 1024))
		used=$((($used * $bsize + 512) / 1024))

And comparison with "$used" is what sometimes fails.

BTW this happens on both distros when loop device is on tmpfs. I'm trying to trigger it on ext4 and btrfs, not successful so far. Looks like it's tmpfs related.

If that's really expected, we might remove this check for used for XFS (not sure if check only for total makes sense).

Quoted 11 lines

Quoted 10 lines
Quoted 2 lines
I was able to reproduce it on v5.19 distro kernels (openSUSE, Debian). I haven't bisected (yet), nor checked Jens' git tree (maybe it has been fixed).
Forget to note dmesg "operation not supported error" warning on *each* run (even successful) on affected v5.19:
[ 5097.594021] loop0: detected capacity change from 0 to 524288
[ 5097.658201] operation not supported error, dev loop0, sector 262192 op 0x9:(WRITE_ZEROES) flags 0x8000800 phys_seg 0 prio class 0
[ 5097.675670] XFS (loop0): Mounting V5 Filesystem
[ 5097.681668] XFS (loop0): Ending clean mount
[ 5097.956445] XFS (loop0): Unmounting Filesystem

Quoted 6 lines

That warning is from mkfs attempting to use fallocate(ZERO_RANGE) to offload the zeroing of the journal to the block device. It would seem that the loop device image file is being hosted on a filesystem that does not support the fallocate() ZERO_RANGE (or maybe PUNCH_HOLE) operation. That warning should simply be removed - it serves no useful purpose to a user...

Interesting. Which one of these two is not supported on tmpfs?

Kind regards,
Petr

Quoted 3 lines

CHeers,
Dave.

[1] https://github.com/linux-test-project/ltp/blob/f42f6f3b4671f447b743afe8612917ba4362b8a6/testcases/commands/df/df01.sh

Eric Sandeen

2022-08-15 20:10 UTC #5 lore raw

Quoted 46 lines · Petr Vorel

Hi Dave,
Quoted 37 lines · Petr Vorel
Quoted 18 lines
Hi all,
LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:
grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]
How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit
df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
Quoted 4 lines
tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.
Yup, most likely because we changed something in XFS related to internal block reservation spaces. That is, the test is making fundamentally flawed assumptions about filesystem used space accounting.
It is wrong to assuming that the available capacity of a given empty filesystem will never change. Assumptions like this have been invalid for decades because the available space can change based on the underlying configuration or the filesystem. e.g. different versions of mkfs.xfs set different default parameters and so simply changing the version of xfsprogs you use between the two comparision tests will make it fail....
Quoted 4 lines
And, well, XFS also has XFS_IOC_{GS}ET_RESBLKS ioctls that allow userspace to change the amount of reserved blocks. They were introduced in 1997, and since then we've changed the default reservation the filesystem takes at least a dozen times.
Thanks a lot for valuable info.

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
Quoted 1 line
It might be a false positive / bug in the test, but it's at least a changed behavior.
Yup, any test that assumes "available space" does not change from kernel version to kernel version is flawed. There is no guarantee that this ever stays the same, nor that it needs to stay the same.
I'm sorry I was not clear. Test [1] does not measure "available space" between kernel releases. It just run df command with parameters, saves it's output and compares "1024-blocks" and "Used" columns of df output with stat output:

Annotating what these do...

Quoted 8 lines

		local total=$(stat -f mntpoint --printf=%b)  # number of blocks allocated
		local free=$(stat -f mntpoint --printf=%f)   # free blocks in filesystem
		local used=$((total-free))                   # (number of blocks free)

		local bsize=$(stat -f mntpoint --printf=%s)  # block size ("for faster transfers")
		total=$((($total * $bsize + 512)/ 1024))     # number of 1k blocks allocated?
		used=$((($used * $bsize + 512) / 1024))      # number of 1k blocks used?

Quoted 8 lines

And comparison with "$used" is what sometimes fails.
BTW this happens on both distros when loop device is on tmpfs. I'm trying to trigger it on ext4 and btrfs, not successful so far. Looks like it's tmpfs related.
If that's really expected, we might remove this check for used for XFS (not sure if check only for total makes sense).

It's kind of hard to follow this test, but it seems to be trying to ensure that df output is consistent with du (statfs) numbers, before and after creating and removing a 1MB file. I guess it's literally testing df itself, i.e. that it actually presents the numbers it obtained from statfs.

AFAICT the difference in the failure is 1024 1K blocks, which is the size the file that's been created and removed during the test.

My best guess is that this is xfs inode deferred inode inactivation hanging onto the space a little longer than the test expects.

This is probably because the new-ish xfs inode inactivation no longer blocks on inode garbage collection during statfs().

IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.

(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)

Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.

-Eric

Signature · 1 line

Mailing list info: https://lists.linux.it/listinfo/ltp

Petr Vorel

author

2022-08-18 15:25 UTC #6 lore raw

Hi Eric, all,

Quoted 94 lines · Petr Vorel

Quoted 46 lines
Hi Dave,
Quoted 37 lines · Petr Vorel
Quoted 18 lines
Hi all,
LTP test df01.sh found different size of loop device in v5.19. Test uses loop device formatted on various file systems, only XFS fails. It randomly fails during verifying that loop size usage changes:
grep ${TST_DEVICE} output | grep -q "${total}.*${used}" [1]
How to reproduce:
# PATH="/opt/ltp/testcases/bin:$PATH" df01.sh -f xfs # it needs several tries to hit
df saved output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
/dev/loop0 256672 16208 240464 7% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
df output:
Filesystem 1024-blocks Used Available Capacity Mounted on
...
Quoted 4 lines
tmpfs               201780       0    201780       0% /run/user/0
/dev/loop0          256672   15160    241512       6% /tmp/LTP_df01.1kRwoUCCR7/mntpoint
=> different size
df01 4 TFAIL: 'df -k -P' failed, not expected.
Yup, most likely because we changed something in XFS related to internal block reservation spaces. That is, the test is making fundamentally flawed assumptions about filesystem used space accounting.
It is wrong to assuming that the available capacity of a given empty filesystem will never change. Assumptions like this have been invalid for decades because the available space can change based on the underlying configuration or the filesystem. e.g. different versions of mkfs.xfs set different default parameters and so simply changing the version of xfsprogs you use between the two comparision tests will make it fail....
Quoted 4 lines
And, well, XFS also has XFS_IOC_{GS}ET_RESBLKS ioctls that allow userspace to change the amount of reserved blocks. They were introduced in 1997, and since then we've changed the default reservation the filesystem takes at least a dozen times.
Thanks a lot for valuable info.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
Quoted 1 line
It might be a false positive / bug in the test, but it's at least a changed behavior.
Yup, any test that assumes "available space" does not change from kernel version to kernel version is flawed. There is no guarantee that this ever stays the same, nor that it needs to stay the same.
I'm sorry I was not clear. Test [1] does not measure "available space" between kernel releases. It just run df command with parameters, saves it's output and compares "1024-blocks" and "Used" columns of df output with stat output:
Annotating what these do...
Quoted 8 lines
		local total=$(stat -f mntpoint --printf=%b)  # number of blocks allocated
		local free=$(stat -f mntpoint --printf=%f)   # free blocks in filesystem
		local used=$((total-free))                   # (number of blocks free)
		local bsize=$(stat -f mntpoint --printf=%s)  # block size ("for faster transfers")
		total=$((($total * $bsize + 512)/ 1024))     # number of 1k blocks allocated?
		used=$((($used * $bsize + 512) / 1024))      # number of 1k blocks used?
Quoted 8 lines
And comparison with "$used" is what sometimes fails.
BTW this happens on both distros when loop device is on tmpfs. I'm trying to trigger it on ext4 and btrfs, not successful so far. Looks like it's tmpfs related.
If that's really expected, we might remove this check for used for XFS (not sure if check only for total makes sense).
It's kind of hard to follow this test, but it seems to be trying to ensure that df output is consistent with du (statfs) numbers, before and after creating and removing a 1MB file. I guess it's literally testing df itself, i.e. that it actually presents the numbers it obtained from statfs.
AFAICT the difference in the failure is 1024 1K blocks, which is the size the file that's been created and removed during the test.
My best guess is that this is xfs inode deferred inode inactivation hanging onto the space a little longer than the test expects.
This is probably because the new-ish xfs inode inactivation no longer blocks on inode garbage collection during statfs().
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.

Quoted 5 lines

(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.

Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.

Thanks for your input!

Kind regards,
Petr

[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests

Quoted 1 line

-Eric

+++ testcases/commands/df/df01.sh
@@ -63,6 +63,10 @@ df_test()
tst_res TFAIL "'$cmd' failed."
fi

+ if [ "$DF_FS_TYPE" = xfs ]; then
+ tst_sleep 50ms
+ fi
+
ROD_SILENT rm -rf mntpoint/testimg

 	# flush file system buffers, then we can get the actual sizes.

Eric Sandeen

2022-08-18 16:05 UTC #7 lore raw

Quoted 2 lines · Petr Vorel

Hi Eric, all,

...

Quoted 26 lines

Quoted 10 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh

Review df_test() · line 69

Quoted 7 lines

(quoted diff) +4 -0

@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
 
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+

Probably worth at least a comment as to why ...

Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...

-Eric

Quoted 4 lines

(quoted diff) +0 -0

 	ROD_SILENT rm -rf mntpoint/testimg
 
 	# flush file system buffers, then we can get the actual sizes.

Darrick J. Wong

2022-08-18 16:27 UTC #8 lore raw

Quoted 42 lines · Eric Sandeen

Quoted 2 lines · Petr Vorel
Hi Eric, all,
...
Quoted 26 lines
Quoted 10 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
Review df_test() · line 69
Quoted 7 lines
(quoted diff) +4 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
 
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...

Quoted 2 lines

Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...

A fsfreeze -f / fsfreeze -u cycle will force all the background garbage collection to run to completion when precise free space accounting is being tested.

--D

Quoted 6 lines

-Eric

Quoted 4 lines

(quoted diff) +0 -0

 	ROD_SILENT rm -rf mntpoint/testimg
 
 	# flush file system buffers, then we can get the actual sizes.

Petr Vorel

author

2022-08-18 17:01 UTC #9 lore raw

Quoted 36 lines · Eric Sandeen

Quoted 34 lines · Petr Vorel
Quoted 1 line
Hi Eric, all,
...
Quoted 27 lines
Quoted 9 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
(quoted diff) +0 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi

Quoted 6 lines

Quoted 6 lines
Review df_test() · line 68
Quoted 4 lines
(quoted diff) +4 -0
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...

Sure, that was just to document possible fix. BTW even 200ms was not reliable in the long run => not a good solution.

Quoted 6 lines

Quoted 2 lines
Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...
A fsfreeze -f / fsfreeze -u cycle will force all the background garbage collection to run to completion when precise free space accounting is being tested.

Thanks for a hint, do you mean to put it into df_test after creating file with dd to wrap second df_verify (calls df) and df_check (runs stat and compare values)? Because that does not help - it fails when running in the loop (managed to break after 5th run).

Kind regards,
Petr

df_test()
{
local cmd="$1 -P"

df_verify $cmd
if [ $? -ne 0 ]; then
return
fi

df_check $cmd
if [ $? -ne 0 ]; then
tst_res TFAIL "'$cmd' failed, not expected." return
fi

ROD_SILENT dd if=/dev/zero of=mntpoint/testimg bs=1024 count=1024

if [ "$DF_FS_TYPE" = xfs ]; then
fsfreeze -f $TST_MNTPOINT
fi

df_verify $cmd

df_check $cmd
if [ $? -eq 0 ]; then
tst_res TPASS "'$cmd' passed."
else
tst_res TFAIL "'$cmd' failed."
fi

if [ "$DF_FS_TYPE" = xfs ]; then
fsfreeze -u $TST_MNTPOINT
fi

ROD_SILENT rm -rf mntpoint/testimg

# flush file system buffers, then we can get the actual sizes. sync
}

df_verify()

{
	$@ >output 2>&1
	if [ $? -ne 0 ]; then
		grep -q -E "unrecognized option | invalid option" output
		if [ $? -eq 0 ]; then
			tst_res TCONF "'$@' not supported."
			return 32
		else
			tst_res TFAIL "'$@' failed."
			cat output
			return 1
		fi
	fi
}

df_check()

{
	if [ "$(echo $@)" = "df -i -P" ]; then
		local total=$(stat -f mntpoint --printf=%c)
		local free=$(stat -f mntpoint --printf=%d)
		local used=$((total-free))
	else
		local total=$(stat -f mntpoint --printf=%b)
		local free=$(stat -f mntpoint --printf=%f)
		local used=$((total-free))
		local bsize=$(stat -f mntpoint --printf=%s)
		total=$((($total * $bsize + 512)/ 1024))
		used=$((($used * $bsize + 512) / 1024))
	fi

	grep ${TST_DEVICE} output | grep -q "${total}.*${used}"
	if [ $? -ne 0 ]; then
		echo "total: ${total}, used: ${used}"
		echo "df saved output:"
		cat output
		echo "df output:"
		$@
		return 1
	fi
}

Quoted 7 lines

--D
Quoted 5 lines
-Eric
Quoted 3 lines
(quoted diff) +0 -0
 	ROD_SILENT rm -rf mntpoint/testimg
 
 	# flush file system buffers, then we can get the actual sizes.

Signature · 1 line

Mailing list info: https://lists.linux.it/listinfo/ltp

Eric Sandeen

2022-08-18 21:27 UTC #10 lore raw

Quoted 49 lines · Petr Vorel

Quoted 36 lines · Eric Sandeen
Quoted 34 lines · Petr Vorel
Quoted 1 line
Hi Eric, all,
...
Quoted 27 lines
Quoted 9 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
(quoted diff) +0 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
Quoted 6 lines
Quoted 6 lines
Review df_test() · line 68
Quoted 4 lines
(quoted diff) +4 -0
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...
Sure, that was just to document possible fix. BTW even 200ms was not reliable in the long run => not a good solution.
Quoted 2 lines
Quoted 2 lines
Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...

Quoted 6 lines

Quoted 3 lines
A fsfreeze -f / fsfreeze -u cycle will force all the background garbage collection to run to completion when precise free space accounting is being tested.
Thanks for a hint, do you mean to put it into df_test after creating file with dd to wrap second df_verify (calls df) and df_check (runs stat and compare values)? Because that does not help - it fails when running in the loop (managed to break after 5th run).

I think it would go after you remove the file, to ensure that no space usage changes are pending when you check.

<tests>

This seems to work fine (pseudopatch):

ROD_SILENT rm -rf mntpoint/testimg

+       # Ensure free space change can be seen by statfs
+       fsfreeze -f $TST_MNTPOINT
+       fsfreeze -u $TST_MNTPOINT

        # flush file system buffers, then we can get the actual sizes.
        sync

(although: what's the difference between $TST_MNTPOINT and mountpoint/ ?)

You just don't want to accidentally freeze the root filesystem ;)

-Eric

Petr Vorel

author

2022-08-19 16:00 UTC #11 lore raw

Quoted 66 lines · Petr Vorel

Quoted 49 lines
Quoted 36 lines · Eric Sandeen
Quoted 34 lines · Petr Vorel
Quoted 1 line
Hi Eric, all,
...
Quoted 27 lines
Quoted 9 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
(quoted diff) +0 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
Quoted 6 lines
Quoted 6 lines
Review df_test() · line 68
Quoted 4 lines
(quoted diff) +4 -0
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...
Sure, that was just to document possible fix. BTW even 200ms was not reliable in the long run => not a good solution.
Quoted 2 lines
Quoted 2 lines
Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...
Quoted 6 lines
Quoted 3 lines
A fsfreeze -f / fsfreeze -u cycle will force all the background garbage collection to run to completion when precise free space accounting is being tested.
Thanks for a hint, do you mean to put it into df_test after creating file with dd to wrap second df_verify (calls df) and df_check (runs stat and compare values)? Because that does not help - it fails when running in the loop (managed to break after 5th run).
I think it would go after you remove the file, to ensure that no space usage changes are pending when you check.
<tests>
This seems to work fine (pseudopatch):
ROD_SILENT rm -rf mntpoint/testimg

Review

Quoted 3 lines

(quoted diff) +3 -0

+       # Ensure free space change can be seen by statfs
+       fsfreeze -f $TST_MNTPOINT
+       fsfreeze -u $TST_MNTPOINT

It looks like it works. We might add small binary which just calls these 2 ioctl (FIFREEZE and FITHAW), just to be friendly to people on embedded environment with minimal dependencies (yes, some people might not install util-linux).

Quoted 5 lines

        # flush file system buffers, then we can get the actual sizes.
        sync
(although: what's the difference between $TST_MNTPOINT and mountpoint/ ?)

Thanks for a report, fixed in 96ae882d3 ("df01.sh: Use $TST_MNTPOINT")

Quoted 1 line

You just don't want to accidentally freeze the root filesystem ;)

Sure :)

Kind regards,
Petr

Quoted 1 line

-Eric

Signature · 1 line

Mailing list info: https://lists.linux.it/listinfo/ltp

Bird, Tim

2022-08-19 16:07 UTC #12 lore raw

Quoted 71 lines

-----Original Message-----
From: ltp <[email protected]> On Behalf Of Petr Vorel
Quoted 65 lines · Petr Vorel
Quoted 49 lines
Quoted 36 lines · Eric Sandeen
Quoted 34 lines · Petr Vorel
Quoted 1 line
Hi Eric, all,
...
Quoted 27 lines
Quoted 9 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
(quoted diff) +0 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
Quoted 6 lines
Quoted 6 lines
Review df_test() · line 68
Quoted 4 lines
(quoted diff) +4 -0
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...
Sure, that was just to document possible fix. BTW even 200ms was not reliable in the long run => not a good solution.
Quoted 2 lines
Quoted 2 lines
Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...
Quoted 6 lines
Quoted 3 lines
A fsfreeze -f / fsfreeze -u cycle will force all the background garbage collection to run to completion when precise free space accounting is being tested.
Thanks for a hint, do you mean to put it into df_test after creating file with dd to wrap second df_verify (calls df) and df_check (runs stat and compare values)? Because that does not help - it fails when running in the loop (managed to break after 5th run).
I think it would go after you remove the file, to ensure that no space usage changes are pending when you check.
<tests>
This seems to work fine (pseudopatch):
ROD_SILENT rm -rf mntpoint/testimg

Quoted 7 lines

Review
Quoted 3 lines
(quoted diff) +3 -0
+       # Ensure free space change can be seen by statfs
+       fsfreeze -f $TST_MNTPOINT
+       fsfreeze -u $TST_MNTPOINT
It looks like it works. We might add small binary which just calls these 2 ioctl (FIFREEZE and FITHAW), just to be friendly to people on embedded environment with minimal dependencies (yes, some people might not install util-linux).

Thank you!! It's good to know that small embedded systems are still considered, and the consideration is much appreciated! :-)

Let me know if you'd like me to try writing the utility.
-- Tim

Signature · 1 line

Mailing list info: https://lists.linux.it/listinfo/ltp

Petr Vorel

author

2022-08-19 19:30 UTC #13 lore raw

Quoted 79 lines

Quoted 71 lines
-----Original Message-----
From: ltp <[email protected]> On Behalf Of Petr Vorel
Quoted 65 lines · Petr Vorel
Quoted 49 lines
Quoted 36 lines · Eric Sandeen
Quoted 34 lines · Petr Vorel
Quoted 1 line
Hi Eric, all,
...
Quoted 27 lines
Quoted 9 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
(quoted diff) +0 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
Quoted 6 lines
Quoted 6 lines
Review df_test() · line 68
Quoted 4 lines
(quoted diff) +4 -0
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...
Sure, that was just to document possible fix. BTW even 200ms was not reliable in the long run => not a good solution.
Quoted 2 lines
Quoted 2 lines
Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...
Quoted 6 lines
Quoted 3 lines
A fsfreeze -f / fsfreeze -u cycle will force all the background garbage collection to run to completion when precise free space accounting is being tested.
Thanks for a hint, do you mean to put it into df_test after creating file with dd to wrap second df_verify (calls df) and df_check (runs stat and compare values)? Because that does not help - it fails when running in the loop (managed to break after 5th run).
I think it would go after you remove the file, to ensure that no space usage changes are pending when you check.
<tests>
This seems to work fine (pseudopatch):
ROD_SILENT rm -rf mntpoint/testimg
Quoted 7 lines
Review
Quoted 3 lines
(quoted diff) +3 -0
+       # Ensure free space change can be seen by statfs
+       fsfreeze -f $TST_MNTPOINT
+       fsfreeze -u $TST_MNTPOINT
It looks like it works. We might add small binary which just calls these 2 ioctl (FIFREEZE and FITHAW), just to be friendly to people on embedded environment with minimal dependencies (yes, some people might not install util-linux).

Quoted 4 lines

Thank you!! It's good to know that small embedded systems are still considered, and the consideration is much appreciated! :-)
Let me know if you'd like me to try writing the utility.

Thank you, Tim! I'll Cc you when sending this patch (likely next week).

You might also appreciate our effort to lower down loop device size (used for all_filesystems): https://lore.kernel.org/ltp/Yv%2FkVXSK0xJGb3RO@pevik/

Kind regards,
Petr

Quoted 1 line

-- Tim

Dave Chinner

2022-08-18 21:03 UTC #14 lore raw

In reply to Eric Sandeen

Quoted 42 lines · Eric Sandeen

Quoted 2 lines · Petr Vorel
Hi Eric, all,
...
Quoted 26 lines
Quoted 10 lines
IOWS, I think the test expects that free space is reflected in statfs numbers immediately after a file is removed, and that's no longer the case here. They change in between the df check and the statfs check.
(The test isn't just checking that the values are correct, it is checking that the values are /immediately/ correct.)
Putting a "sleep 1" after the "rm -f" in the test seems to fix it; IIRC the max time to wait for inodegc is 1s. This does slow the test down a bit.
Sure, it looks like we can sleep just 50ms on my hw (although better might be to poll for the result [1]), I just wanted to make sure there is no bug/regression before hiding it with sleep.
Thanks for your input!
Kind regards,
Petr
[1] https://people.kernel.org/metan/why-sleep-is-almost-never-acceptable-in-tests
Quoted 1 line
-Eric
+++ testcases/commands/df/df01.sh
Review df_test() · line 69
Quoted 7 lines
(quoted diff) +4 -0
@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi
 
+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+
Probably worth at least a comment as to why ...

Quoted 2 lines

Dave / Darrick / Brian - I'm not sure how long it might take to finish inodegc? A too-short sleep will let the flakiness remain ...

For a single 1MB file? inodegc is delayed 1 jiffie (max 10ms). If it's processed immediately because everything else is idle and nothing blocks, it will be done in 250-500 microsends.

Of course, the moment you put such tests in a VM on a host that is running other VMs nothing is ever idle and inodegc delays are outside the control of the guest kernel/filesystem....

Cheers,

Dave.

Signature · 5 lines

Dave Chinner
[email protected]

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp