On Thu, Aug 18, 2022 at 11:05:33AM -0500, Eric Sandeen wrote:

quoted

On 8/18/22 10:25 AM, Petr Vorel wrote:

quoted

Hi Eric, all,

quoted

...

quoted

quoted

quoted

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

quoted

quoted

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

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

quoted

Thanks for your input!

quoted

quoted

Kind regards,
Petr

quoted

quoted

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

quoted

quoted

quoted

-Eric

quoted

quoted

+++ testcases/commands/df/df01.sh

@@ -63,6 +63,10 @@ df_test()
 		tst_res TFAIL "'$cmd' failed."
 	fi

quoted

quoted

+	if [ "$DF_FS_TYPE" = xfs ]; then
+		tst_sleep 50ms
+	fi
+

quoted

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

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
}

--D

quoted

-Eric

quoted

quoted

 	ROD_SILENT rm -rf mntpoint/testimg

quoted

quoted

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