[PATCH] generic/402: fix for updated behavior of timestamp limits

[PATCH] generic/402: fix for updated behavior of timestamp limits

[Deepa Dinamani]

The mount behavior will not be altered because of the unsupported
timestamps on the filesystems.

Adjust the test accordingly.

An updated series to be posted after the merge window is hosted at
<https://github.com/deepa-hub/vfs/tree/limits>

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 common/rc             | 36 +++++++++---------
 tests/generic/402     | 87 ++++++++++++++++---------------------------
 tests/generic/402.out |  2 +-
 3 files changed, 53 insertions(+), 72 deletions(-)

diff --git a/common/rc b/common/rc
index 25203bb4..39a2deb0 100644
--- a/common/rc
+++ b/common/rc
@@ -1959,16 +1959,9 @@ _run_aiodio()
     return $status
 }
 
-# this test requires y2038 sysfs switch and filesystem
-# timestamp ranges support.
-_require_y2038()
+_require_timestamp_range()
 {
 	local device=${1:-$TEST_DEV}
-	local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
-
-	if [ ! -e $sysfsdir ]; then
-		_notrun "no kernel support for y2038 sysfs switch"
-	fi
 
 	local tsmin tsmax
 	read tsmin tsmax <<<$(_filesystem_timestamp_range $device)
@@ -1980,23 +1973,32 @@ _require_y2038()
 _filesystem_timestamp_range()
 {
 	local device=${1:-$TEST_DEV}
+	u32max=$(((1<<32)-1))
+	s32min=-$((1<<31))
+	s32max=$(((1<<31)-1))
+	s64max=$(((1<<63)-1))
+	s64min=$((1<<63))
+
 	case $FSTYP in
-	ext4)
+	ext2)
+		echo "$s32min $s32max"
+		;;
+	ext3|ext4)
 		if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | cut -d: -f2) -gt 128 ]; then
-			echo "-2147483648 15032385535"
+			printf "%d %d\n" $s32min 0x37fffffff
 		else
-			echo "-2147483648 2147483647"
+			echo "$s32min $s32max"
 		fi
 		;;
 
-	xfs)
-		echo "-2147483648 2147483647"
-		;;
 	jfs)
-		echo "0 4294967295"
+		echo "0 $u32max"
 		;;
-	f2fs)
-		echo "-2147483648 2147483647"
+	xfs)
+		echo "$s32min $s32max"
+		;;
+	btrfs)
+		echo "$s64min $s64max"
 		;;
 	*)
 		echo "-1 -1"
diff --git a/tests/generic/402 b/tests/generic/402
index f742fedd..dd136ec2 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -4,15 +4,10 @@
 #
 # FS QA Test 402
 #
-# Tests to verify policy for filesystem timestamps for
-# supported ranges:
-# 1. Verify filesystem rw mount according to sysctl
-# timestamp_supported.
-# 2. Verify timestamp clamping for timestamps beyond max
-# timestamp supported.
+# Test to verify filesystem timestamps for supported ranges.
 #
-# Exit status 1: either or both tests above fail.
-# Exit status 0: both the above tests pass.
+# Exit status 1: test failed.
+# Exit status 0: test passed.
 #
 seq=`basename $0`
 seqres=$RESULT_DIR/$seq
@@ -49,47 +44,59 @@ check_stat()
 	prev_timestamp="$timestamp;$timestamp"
 	if [ $prev_timestamp != $stat_timestamp ]; then
 		echo "$prev_timestamp != $stat_timestamp" | tee -a $seqres.full
+		return 1
 	fi
+	return 0
 }
 
 run_test_individual()
 {
+	fail=0
 	file=$1
 	timestamp=$2
 	update_time=$3
 
 	# check if the time needs update
 	if [ $update_time -eq 1 ]; then
-		echo "Updating file: $file to timestamp `date -d @$timestamp`"  >> $seqres.full
+		echo "Updating file: $file to timestamp $timestamp"  >> $seqres.full
 		$XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file
 		if [ $? -ne 0 ]; then
 			echo "Failed to update times on $file" | tee -a $seqres.full
+			fail=1
 		fi
 	fi
 
-	tsclamp=$(($timestamp>$tsmax?$tsmax:$timestamp))
-	echo "Checking file: $file Updated timestamp is `date -d @$tsclamp`"  >> $seqres.full
-	check_stat $file $tsclamp
+	tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp))
+	echo "Checking file: $file Updated timestamp is $tsclamp"  >> $seqres.full
+	if ! check_stat $file $tsclamp; then
+		fail=1
+	fi
+	return $fail
 }
 
 run_test()
 {
+	fail=0
 	update_time=$1
 
 	n=1
 
 	for TIME in "${TIMESTAMPS[@]}"; do
-		run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time
+		if ! run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time; then
+			fail=1
+		fi
 		((n++))
 	done
+
+	return $fail
 }
 
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
-_require_y2038 $SCRATCH_DEV
+_require_timestamp_range $SCRATCH_DEV
 
 read tsmin tsmax <<<$(_filesystem_timestamp_range $SCRATCH_DEV)
-echo min supported timestamp $tsmin $(date --date=@$tsmin) >> $seqres.full
-echo max supported timestamp $tsmax $(date --date=@$tsmax) >> $seqres.full
+echo min supported timestamp $tsmin >> $seqres.full
+echo max supported timestamp $tsmax >> $seqres.full
 
 # Test timestamps array
 
@@ -97,45 +104,14 @@ declare -a TIMESTAMPS=(
 	$tsmin
 	0
 	$tsmax
+	$((tsmax/2))
 	$((tsmax+1))
-	4294967295
-	8589934591
-	34359738367
 )
 
-# Max timestamp is hardcoded to Mon Jan 18 19:14:07 PST 2038
-sys_tsmax=2147483647
-echo "max timestamp that needs to be supported by fs for rw mount is" \
-	"$((sys_tsmax+1)) $(date --date=@$((sys_tsmax+1)))" >> $seqres.full
-
-read ts_check <<<$(cat /proc/sys/fs/fs-timestamp-check-on)
-
 _scratch_mount
 result=$?
 
-if [ $ts_check -ne 0 ]; then
-	echo "sysctl filesystem timestamp check is on" >> $seqres.full
-	# check for mount failure if the minimum requirement for max timestamp
-	# supported is not met.
-	if [ $sys_tsmax -ge $tsmax ]; then
-		if [ $result -eq 0 ]; then
-			echo "mount test failed"  | tee -a $seqres.full
-			exit
-		fi
-	else
-		if [ $result -ne 0 ]; then
-			echo "failed to mount $SCRATCH_DEV"  | tee -a $seqres.full
-			exit
-		fi
-	fi
-else
-	# if sysctl switch is off then mount should succeed always.
-	echo "sysctl filesystem timestamp check is off" >> $seqres.full
-	if [ $result -ne 0 ]; then
-		echo "failed to mount $SCRATCH_DEV and timestamp check is off"  >> $seqres.full
-		exit
-	fi
-fi
+status=0
 
 # Begin test case 1
 echo "In memory timestamps update test start" >> $seqres.full
@@ -143,7 +119,9 @@ echo "In memory timestamps update test start" >> $seqres.full
 # update time on the file
 update_time=1
 
-run_test $update_time
+if ! run_test $update_time; then
+	status=1
+fi
 
 echo "In memory timestamps update complete" >> $seqres.full
 
@@ -162,12 +140,13 @@ update_time=0
 echo "On disk timestamps update test start" >> $seqres.full
 
 # Re-run test
-run_test $update_time
+if ! run_test $update_time; then
+	status=1
+fi
 
 echo "On disk timestamps update test complete" >> $seqres.full
 
-echo "y2038 inode timestamp tests completed successfully"
+echo "inode timestamp tests completed status $status"
 
 # success, all done
-status=0
-exit
+exit $status
diff --git a/tests/generic/402.out b/tests/generic/402.out
index 6c5b9308..4500e6c7 100644
--- a/tests/generic/402.out
+++ b/tests/generic/402.out
@@ -1,2 +1,2 @@
 QA output created by 402
-y2038 inode timestamp tests completed successfully
+inode timestamp tests completed status 0
-- 
2.17.1

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Eryu Guan]

On Thu, Jul 18, 2019 at 09:12:31PM -0700, Deepa Dinamani wrote:
> The mount behavior will not be altered because of the unsupported
> timestamps on the filesystems.

It'd be better to provide more details in the commit log, e.g. what
kind of mount behavior and what's the changes being made to the tests.

> 
> Adjust the test accordingly.
> 
> An updated series to be posted after the merge window is hosted at
> <https://github.com/deepa-hub/vfs/tree/limits>
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>

Thanks for the update! But I'd like to wait for the kernel patches
merged into mainline first. Would you please send out a notification by
replying this thread then? Thanks a lot!

Eryu

> ---
>  common/rc             | 36 +++++++++---------
>  tests/generic/402     | 87 ++++++++++++++++---------------------------
>  tests/generic/402.out |  2 +-
>  3 files changed, 53 insertions(+), 72 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 25203bb4..39a2deb0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1959,16 +1959,9 @@ _run_aiodio()
>      return $status
>  }
>  
> -# this test requires y2038 sysfs switch and filesystem
> -# timestamp ranges support.
> -_require_y2038()
> +_require_timestamp_range()
>  {
>  	local device=${1:-$TEST_DEV}
> -	local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
> -
> -	if [ ! -e $sysfsdir ]; then
> -		_notrun "no kernel support for y2038 sysfs switch"
> -	fi
>  
>  	local tsmin tsmax
>  	read tsmin tsmax <<<$(_filesystem_timestamp_range $device)
> @@ -1980,23 +1973,32 @@ _require_y2038()
>  _filesystem_timestamp_range()
>  {
>  	local device=${1:-$TEST_DEV}
> +	u32max=$(((1<<32)-1))
> +	s32min=-$((1<<31))
> +	s32max=$(((1<<31)-1))
> +	s64max=$(((1<<63)-1))
> +	s64min=$((1<<63))
> +
>  	case $FSTYP in
> -	ext4)
> +	ext2)
> +		echo "$s32min $s32max"
> +		;;
> +	ext3|ext4)
>  		if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | cut -d: -f2) -gt 128 ]; then
> -			echo "-2147483648 15032385535"
> +			printf "%d %d\n" $s32min 0x37fffffff
>  		else
> -			echo "-2147483648 2147483647"
> +			echo "$s32min $s32max"
>  		fi
>  		;;
>  
> -	xfs)
> -		echo "-2147483648 2147483647"
> -		;;
>  	jfs)
> -		echo "0 4294967295"
> +		echo "0 $u32max"
>  		;;
> -	f2fs)
> -		echo "-2147483648 2147483647"
> +	xfs)
> +		echo "$s32min $s32max"
> +		;;
> +	btrfs)
> +		echo "$s64min $s64max"
>  		;;
>  	*)
>  		echo "-1 -1"
> diff --git a/tests/generic/402 b/tests/generic/402
> index f742fedd..dd136ec2 100755
> --- a/tests/generic/402
> +++ b/tests/generic/402
> @@ -4,15 +4,10 @@
>  #
>  # FS QA Test 402
>  #
> -# Tests to verify policy for filesystem timestamps for
> -# supported ranges:
> -# 1. Verify filesystem rw mount according to sysctl
> -# timestamp_supported.
> -# 2. Verify timestamp clamping for timestamps beyond max
> -# timestamp supported.
> +# Test to verify filesystem timestamps for supported ranges.
>  #
> -# Exit status 1: either or both tests above fail.
> -# Exit status 0: both the above tests pass.
> +# Exit status 1: test failed.
> +# Exit status 0: test passed.
>  #
>  seq=`basename $0`
>  seqres=$RESULT_DIR/$seq
> @@ -49,47 +44,59 @@ check_stat()
>  	prev_timestamp="$timestamp;$timestamp"
>  	if [ $prev_timestamp != $stat_timestamp ]; then
>  		echo "$prev_timestamp != $stat_timestamp" | tee -a $seqres.full
> +		return 1
>  	fi
> +	return 0
>  }
>  
>  run_test_individual()
>  {
> +	fail=0
>  	file=$1
>  	timestamp=$2
>  	update_time=$3
>  
>  	# check if the time needs update
>  	if [ $update_time -eq 1 ]; then
> -		echo "Updating file: $file to timestamp `date -d @$timestamp`"  >> $seqres.full
> +		echo "Updating file: $file to timestamp $timestamp"  >> $seqres.full
>  		$XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file
>  		if [ $? -ne 0 ]; then
>  			echo "Failed to update times on $file" | tee -a $seqres.full
> +			fail=1
>  		fi
>  	fi
>  
> -	tsclamp=$(($timestamp>$tsmax?$tsmax:$timestamp))
> -	echo "Checking file: $file Updated timestamp is `date -d @$tsclamp`"  >> $seqres.full
> -	check_stat $file $tsclamp
> +	tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp))
> +	echo "Checking file: $file Updated timestamp is $tsclamp"  >> $seqres.full
> +	if ! check_stat $file $tsclamp; then
> +		fail=1
> +	fi
> +	return $fail
>  }
>  
>  run_test()
>  {
> +	fail=0
>  	update_time=$1
>  
>  	n=1
>  
>  	for TIME in "${TIMESTAMPS[@]}"; do
> -		run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time
> +		if ! run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time; then
> +			fail=1
> +		fi
>  		((n++))
>  	done
> +
> +	return $fail
>  }
>  
>  _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
> -_require_y2038 $SCRATCH_DEV
> +_require_timestamp_range $SCRATCH_DEV
>  
>  read tsmin tsmax <<<$(_filesystem_timestamp_range $SCRATCH_DEV)
> -echo min supported timestamp $tsmin $(date --date=@$tsmin) >> $seqres.full
> -echo max supported timestamp $tsmax $(date --date=@$tsmax) >> $seqres.full
> +echo min supported timestamp $tsmin >> $seqres.full
> +echo max supported timestamp $tsmax >> $seqres.full
>  
>  # Test timestamps array
>  
> @@ -97,45 +104,14 @@ declare -a TIMESTAMPS=(
>  	$tsmin
>  	0
>  	$tsmax
> +	$((tsmax/2))
>  	$((tsmax+1))
> -	4294967295
> -	8589934591
> -	34359738367
>  )
>  
> -# Max timestamp is hardcoded to Mon Jan 18 19:14:07 PST 2038
> -sys_tsmax=2147483647
> -echo "max timestamp that needs to be supported by fs for rw mount is" \
> -	"$((sys_tsmax+1)) $(date --date=@$((sys_tsmax+1)))" >> $seqres.full
> -
> -read ts_check <<<$(cat /proc/sys/fs/fs-timestamp-check-on)
> -
>  _scratch_mount
>  result=$?
>  
> -if [ $ts_check -ne 0 ]; then
> -	echo "sysctl filesystem timestamp check is on" >> $seqres.full
> -	# check for mount failure if the minimum requirement for max timestamp
> -	# supported is not met.
> -	if [ $sys_tsmax -ge $tsmax ]; then
> -		if [ $result -eq 0 ]; then
> -			echo "mount test failed"  | tee -a $seqres.full
> -			exit
> -		fi
> -	else
> -		if [ $result -ne 0 ]; then
> -			echo "failed to mount $SCRATCH_DEV"  | tee -a $seqres.full
> -			exit
> -		fi
> -	fi
> -else
> -	# if sysctl switch is off then mount should succeed always.
> -	echo "sysctl filesystem timestamp check is off" >> $seqres.full
> -	if [ $result -ne 0 ]; then
> -		echo "failed to mount $SCRATCH_DEV and timestamp check is off"  >> $seqres.full
> -		exit
> -	fi
> -fi
> +status=0
>  
>  # Begin test case 1
>  echo "In memory timestamps update test start" >> $seqres.full
> @@ -143,7 +119,9 @@ echo "In memory timestamps update test start" >> $seqres.full
>  # update time on the file
>  update_time=1
>  
> -run_test $update_time
> +if ! run_test $update_time; then
> +	status=1
> +fi
>  
>  echo "In memory timestamps update complete" >> $seqres.full
>  
> @@ -162,12 +140,13 @@ update_time=0
>  echo "On disk timestamps update test start" >> $seqres.full
>  
>  # Re-run test
> -run_test $update_time
> +if ! run_test $update_time; then
> +	status=1
> +fi
>  
>  echo "On disk timestamps update test complete" >> $seqres.full
>  
> -echo "y2038 inode timestamp tests completed successfully"
> +echo "inode timestamp tests completed status $status"
>  
>  # success, all done
> -status=0
> -exit
> +exit $status
> diff --git a/tests/generic/402.out b/tests/generic/402.out
> index 6c5b9308..4500e6c7 100644
> --- a/tests/generic/402.out
> +++ b/tests/generic/402.out
> @@ -1,2 +1,2 @@
>  QA output created by 402
> -y2038 inode timestamp tests completed successfully
> +inode timestamp tests completed status 0
> -- 
> 2.17.1
> 

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Deepa Dinamani]

On Sun, Jul 21, 2019 at 9:47 AM Eryu Guan <guaneryu@gmail.com> wrote:
>
> On Thu, Jul 18, 2019 at 09:12:31PM -0700, Deepa Dinamani wrote:
> > The mount behavior will not be altered because of the unsupported
> > timestamps on the filesystems.
>
> It'd be better to provide more details in the commit log, e.g. what
> kind of mount behavior and what's the changes being made to the tests.
>
> >
> > Adjust the test accordingly.
> >
> > An updated series to be posted after the merge window is hosted at
> > <https://github.com/deepa-hub/vfs/tree/limits>
> >
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
>
> Thanks for the update! But I'd like to wait for the kernel patches
> merged into mainline first. Would you please send out a notification by
> replying this thread then? Thanks a lot!

This series has been merged now:
https://git.kernel.org/torvalds/c/cfb82e1df8b7c76991ea12958855897c2fb4debc

-Deepa

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Eryu Guan]

On Thu, Jul 18, 2019 at 09:12:31PM -0700, Deepa Dinamani wrote:
> The mount behavior will not be altered because of the unsupported
> timestamps on the filesystems.
> 
> Adjust the test accordingly.

Thanks for the heads-up about the merge of the fixes

https://git.kernel.org/torvalds/c/cfb82e1df8b7c76991ea12958855897c2fb4debc

> 
> An updated series to be posted after the merge window is hosted at
> <https://github.com/deepa-hub/vfs/tree/limits>
> 
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
>
> ---
>  common/rc             | 36 +++++++++---------
>  tests/generic/402     | 87 ++++++++++++++++---------------------------
>  tests/generic/402.out |  2 +-
>  3 files changed, 53 insertions(+), 72 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 25203bb4..39a2deb0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1959,16 +1959,9 @@ _run_aiodio()
>      return $status
>  }
>  
> -# this test requires y2038 sysfs switch and filesystem
> -# timestamp ranges support.
> -_require_y2038()
> +_require_timestamp_range()
>  {
>  	local device=${1:-$TEST_DEV}
> -	local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
> -
> -	if [ ! -e $sysfsdir ]; then
> -		_notrun "no kernel support for y2038 sysfs switch"
> -	fi
>  
>  	local tsmin tsmax
>  	read tsmin tsmax <<<$(_filesystem_timestamp_range $device)
> @@ -1980,23 +1973,32 @@ _require_y2038()
>  _filesystem_timestamp_range()
>  {
>  	local device=${1:-$TEST_DEV}
> +	u32max=$(((1<<32)-1))
> +	s32min=-$((1<<31))
> +	s32max=$(((1<<31)-1))
> +	s64max=$(((1<<63)-1))
> +	s64min=$((1<<63))
> +
>  	case $FSTYP in
> -	ext4)
> +	ext2)
> +		echo "$s32min $s32max"
> +		;;
> +	ext3|ext4)
>  		if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | cut -d: -f2) -gt 128 ]; then
> -			echo "-2147483648 15032385535"
> +			printf "%d %d\n" $s32min 0x37fffffff
>  		else
> -			echo "-2147483648 2147483647"
> +			echo "$s32min $s32max"
>  		fi
>  		;;
>  
> -	xfs)
> -		echo "-2147483648 2147483647"
> -		;;
>  	jfs)
> -		echo "0 4294967295"
> +		echo "0 $u32max"
>  		;;
> -	f2fs)
> -		echo "-2147483648 2147483647"
> +	xfs)
> +		echo "$s32min $s32max"
> +		;;
> +	btrfs)
> +		echo "$s64min $s64max"
>  		;;
>  	*)
>  		echo "-1 -1"
> diff --git a/tests/generic/402 b/tests/generic/402
> index f742fedd..dd136ec2 100755
> --- a/tests/generic/402
> +++ b/tests/generic/402
> @@ -4,15 +4,10 @@
>  #
>  # FS QA Test 402
>  #
> -# Tests to verify policy for filesystem timestamps for
> -# supported ranges:
> -# 1. Verify filesystem rw mount according to sysctl
> -# timestamp_supported.
> -# 2. Verify timestamp clamping for timestamps beyond max
> -# timestamp supported.
> +# Test to verify filesystem timestamps for supported ranges.
>  #
> -# Exit status 1: either or both tests above fail.
> -# Exit status 0: both the above tests pass.
> +# Exit status 1: test failed.
> +# Exit status 0: test passed.

These exit status checks are not needed, please see below.

>  #
>  seq=`basename $0`
>  seqres=$RESULT_DIR/$seq
> @@ -49,47 +44,59 @@ check_stat()
>  	prev_timestamp="$timestamp;$timestamp"
>  	if [ $prev_timestamp != $stat_timestamp ]; then
>  		echo "$prev_timestamp != $stat_timestamp" | tee -a $seqres.full
> +		return 1

We already print error message on test failure, which will break the
golden image, so there's no need to return 0 or 1. All similar checks
and returns are not needed in other functions.

Thanks,
Eryu

>  	fi
> +	return 0
>  }
>  
>  run_test_individual()
>  {
> +	fail=0
>  	file=$1
>  	timestamp=$2
>  	update_time=$3
>  
>  	# check if the time needs update
>  	if [ $update_time -eq 1 ]; then
> -		echo "Updating file: $file to timestamp `date -d @$timestamp`"  >> $seqres.full
> +		echo "Updating file: $file to timestamp $timestamp"  >> $seqres.full
>  		$XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file
>  		if [ $? -ne 0 ]; then
>  			echo "Failed to update times on $file" | tee -a $seqres.full
> +			fail=1
>  		fi
>  	fi
>  
> -	tsclamp=$(($timestamp>$tsmax?$tsmax:$timestamp))
> -	echo "Checking file: $file Updated timestamp is `date -d @$tsclamp`"  >> $seqres.full
> -	check_stat $file $tsclamp
> +	tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp))
> +	echo "Checking file: $file Updated timestamp is $tsclamp"  >> $seqres.full
> +	if ! check_stat $file $tsclamp; then
> +		fail=1
> +	fi
> +	return $fail
>  }
>  
>  run_test()
>  {
> +	fail=0
>  	update_time=$1
>  
>  	n=1
>  
>  	for TIME in "${TIMESTAMPS[@]}"; do
> -		run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time
> +		if ! run_test_individual ${SCRATCH_MNT}/test_$n $TIME $update_time; then
> +			fail=1
> +		fi
>  		((n++))
>  	done
> +
> +	return $fail
>  }
>  
>  _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
> -_require_y2038 $SCRATCH_DEV
> +_require_timestamp_range $SCRATCH_DEV
>  
>  read tsmin tsmax <<<$(_filesystem_timestamp_range $SCRATCH_DEV)
> -echo min supported timestamp $tsmin $(date --date=@$tsmin) >> $seqres.full
> -echo max supported timestamp $tsmax $(date --date=@$tsmax) >> $seqres.full
> +echo min supported timestamp $tsmin >> $seqres.full
> +echo max supported timestamp $tsmax >> $seqres.full
>  
>  # Test timestamps array
>  
> @@ -97,45 +104,14 @@ declare -a TIMESTAMPS=(
>  	$tsmin
>  	0
>  	$tsmax
> +	$((tsmax/2))
>  	$((tsmax+1))
> -	4294967295
> -	8589934591
> -	34359738367
>  )
>  
> -# Max timestamp is hardcoded to Mon Jan 18 19:14:07 PST 2038
> -sys_tsmax=2147483647
> -echo "max timestamp that needs to be supported by fs for rw mount is" \
> -	"$((sys_tsmax+1)) $(date --date=@$((sys_tsmax+1)))" >> $seqres.full
> -
> -read ts_check <<<$(cat /proc/sys/fs/fs-timestamp-check-on)
> -
>  _scratch_mount
>  result=$?
>  
> -if [ $ts_check -ne 0 ]; then
> -	echo "sysctl filesystem timestamp check is on" >> $seqres.full
> -	# check for mount failure if the minimum requirement for max timestamp
> -	# supported is not met.
> -	if [ $sys_tsmax -ge $tsmax ]; then
> -		if [ $result -eq 0 ]; then
> -			echo "mount test failed"  | tee -a $seqres.full
> -			exit
> -		fi
> -	else
> -		if [ $result -ne 0 ]; then
> -			echo "failed to mount $SCRATCH_DEV"  | tee -a $seqres.full
> -			exit
> -		fi
> -	fi
> -else
> -	# if sysctl switch is off then mount should succeed always.
> -	echo "sysctl filesystem timestamp check is off" >> $seqres.full
> -	if [ $result -ne 0 ]; then
> -		echo "failed to mount $SCRATCH_DEV and timestamp check is off"  >> $seqres.full
> -		exit
> -	fi
> -fi
> +status=0
>  
>  # Begin test case 1
>  echo "In memory timestamps update test start" >> $seqres.full
> @@ -143,7 +119,9 @@ echo "In memory timestamps update test start" >> $seqres.full
>  # update time on the file
>  update_time=1
>  
> -run_test $update_time
> +if ! run_test $update_time; then
> +	status=1
> +fi
>  
>  echo "In memory timestamps update complete" >> $seqres.full
>  
> @@ -162,12 +140,13 @@ update_time=0
>  echo "On disk timestamps update test start" >> $seqres.full
>  
>  # Re-run test
> -run_test $update_time
> +if ! run_test $update_time; then
> +	status=1
> +fi
>  
>  echo "On disk timestamps update test complete" >> $seqres.full
>  
> -echo "y2038 inode timestamp tests completed successfully"
> +echo "inode timestamp tests completed status $status"
>  
>  # success, all done
> -status=0
> -exit
> +exit $status
> diff --git a/tests/generic/402.out b/tests/generic/402.out
> index 6c5b9308..4500e6c7 100644
> --- a/tests/generic/402.out
> +++ b/tests/generic/402.out
> @@ -1,2 +1,2 @@
>  QA output created by 402
> -y2038 inode timestamp tests completed successfully
> +inode timestamp tests completed status 0
> -- 
> 2.17.1
> 

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Deepa Dinamani]

On Sat, Oct 5, 2019 at 11:35 AM Eryu Guan <guaneryu@gmail.com> wrote:
> We already print error message on test failure, which will break the
> golden image, so there's no need to return 0 or 1. All similar checks
> and returns are not needed in other functions.

I removed the status checks and associated returns.

The patch is posted at
https://lore.kernel.org/fstests/20191023220401.12335-1-deepa.kernel@gmail.com/

-Deepa

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Amir Goldstein]

On Fri, Jul 19, 2019 at 7:21 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> The mount behavior will not be altered because of the unsupported
> timestamps on the filesystems.
>
> Adjust the test accordingly.
>
> An updated series to be posted after the merge window is hosted at
> <https://github.com/deepa-hub/vfs/tree/limits>
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
>  common/rc             | 36 +++++++++---------
>  tests/generic/402     | 87 ++++++++++++++++---------------------------
>  tests/generic/402.out |  2 +-
>  3 files changed, 53 insertions(+), 72 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 25203bb4..39a2deb0 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1959,16 +1959,9 @@ _run_aiodio()
>      return $status
>  }
>
> -# this test requires y2038 sysfs switch and filesystem
> -# timestamp ranges support.
> -_require_y2038()
> +_require_timestamp_range()
>  {
>         local device=${1:-$TEST_DEV}
> -       local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
> -
> -       if [ ! -e $sysfsdir ]; then
> -               _notrun "no kernel support for y2038 sysfs switch"
> -       fi
>

Deepa,

This change, which is already merged removed the test for kernel support
and replaced it with test only for filesystem support.

This impacts people validating stable kernel releases with xfstest, because
this test now always fails on stable kernels and I don't think that timestamp
clamping behavior is going to stable kernels.

Of course stable kernel testers can exclude this test, but this will remove test
coverage and may result in silent breakage of > y2038 timetamps in stable
kernels.

I think test should identify if kernel had clamping behavior and change the
expected timestamp values accordingly, similar to how the test was before
adjusting it to new behavior.

Do you agree? Can you make these changes?

Thanks,
Amir.

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Deepa Dinamani]

> >  {
> >         local device=${1:-$TEST_DEV}
> > -       local sysfsdir=/proc/sys/fs/fs-timestamp-check-on
> > -
> > -       if [ ! -e $sysfsdir ]; then
> > -               _notrun "no kernel support for y2038 sysfs switch"
> > -       fi
> >
>
> Deepa,
>
> This change, which is already merged removed the test for kernel support
> and replaced it with test only for filesystem support.
>
> This impacts people validating stable kernel releases with xfstest, because
> this test now always fails on stable kernels and I don't think that timestamp
> clamping behavior is going to stable kernels.
>
> Of course stable kernel testers can exclude this test, but this will remove test
> coverage and may result in silent breakage of > y2038 timetamps in stable
> kernels.
>
> I think test should identify if kernel had clamping behavior and change the
> expected timestamp values accordingly, similar to how the test was before
> adjusting it to new behavior.
>
> Do you agree? Can you make these changes?

Initially, we were going to allow rw mounts for filesystems which
could not update timestamps. And, this was a big change in behavior.
That is why we had the behavior exposed from the kernel.
I wasn't aware that the failing fstests would cause such a nuisence.
Since everybody seems to just rely on all the fstests passing, yes I
agree that bringing this back would be the easiest to not fail on
stable kernels.
I will post the kernel and the xfstest patch.

Thanks for helping me catch it.

-Deepa

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Deepa Dinamani]

I looked at this more closely. Here is the patch that added the sysctl
to the kernel previously: https://lkml.org/lkml/2016/11/2/300.

This was meant to be configurable earlier. That is why this made
sense. But, now it is not. We unconditionally clamp to the fs limits.
I looked around to see if we ever expose information about internal
kernel changes to userspace. This is almost never done. And, this is
always in the form of maybe a syscall failing. Given that we don't see
any modified behaviour that the user can point out, I don't think we
can expose the presence of clamping in the kernel.

fsinfo though exposes a fs max and min that could be useful if we fill
in an unknown pattern as default:
https://lwn.net/ml/linux-api/153314004147.18964.11925284995448007945.stgit@warthog.procyon.org.uk/.

I also spoke about this to Arnd, and he also suggested the fsinfo as
an alternative.

Is it easy to not run the test on older kernels? Otherwise, we just
have to rely on fsinfo being merged?

-Deepa

Re: [PATCH] generic/402: fix for updated behavior of timestamp limits

[Amir Goldstein]

On Wed, Dec 18, 2019 at 10:21 PM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> I looked at this more closely. Here is the patch that added the sysctl
> to the kernel previously: https://lkml.org/lkml/2016/11/2/300.
>
> This was meant to be configurable earlier. That is why this made
> sense. But, now it is not. We unconditionally clamp to the fs limits.
> I looked around to see if we ever expose information about internal
> kernel changes to userspace. This is almost never done. And, this is
> always in the form of maybe a syscall failing. Given that we don't see
> any modified behaviour that the user can point out, I don't think we
> can expose the presence of clamping in the kernel.
>
> fsinfo though exposes a fs max and min that could be useful if we fill
> in an unknown pattern as default:
> https://lwn.net/ml/linux-api/153314004147.18964.11925284995448007945.stgit@warthog.procyon.org.uk/.
>
> I also spoke about this to Arnd, and he also suggested the fsinfo as
> an alternative.
>
> Is it easy to not run the test on older kernels? Otherwise, we just
> have to rely on fsinfo being merged?
>

Is it easy to blacklist the test if that is what you are asking.
How people run their stable kernel tests I don't know.
I believe Sasha is running xfstests to validate stable release
candidate patches.

I don't think there is a clear policy about being friendly to testing
less that master kernels in xfstest (Eryu?), but IMO we should try to
accommodate
this use case, because it is in the best interest of everyone that stable kernel
will be regularly tested with xfstests with as little noisy failures
as possible.

Thanks,
Amir.

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Arnd Bergmann]

On Wed, Dec 18, 2019 at 9:46 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> I don't think there is a clear policy about being friendly to testing
> less that master kernels in xfstest (Eryu?), but IMO we should try to
> accommodate
> this use case, because it is in the best interest of everyone that stable kernel
> will be regularly tested with xfstests with as little noisy failures
> as possible.

I think what makes this one particularly hard is that there are most likely
people that do care about the failure on older kernels being reported and
would rather backport the kernel changes into their product kernels
to have them behave sanely.

I'm also not sure if we could just backport the changes to stable
kernels after all.

Greg, do you have an opinion on whether the 19 patches from
v5.3-rc6 to cba465b4f982 can be considered stable material?

The best argument that I have seen in favor of treating it as a bugfix
is that the posx man pages require that "The file's relevant timestamp shall
be set to the greatest value supported by the file system that is not greater
than the specified time"[1], and this is something that Linux has always
done wrong before the series (we overflow and underflow out-of-range
arguments to a value that is both file system and CPU architecture
specific).

The main argument against backporting would be that 19 patches
is too much, I think each patch in the series would qualify on its own.
Changing the layout of 'struct super_block' also breaks the module
binary interface, which will annoy some distros that care about this,
but I don't think it's stopping us from adding the patch to a stable
kernel.

       Arnd

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Greg KH]

On Thu, Dec 19, 2019 at 09:28:23AM +0100, Arnd Bergmann wrote:
> On Wed, Dec 18, 2019 at 9:46 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > I don't think there is a clear policy about being friendly to testing
> > less that master kernels in xfstest (Eryu?), but IMO we should try to
> > accommodate
> > this use case, because it is in the best interest of everyone that stable kernel
> > will be regularly tested with xfstests with as little noisy failures
> > as possible.
> 
> I think what makes this one particularly hard is that there are most likely
> people that do care about the failure on older kernels being reported and
> would rather backport the kernel changes into their product kernels
> to have them behave sanely.
> 
> I'm also not sure if we could just backport the changes to stable
> kernels after all.
> 
> Greg, do you have an opinion on whether the 19 patches from
> v5.3-rc6 to cba465b4f982 can be considered stable material?
> 
> The best argument that I have seen in favor of treating it as a bugfix
> is that the posx man pages require that "The file's relevant timestamp shall
> be set to the greatest value supported by the file system that is not greater
> than the specified time"[1], and this is something that Linux has always
> done wrong before the series (we overflow and underflow out-of-range
> arguments to a value that is both file system and CPU architecture
> specific).
> 
> The main argument against backporting would be that 19 patches
> is too much, I think each patch in the series would qualify on its own.
> Changing the layout of 'struct super_block' also breaks the module
> binary interface, which will annoy some distros that care about this,
> but I don't think it's stopping us from adding the patch to a stable
> kernel.
> 
>        Arnd
> 
> [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html

Ugh, that's a mess.  Why not just use 5.4 if people really care about
this type of thing?

But yes, on their own, each individual patch would be fine for stable,
it's just that I would want someone to "own" the backport and testing of
such a thing.  If for no other reason than to have someone to "blame"
for when things go wrong and get them to fix up the fallout :)

Who really really wants this in their older kernels?  And are those same
people already taking all of the stable updates for those kernels as
well?

thanks,

greg k-h

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Arnd Bergmann]

On Thu, Dec 19, 2019 at 9:40 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Thu, Dec 19, 2019 at 09:28:23AM +0100, Arnd Bergmann wrote:
> > On Wed, Dec 18, 2019 at 9:46 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
>
> Ugh, that's a mess.  Why not just use 5.4 if people really care about
> this type of thing?
>
> But yes, on their own, each individual patch would be fine for stable,
> it's just that I would want someone to "own" the backport and testing of
> such a thing.  If for no other reason than to have someone to "blame"
> for when things go wrong and get them to fix up the fallout :)

I was going to volunteer Deepa and me, but I just tried out what a backport
would look like, and backporting to v4.14 or earlier would involve a
major rewrite unless we also backport Deepa's earlier y2038 patches that
are much more invasive. Backporting to v4.19 (across the mount API
change) would be possible, but this doesn't really help the cause of
getting xfstests to report correct behavior on all stable kernels.

> Who really really wants this in their older kernels?  And are those same
> people already taking all of the stable updates for those kernels as
> well?

I see two potential groups of people:

- the one that started this thread: xfstests correctly reports a failure on
  stable kernels that have a known problem with compliance. If you are
  aiming for 100% pass rate on a test suite, you can either mark a correct
  test case as "skip", or backport the fix. Neither one is super attractive
  here, but it seemed worth considering which one is more harmful. (I
  guess I answered that now -- backporting to v4.14 would be more
  harmful)

- Users of CIP SLTS kernels with extreme service life that may involve
  not upgrading until after y2038 (this is obviously not recommended if
  you connect to a public network, but I'm sure some people do this anyway).
  For running user space, this requires either a 32-bit kernel with the
  linux-5.1 syscall changes or a 64-bit kernel. If you run a 64-bit linux-4.9
  kernel in a deeply embedded non-networked machine, it still makes
  sense to have working inode timestamps and be able to test that.

It may still make sense to backport this to linux-4.19.y-cip or another
downstream version of 4.19, but let's not do it for the normal LTS
kernels then.

       Arnd

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Greg KH]

On Thu, Dec 19, 2019 at 12:29:39PM +0100, Arnd Bergmann wrote:
> On Thu, Dec 19, 2019 at 9:40 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Thu, Dec 19, 2019 at 09:28:23AM +0100, Arnd Bergmann wrote:
> > > On Wed, Dec 18, 2019 at 9:46 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > [1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/futimens.html
> >
> > Ugh, that's a mess.  Why not just use 5.4 if people really care about
> > this type of thing?
> >
> > But yes, on their own, each individual patch would be fine for stable,
> > it's just that I would want someone to "own" the backport and testing of
> > such a thing.  If for no other reason than to have someone to "blame"
> > for when things go wrong and get them to fix up the fallout :)
> 
> I was going to volunteer Deepa and me, but I just tried out what a backport
> would look like, and backporting to v4.14 or earlier would involve a
> major rewrite unless we also backport Deepa's earlier y2038 patches that
> are much more invasive. Backporting to v4.19 (across the mount API
> change) would be possible, but this doesn't really help the cause of
> getting xfstests to report correct behavior on all stable kernels.
> 
> > Who really really wants this in their older kernels?  And are those same
> > people already taking all of the stable updates for those kernels as
> > well?
> 
> I see two potential groups of people:
> 
> - the one that started this thread: xfstests correctly reports a failure on
>   stable kernels that have a known problem with compliance. If you are
>   aiming for 100% pass rate on a test suite, you can either mark a correct
>   test case as "skip", or backport the fix. Neither one is super attractive
>   here, but it seemed worth considering which one is more harmful. (I
>   guess I answered that now -- backporting to v4.14 would be more
>   harmful)

Marking the test as "skip" for older kernels should be just fine for
this.

> - Users of CIP SLTS kernels with extreme service life that may involve
>   not upgrading until after y2038 (this is obviously not recommended if
>   you connect to a public network, but I'm sure some people do this anyway).
>   For running user space, this requires either a 32-bit kernel with the
>   linux-5.1 syscall changes or a 64-bit kernel. If you run a 64-bit linux-4.9
>   kernel in a deeply embedded non-networked machine, it still makes
>   sense to have working inode timestamps and be able to test that.
> 
> It may still make sense to backport this to linux-4.19.y-cip or another
> downstream version of 4.19, but let's not do it for the normal LTS
> kernels then.

CIP is in for a world of hurt for a lot of other reasons, all
self-inflicted :)

I'll let those people deal with the fallout of their odd decisions, but
I will quote a company that is supporting Linux devices in the wild for
20+ years:
	We update the kernel and all software on them for at least 18
	years as these devices are "alive", why would we declare them
	"dead" right away and only provide triage?  That's an insane way
	to support a product.

So let's just leave this as-is, 5.4 should be fine for people worrying
about y2038.

thanks,

greg k-h

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Amir Goldstein]

On Thu, Dec 19, 2019 at 10:28 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Dec 18, 2019 at 9:46 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > I don't think there is a clear policy about being friendly to testing
> > less that master kernels in xfstest (Eryu?), but IMO we should try to
> > accommodate
> > this use case, because it is in the best interest of everyone that stable kernel
> > will be regularly tested with xfstests with as little noisy failures
> > as possible.
>
> I think what makes this one particularly hard is that there are most likely
> people that do care about the failure on older kernels being reported and
> would rather backport the kernel changes into their product kernels
> to have them behave sanely.

Getting back to the thread before it diverged into the backport option.

The test used to detect kernel support and be skipped automatically on
old kernel
and now the test fails because the kernel knob and test for it were removed.

I perceive that as a regression to the test.
I don't mind waiting for fsinfo() to fix this regression, as long as
fsinfo() is going to
be backported to stable kernel 5.4???

Deepa,

You added this warning:
pr_warn("Mounted %s file system at %s supports timestamps until ...
along with timestamp clamping

I suggest that you implement kernel support check based on grepping
for this warning after loop mounting an ext2 test image.
A bit over the top and ugly, but the test should be reliable and
mkfs.ext2 is probably available in all xfstest deployments.

xfs/049 makes use of an ext2 loop mount, so your test won't be the
first one to use ext2 for testing other fs.

When kernel gets fsinfo() the test for kernel support can be improved
to using fsinfo() before doing the ext2 loop mount hack.

Thanks,
Amir.

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Ben Hutchings]

[+cc cip-dev]

On Thu, 2019-12-19 at 12:29 +0100, Arnd Bergmann wrote:
[...]
> - Users of CIP SLTS kernels with extreme service life that may involve
>   not upgrading until after y2038 (this is obviously not recommended if
>   you connect to a public network, but I'm sure some people do this anyway).
>   For running user space, this requires either a 32-bit kernel with the
>   linux-5.1 syscall changes or a 64-bit kernel. If you run a 64-bit linux-4.9
>   kernel in a deeply embedded non-networked machine, it still makes
>   sense to have working inode timestamps and be able to test that.
[...]

CIP is currently aiming for a 10 year support lifetime, so both of its
currently existing branches (4.4, 4.19) should be long out of support
in 2038.  Still, it's possible that some people hope to extend that
later.

Ben.

-- 
Ben Hutchings, Software Developer                         Codethink Ltd
https://www.codethink.co.uk/                 Dale House, 35 Dale Street
                                     Manchester, M1 2HF, United Kingdom

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Arnd Bergmann]

On Thu, Dec 19, 2019 at 4:48 PM Ben Hutchings
<ben.hutchings@codethink.co.uk> wrote:
> On Thu, 2019-12-19 at 12:29 +0100, Arnd Bergmann wrote:
> [...]
> > - Users of CIP SLTS kernels with extreme service life that may involve
> >   not upgrading until after y2038 (this is obviously not recommended if
> >   you connect to a public network, but I'm sure some people do this anyway).
> >   For running user space, this requires either a 32-bit kernel with the
> >   linux-5.1 syscall changes or a 64-bit kernel. If you run a 64-bit linux-4.9
> >   kernel in a deeply embedded non-networked machine, it still makes
> >   sense to have working inode timestamps and be able to test that.
> [...]
>
> CIP is currently aiming for a 10 year support lifetime, so both of its
> currently existing branches (4.4, 4.19) should be long out of support
> in 2038.  Still, it's possible that some people hope to extend that
> later.

It is also common that end-users keep relying on machines that they
bought for many years after any support contracts end. This may be
fine for a lot of use cases where there is no risk in running an old
operating system, and replacing it is prohibitively expensive, as
software generally does not suddenly stop working after support ends.

With the y2038-safe interfaces and with file system limits, the
difference is that there is a specific point in time when things will
break.

        Arnd

Re: [Y2038] [PATCH] generic/402: fix for updated behavior of timestamp limits

[Deepa Dinamani]

> Deepa,
>
> You added this warning:
> pr_warn("Mounted %s file system at %s supports timestamps until ...
> along with timestamp clamping
>
> I suggest that you implement kernel support check based on grepping
> for this warning after loop mounting an ext2 test image.
> A bit over the top and ugly, but the test should be reliable and
> mkfs.ext2 is probably available in all xfstest deployments.
>
> xfs/049 makes use of an ext2 loop mount, so your test won't be the
> first one to use ext2 for testing other fs.

Ok, this can work. Let me give this a try.

Thanks,
Deepa

[PATCH] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

Addition of fs-specific timestamp range checking was added
in 188d20bcd1eb ("vfs: Add file timestamp range support").

Add a check for whether the kernel supports the limits check
before running the associated test.

Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 common/rc         | 11 +++++++++++
 tests/generic/402 |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/common/rc b/common/rc
index 816588d6..472db995 100644
--- a/common/rc
+++ b/common/rc
@@ -1981,6 +1981,17 @@ _run_aiodio()
     return $status
 }
 
+_require_kernel_timestamp_range()
+{
+	# 128-byte inodes do not have room for extended timestamp
+	MKFS_OPTIONS=-I128 _scratch_mkfs_ext4 &>> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
+
+	mount -t ext4 ${SCRATCH_DEV} ${SCRATCH_MNT}
+	_check_dmesg_for "ext4 filesystem being mounted at ${SCRATCH_MNT} supports timestamps until 2038" || \
+		_notrun "Kernel does not support timestamp limits"
+	umount ${SCRATCH_MNT}
+}
+
 _require_timestamp_range()
 {
 	local device=${1:-$TEST_DEV}
diff --git a/tests/generic/402 b/tests/generic/402
index 0392c258..2be94c54 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -30,6 +30,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_check_dmesg
 _require_xfs_io_command utimes
 
 # Compare file timestamps obtained from stat
@@ -79,6 +80,8 @@ run_test()
 	done
 }
 
+_require_kernel_timestamp_range
+
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
 _require_timestamp_range $SCRATCH_DEV
 
-- 
2.17.1

Re: [PATCH] generic/402: Make timestamp range check conditional

[Amir Goldstein]

On Mon, Dec 23, 2019 at 7:16 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> Addition of fs-specific timestamp range checking was added
> in 188d20bcd1eb ("vfs: Add file timestamp range support").
>
> Add a check for whether the kernel supports the limits check
> before running the associated test.
>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
>  common/rc         | 11 +++++++++++
>  tests/generic/402 |  3 +++
>  2 files changed, 14 insertions(+)
>
> diff --git a/common/rc b/common/rc
> index 816588d6..472db995 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1981,6 +1981,17 @@ _run_aiodio()
>      return $status
>  }
>
> +_require_kernel_timestamp_range()
> +{
> +       # 128-byte inodes do not have room for extended timestamp
> +       MKFS_OPTIONS=-I128 _scratch_mkfs_ext4 &>> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> +
> +       mount -t ext4 ${SCRATCH_DEV} ${SCRATCH_MNT}
> +       _check_dmesg_for "ext4 filesystem being mounted at ${SCRATCH_MNT} supports timestamps until 2038" || \
> +               _notrun "Kernel does not support timestamp limits"
> +       umount ${SCRATCH_MNT}
> +}
> +

Deepa,

Thank you for following up.
I am not sure if mkfs.ext4 of scratch partition in a generic test is going to be
very popular - let's see what others have to say.
You can certainly now do that without checking that  ${SCRATCH_DEV} is
a blockdev which is not the case for overlay and networking filesystems.

Why did you choose not to use a loop mounted ext2 for the check as
I suggested?
You can use _require_loop() and _require_ext2() inside the check.
In any case, please also check for failure to mount.

Thanks,
Amir.

Re: [PATCH] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

On Sun, Dec 22, 2019 at 10:36 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Dec 23, 2019 at 7:16 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > Addition of fs-specific timestamp range checking was added
> > in 188d20bcd1eb ("vfs: Add file timestamp range support").
> >
> > Add a check for whether the kernel supports the limits check
> > before running the associated test.
> >
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > ---
> >  common/rc         | 11 +++++++++++
> >  tests/generic/402 |  3 +++
> >  2 files changed, 14 insertions(+)
> >
> > diff --git a/common/rc b/common/rc
> > index 816588d6..472db995 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1981,6 +1981,17 @@ _run_aiodio()
> >      return $status
> >  }
> >
> > +_require_kernel_timestamp_range()
> > +{
> > +       # 128-byte inodes do not have room for extended timestamp
> > +       MKFS_OPTIONS=-I128 _scratch_mkfs_ext4 &>> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> > +
> > +       mount -t ext4 ${SCRATCH_DEV} ${SCRATCH_MNT}
> > +       _check_dmesg_for "ext4 filesystem being mounted at ${SCRATCH_MNT} supports timestamps until 2038" || \
> > +               _notrun "Kernel does not support timestamp limits"
> > +       umount ${SCRATCH_MNT}
> > +}
> > +
>
> Deepa,
>
> Thank you for following up.
> I am not sure if mkfs.ext4 of scratch partition in a generic test is going to be
> very popular - let's see what others have to say.
> You can certainly now do that without checking that  ${SCRATCH_DEV} is
> a blockdev which is not the case for overlay and networking filesystems.

 _require_block_device() should alleviate this concern. But, I think
making it a loop back device is a good idea.

I meant to check when mkfs.ext4 would fail, but I forgot. I will
change this in v2.

> Why did you choose not to use a loop mounted ext2 for the check as
> I suggested?
> You can use _require_loop() and _require_ext2() inside the check.
> In any case, please also check for failure to mount.

I did not not see anybody creating ext2 filesystem, and I thought that
finding a kernel supporting ext4 was a lot more likely. We can add a
_require_ext4() along the lines of _ext2_reqire() if really necessary.
We should also add "_require_command "$MKFS_EXT4_PROG" mkfs.ext4".

I think it does not matter which filesystem we use unless we get the
warning. Let me know if I missed something.

I will pick one of the two: ext4 (with small inode) or ext2 and do a
loop back device instead for v2. I will also add the suggested-by that
I forgot on this patch.

-Deepa

[PATCH v2] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

Addition of fs-specific timestamp range checking was added
in 188d20bcd1eb ("vfs: Add file timestamp range support").

Add a check for whether the kernel supports the limits check
before running the associated test.

ext4 has been chosen to test for the presence of kernel support
for this feature. If there is a concern that ext4 could be built
out of the kernel, I can include a _require_ext4() along the
lines of _require_ext2().

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
* Changes since v1:
  used loopback device instead of mkfs scratch dev

 common/rc         | 26 ++++++++++++++++++++++++++
 tests/generic/402 |  3 +++
 2 files changed, 29 insertions(+)

diff --git a/common/rc b/common/rc
index 816588d6..6248adf7 100644
--- a/common/rc
+++ b/common/rc
@@ -1981,6 +1981,32 @@ _run_aiodio()
     return $status
 }
 
+_require_kernel_timestamp_range()
+{
+	LOOP_FILE=$SCRATCH_MNT/loop_file
+	LOOP_MNT=$SCRATCH_MNT/loop_mnt
+
+	dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd || _fail "loopback prep failed"
+
+	# Use ext4 with 128-byte inodes, which do not have room for extended timestamp
+	FSTYP=ext4 MKFS_OPTIONS=-I128 \
+	_mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
+
+	LOOP_DEV=$(_create_loop_device $LOOP_FILE)
+	mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to create $LOOP_MNT"
+	mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "ext4 mount failed"
+	notrun=false
+	_check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT} supports timestamps until 2038" || \
+		notrun=true
+
+	umount ${LOOP_MNT} >> $seqres.full 2>&1 ||_fail "failed to umount $LOOP_MNT"
+	_destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
+
+	if $notrun; then
+		_notrun "Kernel does not support timestamp limits"
+	fi
+}
+
 _require_timestamp_range()
 {
 	local device=${1:-$TEST_DEV}
diff --git a/tests/generic/402 b/tests/generic/402
index 0392c258..2be94c54 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -30,6 +30,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_check_dmesg
 _require_xfs_io_command utimes
 
 # Compare file timestamps obtained from stat
@@ -79,6 +80,8 @@ run_test()
 	done
 }
 
+_require_kernel_timestamp_range
+
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
 _require_timestamp_range $SCRATCH_DEV
 
-- 
2.17.1

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Amir Goldstein]

On Sun, Dec 29, 2019 at 12:13 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> Addition of fs-specific timestamp range checking was added
> in 188d20bcd1eb ("vfs: Add file timestamp range support").
>
> Add a check for whether the kernel supports the limits check
> before running the associated test.
>
> ext4 has been chosen to test for the presence of kernel support
> for this feature. If there is a concern that ext4 could be built
> out of the kernel, I can include a _require_ext4() along the
> lines of _require_ext2().
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
> * Changes since v1:
>   used loopback device instead of mkfs scratch dev
>
>  common/rc         | 26 ++++++++++++++++++++++++++
>  tests/generic/402 |  3 +++
>  2 files changed, 29 insertions(+)
>
> diff --git a/common/rc b/common/rc
> index 816588d6..6248adf7 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1981,6 +1981,32 @@ _run_aiodio()
>      return $status
>  }
>
> +_require_kernel_timestamp_range()
> +{
> +       LOOP_FILE=$SCRATCH_MNT/loop_file
> +       LOOP_MNT=$SCRATCH_MNT/loop_mnt
> +
> +       dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd || _fail "loopback prep failed"
> +
> +       # Use ext4 with 128-byte inodes, which do not have room for extended timestamp
> +       FSTYP=ext4 MKFS_OPTIONS=-I128 \
> +       _mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> +
> +       LOOP_DEV=$(_create_loop_device $LOOP_FILE)
> +       mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to create $LOOP_MNT"
> +       mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "ext4 mount failed"
> +       notrun=false
> +       _check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT} supports timestamps until 2038" || \
> +               notrun=true
> +
> +       umount ${LOOP_MNT} >> $seqres.full 2>&1 ||_fail "failed to umount $LOOP_MNT"
> +       _destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
> +
> +       if $notrun; then
> +               _notrun "Kernel does not support timestamp limits"
> +       fi
> +}
> +

As a generic helper, this function has a few problems:
1. It assumes scratch dev is mounted (and you're not even calling it
after _scratch_mount)
2. The cleanup() hook won't clean loop mnt/dev if interrupted
3. test doesn't have _require_loop (nor require ext4 as you mentioned)

All this leads me to think that perhaps it would be better off, at least until
kernel has fsinfo, to keep this entire helper inside generic/402,
while addressing
the issues above in the test itself.

A more generic solution would be harder and IMO and overkill at this point.

What do you think?

Thanks,
Amir.

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

> As a generic helper, this function has a few problems:
> 1. It assumes scratch dev is mounted (and you're not even calling it
> after _scratch_mount)

Ok, this was an oversight. I was using rootfs as scratch and didn't
catch this in my tests.

> 2. The cleanup() hook won't clean loop mnt/dev if interrupted

Agreed. I can add these in.

There are some existing functions in common/rc that do not clean up.
For example _require_scratch_swapfile might leave partial state if interrupted.

> 3. test doesn't have _require_loop (nor require ext4 as you mentioned)

Some generic functions assume many preconditions.
But, if it is preferred to be more self contained, I can model this
after something like _require_scratch_swapfile()

> All this leads me to think that perhaps it would be better off, at least until
> kernel has fsinfo, to keep this entire helper inside generic/402,
> while addressing
> the issues above in the test itself.

> A more generic solution would be harder and IMO and overkill at this point.

With fsinfo as proposed, it would not be possible to tell if fs ranges
are supported without doing the same kind of workaround.
A capability bit could be added to advertise that feature of VFS, or
it might be reasonable to assume it from the mere existence of fsinfo
syscall.

> What do you think?

The following proposed patch uses a local _cleanup handler that can handle this.
I am okay with either approach. I'm not sure which one is preferable
to xfstests maintainers.
Let me know and I can post it as a V3.

-Deepa

From f539005511f3ad83563cabc6d185b2c76ae37dea Mon Sep 17 00:00:00 2001
From: Deepa Dinamani <deepa.kernel@gmail.com>
Date: Sun, 22 Dec 2019 19:18:14 -0800
Subject: [PATCH v3 1/1] generic/402: Make timestamp range check conditional

Addition of fs-specific timestamp range checking was added
in 188d20bcd1eb ("vfs: Add file timestamp range support").

Add a check for whether the kernel supports the limits check
before running the associated test.

ext4 has been chosen to test for the presence of kernel support
for this feature.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 common/rc         | 50 +++++++++++++++++++++++++++++++++++++++++++----
 tests/generic/402 | 12 +++++++++++-
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/common/rc b/common/rc
index eeac1355..796052e6 100644
--- a/common/rc
+++ b/common/rc
@@ -1751,17 +1751,18 @@ _require_loop()
 #
 _require_ext2()
 {
+    fs=${1:-ext2}
     if [ "$HOSTOS" != "Linux" ]
     then
-    _notrun "This test requires linux for ext2 filesystem support"
+    _notrun "This test requires linux for $fs filesystem support"
     fi

-    modprobe ext2 >/dev/null 2>&1
-    if grep ext2 /proc/filesystems >/dev/null 2>&1
+    modprobe $fs >/dev/null 2>&1
+    if grep $fs /proc/filesystems >/dev/null 2>&1
     then
     :
     else
-    _notrun "This test requires ext2 filesystem support"
+    _notrun "This test requires $fs filesystem support"
     fi
 }

@@ -1981,6 +1982,47 @@ _run_aiodio()
     return $status
 }

+_require_kernel_timestamp_range()
+{
+
+    _require_scratch
+    _require_loop
+    _require_ext2 ext4
+
+    # Use a subshell to clean up the nested loop mount
+    _cleanup='( umount $LOOP_MNT ; _destroy_loop_device $LOOP_DEV ;
rm -f $LOOP_FILE ; _scratch_unmount )'
+
+    _scratch_mkfs >/dev/null
+    _scratch_mount
+
+    LOOP_FILE=$SCRATCH_MNT/loop_file
+    LOOP_MNT=$SCRATCH_MNT/loop_mnt
+
+    dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd ||
_fail "loopback prep failed"
+
+    # Use ext4 with 128-byte inodes, which do not have room for
extended timestamp
+    FSTYP=ext4 MKFS_OPTIONS=-I128 \
+    _mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
+
+    LOOP_DEV=$(_create_loop_device $LOOP_FILE)
+    mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to
create $LOOP_MNT"
+    mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 ||
_fail "ext4 mount failed"
+    notrun=false
+    _check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT}
supports timestamps until 2038" || \
+        notrun=true
+
+    umount ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "failed to
umount $LOOP_MNT"
+    _destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
+
+    _scratch_unmount
+
+    _cleanup=:
+
+    if $notrun; then
+        _notrun "Kernel does not support timestamp limits"
+    fi
+}
+
 _require_timestamp_range()
 {
     local device=${1:-$TEST_DEV}
diff --git a/tests/generic/402 b/tests/generic/402
index 0392c258..4288168a 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -16,7 +16,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1    # failure is the default!
-trap "exit \$status" 0 1 2 3 15
+_cleanup=:
+trap "eval \$_cleanup; _cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}

 # Get standard environment, filters and checks.
 . ./common/rc
@@ -30,6 +37,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_check_dmesg
 _require_xfs_io_command utimes

 # Compare file timestamps obtained from stat
@@ -79,6 +87,8 @@ run_test()
     done
 }

+_require_kernel_timestamp_range
+
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
 _require_timestamp_range $SCRATCH_DEV

-- 
2.17.1

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Amir Goldstein]

On Fri, Jan 3, 2020 at 8:46 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> > As a generic helper, this function has a few problems:
> > 1. It assumes scratch dev is mounted (and you're not even calling it
> > after _scratch_mount)
>
> Ok, this was an oversight. I was using rootfs as scratch and didn't
> catch this in my tests.
>
> > 2. The cleanup() hook won't clean loop mnt/dev if interrupted
>
> Agreed. I can add these in.
>
> There are some existing functions in common/rc that do not clean up.
> For example _require_scratch_swapfile might leave partial state if interrupted.

Right. Things are not perfect.
We should try to not make them worse ;-)

>
> > 3. test doesn't have _require_loop (nor require ext4 as you mentioned)
>
> Some generic functions assume many preconditions.
> But, if it is preferred to be more self contained, I can model this
> after something like _require_scratch_swapfile()

OK.

>
> > All this leads me to think that perhaps it would be better off, at least until
> > kernel has fsinfo, to keep this entire helper inside generic/402,
> > while addressing
> > the issues above in the test itself.
>
> > A more generic solution would be harder and IMO and overkill at this point.
>
> With fsinfo as proposed, it would not be possible to tell if fs ranges
> are supported without doing the same kind of workaround.
> A capability bit could be added to advertise that feature of VFS, or
> it might be reasonable to assume it from the mere existence of fsinfo
> syscall.

That is what I had in mind.

>
> > What do you think?
>
> The following proposed patch uses a local _cleanup handler that can handle this.
> I am okay with either approach. I'm not sure which one is preferable
> to xfstests maintainers.
> Let me know and I can post it as a V3.
>

IMO the "nested" _cleanup handler is not a "clean" solution,
but let's let Eryu decide how to tackle this.

Thanks,
Amir.

>
> From f539005511f3ad83563cabc6d185b2c76ae37dea Mon Sep 17 00:00:00 2001
> From: Deepa Dinamani <deepa.kernel@gmail.com>
> Date: Sun, 22 Dec 2019 19:18:14 -0800
> Subject: [PATCH v3 1/1] generic/402: Make timestamp range check conditional
>
> Addition of fs-specific timestamp range checking was added
> in 188d20bcd1eb ("vfs: Add file timestamp range support").
>
> Add a check for whether the kernel supports the limits check
> before running the associated test.
>
> ext4 has been chosen to test for the presence of kernel support
> for this feature.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---
>  common/rc         | 50 +++++++++++++++++++++++++++++++++++++++++++----
>  tests/generic/402 | 12 +++++++++++-
>  2 files changed, 57 insertions(+), 5 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index eeac1355..796052e6 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1751,17 +1751,18 @@ _require_loop()
>  #
>  _require_ext2()
>  {
> +    fs=${1:-ext2}
>      if [ "$HOSTOS" != "Linux" ]
>      then
> -    _notrun "This test requires linux for ext2 filesystem support"
> +    _notrun "This test requires linux for $fs filesystem support"
>      fi
>
> -    modprobe ext2 >/dev/null 2>&1
> -    if grep ext2 /proc/filesystems >/dev/null 2>&1
> +    modprobe $fs >/dev/null 2>&1
> +    if grep $fs /proc/filesystems >/dev/null 2>&1
>      then
>      :
>      else
> -    _notrun "This test requires ext2 filesystem support"
> +    _notrun "This test requires $fs filesystem support"
>      fi
>  }
>
> @@ -1981,6 +1982,47 @@ _run_aiodio()
>      return $status
>  }
>
> +_require_kernel_timestamp_range()
> +{
> +
> +    _require_scratch
> +    _require_loop
> +    _require_ext2 ext4
> +
> +    # Use a subshell to clean up the nested loop mount
> +    _cleanup='( umount $LOOP_MNT ; _destroy_loop_device $LOOP_DEV ;
> rm -f $LOOP_FILE ; _scratch_unmount )'
> +
> +    _scratch_mkfs >/dev/null
> +    _scratch_mount
> +
> +    LOOP_FILE=$SCRATCH_MNT/loop_file
> +    LOOP_MNT=$SCRATCH_MNT/loop_mnt
> +
> +    dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd ||
> _fail "loopback prep failed"
> +
> +    # Use ext4 with 128-byte inodes, which do not have room for
> extended timestamp
> +    FSTYP=ext4 MKFS_OPTIONS=-I128 \
> +    _mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> +
> +    LOOP_DEV=$(_create_loop_device $LOOP_FILE)
> +    mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to
> create $LOOP_MNT"
> +    mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 ||
> _fail "ext4 mount failed"
> +    notrun=false
> +    _check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT}
> supports timestamps until 2038" || \
> +        notrun=true
> +
> +    umount ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "failed to
> umount $LOOP_MNT"
> +    _destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
> +
> +    _scratch_unmount
> +
> +    _cleanup=:
> +
> +    if $notrun; then
> +        _notrun "Kernel does not support timestamp limits"
> +    fi
> +}
> +
>  _require_timestamp_range()
>  {
>      local device=${1:-$TEST_DEV}
> diff --git a/tests/generic/402 b/tests/generic/402
> index 0392c258..4288168a 100755
> --- a/tests/generic/402
> +++ b/tests/generic/402
> @@ -16,7 +16,14 @@ echo "QA output created by $seq"
>  here=`pwd`
>  tmp=/tmp/$$
>  status=1    # failure is the default!
> -trap "exit \$status" 0 1 2 3 15
> +_cleanup=:
> +trap "eval \$_cleanup; _cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
>
>  # Get standard environment, filters and checks.
>  . ./common/rc
> @@ -30,6 +37,7 @@ rm -f $seqres.full
>  _supported_fs generic
>  _supported_os Linux
>  _require_scratch
> +_require_check_dmesg
>  _require_xfs_io_command utimes
>
>  # Compare file timestamps obtained from stat
> @@ -79,6 +87,8 @@ run_test()
>      done
>  }
>
> +_require_kernel_timestamp_range
> +
>  _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
>  _require_timestamp_range $SCRATCH_DEV
>
> --
> 2.17.1

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Eryu Guan]

On Mon, Dec 30, 2019 at 09:34:47AM +0200, Amir Goldstein wrote:
> On Sun, Dec 29, 2019 at 12:13 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > Addition of fs-specific timestamp range checking was added
> > in 188d20bcd1eb ("vfs: Add file timestamp range support").
> >
> > Add a check for whether the kernel supports the limits check
> > before running the associated test.
> >
> > ext4 has been chosen to test for the presence of kernel support
> > for this feature. If there is a concern that ext4 could be built
> > out of the kernel, I can include a _require_ext4() along the
> > lines of _require_ext2().
> >
> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>

Sorry for chiming in so late..

> > ---
> > * Changes since v1:
> >   used loopback device instead of mkfs scratch dev
> >
> >  common/rc         | 26 ++++++++++++++++++++++++++
> >  tests/generic/402 |  3 +++
> >  2 files changed, 29 insertions(+)
> >
> > diff --git a/common/rc b/common/rc
> > index 816588d6..6248adf7 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -1981,6 +1981,32 @@ _run_aiodio()
> >      return $status
> >  }
> >
> > +_require_kernel_timestamp_range()
> > +{
> > +       LOOP_FILE=$SCRATCH_MNT/loop_file
> > +       LOOP_MNT=$SCRATCH_MNT/loop_mnt
> > +
> > +       dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd || _fail "loopback prep failed"
> > +
> > +       # Use ext4 with 128-byte inodes, which do not have room for extended timestamp
> > +       FSTYP=ext4 MKFS_OPTIONS=-I128 \
> > +       _mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> > +
> > +       LOOP_DEV=$(_create_loop_device $LOOP_FILE)
> > +       mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to create $LOOP_MNT"
> > +       mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "ext4 mount failed"
> > +       notrun=false
> > +       _check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT} supports timestamps until 2038" || \
> > +               notrun=true
> > +
> > +       umount ${LOOP_MNT} >> $seqres.full 2>&1 ||_fail "failed to umount $LOOP_MNT"
> > +       _destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
> > +
> > +       if $notrun; then
> > +               _notrun "Kernel does not support timestamp limits"
> > +       fi
> > +}
> > +
> 
> As a generic helper, this function has a few problems:
> 1. It assumes scratch dev is mounted (and you're not even calling it
> after _scratch_mount)
> 2. The cleanup() hook won't clean loop mnt/dev if interrupted
> 3. test doesn't have _require_loop (nor require ext4 as you mentioned)
> 
> All this leads me to think that perhaps it would be better off, at least until
> kernel has fsinfo, to keep this entire helper inside generic/402,
> while addressing
> the issues above in the test itself.
> 
> A more generic solution would be harder and IMO and overkill at this point.
> 
> What do you think?

After reading through this thread, I prefer waiting for the comming
fsinfo interface, detecting the timestamp limit support using ext2 &
loop device doesn't look "pretty" and is just a temporary solution.

Thanks,
Eryu

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Amir Goldstein]

On Wed, Jan 8, 2020 at 10:09 AM Eryu Guan <guaneryu@gmail.com> wrote:
>
> On Mon, Dec 30, 2019 at 09:34:47AM +0200, Amir Goldstein wrote:
> > On Sun, Dec 29, 2019 at 12:13 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > >
> > > Addition of fs-specific timestamp range checking was added
> > > in 188d20bcd1eb ("vfs: Add file timestamp range support").
> > >
> > > Add a check for whether the kernel supports the limits check
> > > before running the associated test.
> > >
> > > ext4 has been chosen to test for the presence of kernel support
> > > for this feature. If there is a concern that ext4 could be built
> > > out of the kernel, I can include a _require_ext4() along the
> > > lines of _require_ext2().
> > >
> > > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
>
> Sorry for chiming in so late..
>
> > > ---
> > > * Changes since v1:
> > >   used loopback device instead of mkfs scratch dev
> > >
> > >  common/rc         | 26 ++++++++++++++++++++++++++
> > >  tests/generic/402 |  3 +++
> > >  2 files changed, 29 insertions(+)
> > >
> > > diff --git a/common/rc b/common/rc
> > > index 816588d6..6248adf7 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -1981,6 +1981,32 @@ _run_aiodio()
> > >      return $status
> > >  }
> > >
> > > +_require_kernel_timestamp_range()
> > > +{
> > > +       LOOP_FILE=$SCRATCH_MNT/loop_file
> > > +       LOOP_MNT=$SCRATCH_MNT/loop_mnt
> > > +
> > > +       dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd || _fail "loopback prep failed"
> > > +
> > > +       # Use ext4 with 128-byte inodes, which do not have room for extended timestamp
> > > +       FSTYP=ext4 MKFS_OPTIONS=-I128 \
> > > +       _mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> > > +
> > > +       LOOP_DEV=$(_create_loop_device $LOOP_FILE)
> > > +       mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to create $LOOP_MNT"
> > > +       mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "ext4 mount failed"
> > > +       notrun=false
> > > +       _check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT} supports timestamps until 2038" || \
> > > +               notrun=true
> > > +
> > > +       umount ${LOOP_MNT} >> $seqres.full 2>&1 ||_fail "failed to umount $LOOP_MNT"
> > > +       _destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
> > > +
> > > +       if $notrun; then
> > > +               _notrun "Kernel does not support timestamp limits"
> > > +       fi
> > > +}
> > > +
> >
> > As a generic helper, this function has a few problems:
> > 1. It assumes scratch dev is mounted (and you're not even calling it
> > after _scratch_mount)
> > 2. The cleanup() hook won't clean loop mnt/dev if interrupted
> > 3. test doesn't have _require_loop (nor require ext4 as you mentioned)
> >
> > All this leads me to think that perhaps it would be better off, at least until
> > kernel has fsinfo, to keep this entire helper inside generic/402,
> > while addressing
> > the issues above in the test itself.
> >
> > A more generic solution would be harder and IMO and overkill at this point.
> >
> > What do you think?
>
> After reading through this thread, I prefer waiting for the comming
> fsinfo interface, detecting the timestamp limit support using ext2 &
> loop device doesn't look "pretty" and is just a temporary solution.
>

I understand why you dislike the ext2+loop test, but please hear me out.

From all the fs types that are supported by the test, only btrfs and ext4 with
large inode size are y2038 ready.
For the rest of the cases, we actually have a way to detect kernel support
from the dmesg warning, without the need for hacky ext2 loop mount.

So how about just:
1. moving  _scratch_mount before _require_timestamp_range
2. in _require_timestamp_range (untested):
        if [ $tsmax -lt $((1<<32)) ] && ! _check_dmesg_for "supports
timestamps until 2038" ; then
                _notrun "Kernel does not support timestamp limits"
        fi

It's better than nothing and it does not add much complications, nor
is this "hacky"
IMO.

Thoughts?

Amir.

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Eryu Guan]

On Wed, Jan 08, 2020 at 10:45:29AM +0200, Amir Goldstein wrote:
> On Wed, Jan 8, 2020 at 10:09 AM Eryu Guan <guaneryu@gmail.com> wrote:
> >
> > On Mon, Dec 30, 2019 at 09:34:47AM +0200, Amir Goldstein wrote:
> > > On Sun, Dec 29, 2019 at 12:13 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> > > >
> > > > Addition of fs-specific timestamp range checking was added
> > > > in 188d20bcd1eb ("vfs: Add file timestamp range support").
> > > >
> > > > Add a check for whether the kernel supports the limits check
> > > > before running the associated test.
> > > >
> > > > ext4 has been chosen to test for the presence of kernel support
> > > > for this feature. If there is a concern that ext4 could be built
> > > > out of the kernel, I can include a _require_ext4() along the
> > > > lines of _require_ext2().
> > > >
> > > > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > > > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> >
> > Sorry for chiming in so late..
> >
> > > > ---
> > > > * Changes since v1:
> > > >   used loopback device instead of mkfs scratch dev
> > > >
> > > >  common/rc         | 26 ++++++++++++++++++++++++++
> > > >  tests/generic/402 |  3 +++
> > > >  2 files changed, 29 insertions(+)
> > > >
> > > > diff --git a/common/rc b/common/rc
> > > > index 816588d6..6248adf7 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -1981,6 +1981,32 @@ _run_aiodio()
> > > >      return $status
> > > >  }
> > > >
> > > > +_require_kernel_timestamp_range()
> > > > +{
> > > > +       LOOP_FILE=$SCRATCH_MNT/loop_file
> > > > +       LOOP_MNT=$SCRATCH_MNT/loop_mnt
> > > > +
> > > > +       dd if=/dev/zero of=$LOOP_FILE bs=1M count=2 2>&1 | _filter_dd || _fail "loopback prep failed"
> > > > +
> > > > +       # Use ext4 with 128-byte inodes, which do not have room for extended timestamp
> > > > +       FSTYP=ext4 MKFS_OPTIONS=-I128 \
> > > > +       _mkfs_dev $LOOP_FILE >> $seqres.full 2>&1 || _fail "ext4 mkfs failed"
> > > > +
> > > > +       LOOP_DEV=$(_create_loop_device $LOOP_FILE)
> > > > +       mkdir -p $LOOP_MNT >> $seqres.full 2>&1 || _fail "failed to create $LOOP_MNT"
> > > > +       mount -t ext4 ${LOOP_DEV} ${LOOP_MNT} >> $seqres.full 2>&1 || _fail "ext4 mount failed"
> > > > +       notrun=false
> > > > +       _check_dmesg_for "ext4 filesystem being mounted at ${LOOP_MNT} supports timestamps until 2038" || \
> > > > +               notrun=true
> > > > +
> > > > +       umount ${LOOP_MNT} >> $seqres.full 2>&1 ||_fail "failed to umount $LOOP_MNT"
> > > > +       _destroy_loop_device ${LOOP_DEV} >> $seqres.full 2>&1
> > > > +
> > > > +       if $notrun; then
> > > > +               _notrun "Kernel does not support timestamp limits"
> > > > +       fi
> > > > +}
> > > > +
> > >
> > > As a generic helper, this function has a few problems:
> > > 1. It assumes scratch dev is mounted (and you're not even calling it
> > > after _scratch_mount)
> > > 2. The cleanup() hook won't clean loop mnt/dev if interrupted
> > > 3. test doesn't have _require_loop (nor require ext4 as you mentioned)
> > >
> > > All this leads me to think that perhaps it would be better off, at least until
> > > kernel has fsinfo, to keep this entire helper inside generic/402,
> > > while addressing
> > > the issues above in the test itself.
> > >
> > > A more generic solution would be harder and IMO and overkill at this point.
> > >
> > > What do you think?
> >
> > After reading through this thread, I prefer waiting for the comming
> > fsinfo interface, detecting the timestamp limit support using ext2 &
> > loop device doesn't look "pretty" and is just a temporary solution.
> >
> 
> I understand why you dislike the ext2+loop test, but please hear me out.
> 
> From all the fs types that are supported by the test, only btrfs and ext4 with
> large inode size are y2038 ready.
> For the rest of the cases, we actually have a way to detect kernel support
> from the dmesg warning, without the need for hacky ext2 loop mount.
> 
> So how about just:
> 1. moving  _scratch_mount before _require_timestamp_range
> 2. in _require_timestamp_range (untested):
>         if [ $tsmax -lt $((1<<32)) ] && ! _check_dmesg_for "supports

Yeah, this looks fine. I thought about searching for dmesg warning after
_scratch_mount as well, but that would _notrun if the fs is 2038-safe.
This $tsmax check fixes my concern. Thanks!

Eryu

> timestamps until 2038" ; then
>                 _notrun "Kernel does not support timestamp limits"
>         fi
> 
> It's better than nothing and it does not add much complications, nor
> is this "hacky"
> IMO.
> 
> Thoughts?
> 
> Amir.

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Amir Goldstein]

> > I understand why you dislike the ext2+loop test, but please hear me out.
> >
> > From all the fs types that are supported by the test, only btrfs and ext4 with
> > large inode size are y2038 ready.
> > For the rest of the cases, we actually have a way to detect kernel support
> > from the dmesg warning, without the need for hacky ext2 loop mount.
> >
> > So how about just:
> > 1. moving  _scratch_mount before _require_timestamp_range
> > 2. in _require_timestamp_range (untested):
> >         if [ $tsmax -lt $((1<<32)) ] && ! _check_dmesg_for "supports
>
> Yeah, this looks fine. I thought about searching for dmesg warning after
> _scratch_mount as well, but that would _notrun if the fs is 2038-safe.
> This $tsmax check fixes my concern. Thanks!
>

Deepa,

Do you intend to post the simplified version or would you like me
to re-spin your patch?

Thanks,
Amir.

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

On Fri, Jan 17, 2020 at 1:09 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> > > I understand why you dislike the ext2+loop test, but please hear me out.
> > >
> > > From all the fs types that are supported by the test, only btrfs and ext4 with
> > > large inode size are y2038 ready.
> > > For the rest of the cases, we actually have a way to detect kernel support
> > > from the dmesg warning, without the need for hacky ext2 loop mount.
> > >
> > > So how about just:
> > > 1. moving  _scratch_mount before _require_timestamp_range
> > > 2. in _require_timestamp_range (untested):
> > >         if [ $tsmax -lt $((1<<32)) ] && ! _check_dmesg_for "supports
> >
> > Yeah, this looks fine. I thought about searching for dmesg warning after
> > _scratch_mount as well, but that would _notrun if the fs is 2038-safe.
> > This $tsmax check fixes my concern. Thanks!
> >
>
> Deepa,
>
> Do you intend to post the simplified version or would you like me
> to re-spin your patch?

I intend to do this. Sorry, was distracted by other things. FWIW, just
(1<<32) is not enough. The kernel prints this warning based on
(current time + max uptime) <= tsmax. Will post this.

-Deepa

Re: [PATCH v2] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

On Fri, Jan 17, 2020 at 10:23 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> On Fri, Jan 17, 2020 at 1:09 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > > > I understand why you dislike the ext2+loop test, but please hear me out.
> > > >
> > > > From all the fs types that are supported by the test, only btrfs and ext4 with
> > > > large inode size are y2038 ready.
> > > > For the rest of the cases, we actually have a way to detect kernel support
> > > > from the dmesg warning, without the need for hacky ext2 loop mount.
> > > >
> > > > So how about just:
> > > > 1. moving  _scratch_mount before _require_timestamp_range
> > > > 2. in _require_timestamp_range (untested):
> > > >         if [ $tsmax -lt $((1<<32)) ] && ! _check_dmesg_for "supports
> > >
> > > Yeah, this looks fine. I thought about searching for dmesg warning after
> > > _scratch_mount as well, but that would _notrun if the fs is 2038-safe.
> > > This $tsmax check fixes my concern. Thanks!
> > >
> >
> > Deepa,
> >
> > Do you intend to post the simplified version or would you like me
> > to re-spin your patch?
>
> I intend to do this. Sorry, was distracted by other things. FWIW, just
> (1<<32) is not enough. The kernel prints this warning based on
> (current time + max uptime) <= tsmax. Will post this.

Also we are testing for the following timestamps in the test:

declare -a TIMESTAMPS=(
        $tsmin
         0
        $tsmax
        $((tsmax/2))
        $((tsmax+1))
)

So the test can still fail if the above tsmax test passes, but the
limit patches are not in the kernel.
https://lkml.org/lkml/2019/7/29/1850 has the limits for all the
filesystems I filled in. fat, cifs, hpfs etc. all fall in this
category.

I do not see a good way of doing this unless we test for a fs that
always fails in a predictable way.

Checking for fsinfo maybe is ok. But, at that point we could just rely
on any syscall that got merged after the limits series isn't it?

-Deepa

[PATCH v3 1/1] generic/402: Make timestamp range check conditional

[Deepa Dinamani]

Addition of fs-specific timestamp range checking was added
in 188d20bcd1eb ("vfs: Add file timestamp range support").

Add a check for whether the kernel supports the limits check
before running the associated test.

Based on an off-list discussion, we use a simpler interim approach
until fsinfo syscall would provide fs timestamp limits info.
This isn't perfect, but works for filesystems expiring in 2038.

Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 common/rc         |  7 +++++++
 tests/generic/402 | 13 ++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/common/rc b/common/rc
index eeac1355..fc82c17a 100644
--- a/common/rc
+++ b/common/rc
@@ -1990,6 +1990,13 @@ _require_timestamp_range()
 	if [ $tsmin -eq -1 -a $tsmax -eq -1 ]; then
 		_notrun "filesystem $FSTYP timestamp bounds are unknown"
 	fi
+
+	# expect console warning from rw scratch mount if fs limit is near
+	if [ $tsmax -le $((1<<31)) ] && \
+		! _check_dmesg_for "filesystem being mounted at .* supports timestamps until"
+	then
+		_notrun "Kernel does not support timestamp limits"
+	fi
 }
 
 _filesystem_timestamp_range()
diff --git a/tests/generic/402 b/tests/generic/402
index 0392c258..2a34d127 100755
--- a/tests/generic/402
+++ b/tests/generic/402
@@ -16,7 +16,13 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
 
 # Get standard environment, filters and checks.
 . ./common/rc
@@ -30,6 +36,7 @@ rm -f $seqres.full
 _supported_fs generic
 _supported_os Linux
 _require_scratch
+_require_check_dmesg
 _require_xfs_io_command utimes
 
 # Compare file timestamps obtained from stat
@@ -80,6 +87,8 @@ run_test()
 }
 
 _scratch_mkfs &>> $seqres.full 2>&1 || _fail "mkfs failed"
+_scratch_mount || _fail "scratch mount failed"
+
 _require_timestamp_range $SCRATCH_DEV
 
 read tsmin tsmax <<<$(_filesystem_timestamp_range $SCRATCH_DEV)
@@ -96,8 +105,6 @@ declare -a TIMESTAMPS=(
 	$((tsmax+1))
 )
 
-_scratch_mount || _fail "scratch mount failed"
-
 status=0
 
 # Begin test case 1
-- 
2.17.1

Re: [PATCH v3 1/1] generic/402: Make timestamp range check conditional

[Amir Goldstein]

On Sun, Jan 19, 2020 at 2:57 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
>
> Addition of fs-specific timestamp range checking was added
> in 188d20bcd1eb ("vfs: Add file timestamp range support").
>
> Add a check for whether the kernel supports the limits check
> before running the associated test.
>
> Based on an off-list discussion, we use a simpler interim approach
> until fsinfo syscall would provide fs timestamp limits info.
> This isn't perfect, but works for filesystems expiring in 2038.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> ---

Excellent! Thank you.

Eryu, you may add any of:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Tested-by: Amir Goldstein <amir73il@gmail.com>

On kernel v5.4, ext2,ext4,xfs,btrfs (default mkfs) still pass.
On Kernel v5.3, ext2,xfs are notrun for lack of kernel support
(instead of failing),
ext4 (256 bytes inodes) still fails
and btrfs still pass, because bash overflows $(($s64max + 1 )) just the same as
the kernel...

Thanks,
Amir.

Re: [PATCH v3 1/1] generic/402: Make timestamp range check conditional

[Eryu Guan]

On Sun, Jan 19, 2020 at 11:19:24AM +0200, Amir Goldstein wrote:
> On Sun, Jan 19, 2020 at 2:57 AM Deepa Dinamani <deepa.kernel@gmail.com> wrote:
> >
> > Addition of fs-specific timestamp range checking was added
> > in 188d20bcd1eb ("vfs: Add file timestamp range support").
> >
> > Add a check for whether the kernel supports the limits check
> > before running the associated test.
> >
> > Based on an off-list discussion, we use a simpler interim approach
> > until fsinfo syscall would provide fs timestamp limits info.
> > This isn't perfect, but works for filesystems expiring in 2038.
> >
> > Suggested-by: Amir Goldstein <amir73il@gmail.com>
> > Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
> > ---
> 
> Excellent! Thank you.
> 
> Eryu, you may add any of:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> Tested-by: Amir Goldstein <amir73il@gmail.com>
> 
> On kernel v5.4, ext2,ext4,xfs,btrfs (default mkfs) still pass.
> On Kernel v5.3, ext2,xfs are notrun for lack of kernel support
> (instead of failing),
> ext4 (256 bytes inodes) still fails
> and btrfs still pass, because bash overflows $(($s64max + 1 )) just the same as
> the kernel...

Thanks a lot for the double checking! And many thanks to Deepa for the
fix!

Thanks,
Eryu