Skip to contentSkip to conversation

fs/ext4: enhance logic of ext4_nsec_timestamps_test

PATCH Superseded Yixin Zhang 4 participants 6 messages Apr 29, 2018 – May 4, 2018

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fs/ext4: enhance logic of ext4_nsec_timestamps_test

1. Check the big block devices at setup stage, umount it if it's mounted
2. Add "sleep 1" between mount and umount, to avoid umount failed due to device busy (accessed by system service like systemd-udev)
3. Add TFAIL if umount failed
4. Fix typo error

---
 .../ext4-nsec-timestamps/ext4_nsec_timestamps_test.sh      | 14 +++++++++-----
 testcases/kernel/fs/ext4-new-features/ext4_funcs.sh        |  6 ++++++
 2 files changed, 15 insertions(+), 5 deletions(-)
testcases/kernel/fs/ext4-new-features/ext4-nsec-timestamps/ext4_nsec_timestamps_test.sh +9 -5
index c6ff7c2ba..5d561adf3 100755
@@ -76,8 +76,8 @@ ext4_test_nsec_timestamps()
 
 	mkfs.ext3 -I 256 $EXT4_DEV >/dev/null 2>&1
 	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "failed to create ext4 filesystem"
-		return
+		tst_resm TFAIL "failed to create ext3 filesystem"
+	  return
 	fi
 
 	mount -t ext4 $EXT4_DEV mnt_point
@@ -105,8 +105,7 @@ ext4_test_nsec_timestamps()
 	nsec_ctime=`ext4_file_time mnt_point/tmp_file ctime nsec`
 
 	# Test nanosecond
-	if [ $nsec_atime -eq 0 -a $nsec_mtime -eq 0 -a $nsec_ctime -eq 0 ]
-	then
+	if [ $nsec_atime -eq 0 -a $nsec_mtime -eq 0 -a $nsec_ctime -eq 0 ]; then
 		tst_resm TFAIL "The timestamp is not nanosecond(nsec_atime: $nsec_atime, nsec_mtime: $nsec_mtime, nsec_ctime: $nsec_ctime)"
 		umount mnt_point
 		return
@@ -138,7 +137,12 @@ ext4_test_nsec_timestamps()
 		tst_resm TFAIL "failed to mount to ext3"
 		return
 	fi
+	sleep 1
 	umount mnt_point
+	if [ $? -ne 0 ]; then
+		tst_resm TFAIL "failed to umount ext3 filesystem"
+		return
+	fi
 
 	mount -t ext4 $EXT4_DEV mnt_point
 	if [ $? -ne 0 ]; then
@@ -148,7 +152,7 @@ ext4_test_nsec_timestamps()
 
 	nsec_atime2=`ext4_file_time mnt_point/tmp_file atime nsec`
 	nsec_mtime2=`ext4_file_time mnt_point/tmp_file mtime nsec`
-	nsec_ctime2=`ext4_file_time mnt_point/tmp_file mtime nsec`
+	nsec_ctime2=`ext4_file_time mnt_point/tmp_file ctime nsec`
 
 	if [ $nsec_atime -ne $nsec_atime2 -o $nsec_ctime -ne $nsec_ctime2 -o \
 	     $nsec_mtime -ne $nsec_mtime2 ]; then
testcases/kernel/fs/ext4-new-features/ext4_funcs.sh +6 -0
index a9eb54e8d..4de7899e4 100755
@@ -45,6 +45,12 @@ ext4_setup()
 		tst_brkm TCONF "tests need a big block device(5G-10G)"
 	else
 		export EXT4_DEV=$LTP_BIG_DEV
+		if mount | cut -d' ' -f1 | grep -q ^$EXT4_DEV$ ; then
+			umount $EXT4_DEV >/dev/null 2>&1
+			if [ $? -ne 0 ]; then
+				tst_brkm TCONF "umount $EXT4_DEV failed"
+			fi
+		fi
 	fi
 
 	tst_tmpdir
Signature · 1 line
2.14.1
#1 lore raw

PATCH 1/1 fs/ext4: enhance logic of ext4_nsec_timestamps_test

Hi!

Quoted 1 line

1. Check the big block devices at setup stage, umount it if it's mounted

I do not like this one much as this will only hide bugs in the rest of the tests that fail to umount the device.

I would rather see the test exit with TBROK and let the user handle the situation.

Quoted 2 lines

2. Add "sleep 1" between mount and umount, to avoid umount failed due to device busy (accessed by system service like systemd-udev)

NACK to this.

In a case that we need to wait for something like this we are supposed to retry in a loop with exponential backoff, which is usually faster than hardcoded sleep but also more robust.

There is a in-flight patch for adding TST_RETRY_FUNC function to the shell library "[PATCH] tst_test.sh: achieve TST_RETRY_FUNC function in shell" which should make this really easy but we would have to add it to the old shell test library as well so that we can reuse it here.

Quoted 2 lines

3. Add TFAIL if umount failed
4. Fix typo error

This looks good.

Quoted 75 lines
---
 .../ext4-nsec-timestamps/ext4_nsec_timestamps_test.sh      | 14 +++++++++-----
 testcases/kernel/fs/ext4-new-features/ext4_funcs.sh        |  6 ++++++
 2 files changed, 15 insertions(+), 5 deletions(-)
testcases/kernel/fs/ext4-new-features/ext4-nsec-timestamps/ext4_nsec_timestamps_test.sh +9 -5
index c6ff7c2ba..5d561adf3 100755
@@ -76,8 +76,8 @@ ext4_test_nsec_timestamps()
 
 	mkfs.ext3 -I 256 $EXT4_DEV >/dev/null 2>&1
 	if [ $? -ne 0 ]; then
-		tst_resm TFAIL "failed to create ext4 filesystem"
-		return
+		tst_resm TFAIL "failed to create ext3 filesystem"
+	  return
 	fi
 	mount -t ext4 $EXT4_DEV mnt_point
@@ -105,8 +105,7 @@ ext4_test_nsec_timestamps()
 	nsec_ctime=`ext4_file_time mnt_point/tmp_file ctime nsec`
 
 	# Test nanosecond
-	if [ $nsec_atime -eq 0 -a $nsec_mtime -eq 0 -a $nsec_ctime -eq 0 ]
-	then
+	if [ $nsec_atime -eq 0 -a $nsec_mtime -eq 0 -a $nsec_ctime -eq 0 ]; then
 		tst_resm TFAIL "The timestamp is not nanosecond(nsec_atime: $nsec_atime, nsec_mtime: $nsec_mtime, nsec_ctime: $nsec_ctime)"
 		umount mnt_point
 		return
@@ -138,7 +137,12 @@ ext4_test_nsec_timestamps()
 		tst_resm TFAIL "failed to mount to ext3"
 		return
 	fi
+	sleep 1
 	umount mnt_point
+	if [ $? -ne 0 ]; then
+		tst_resm TFAIL "failed to umount ext3 filesystem"
+		return
+	fi
 
 	mount -t ext4 $EXT4_DEV mnt_point
 	if [ $? -ne 0 ]; then
@@ -148,7 +152,7 @@ ext4_test_nsec_timestamps()
 
 	nsec_atime2=`ext4_file_time mnt_point/tmp_file atime nsec`
 	nsec_mtime2=`ext4_file_time mnt_point/tmp_file mtime nsec`
-	nsec_ctime2=`ext4_file_time mnt_point/tmp_file mtime nsec`
+	nsec_ctime2=`ext4_file_time mnt_point/tmp_file ctime nsec`
 
 	if [ $nsec_atime -ne $nsec_atime2 -o $nsec_ctime -ne $nsec_ctime2 -o \
 	     $nsec_mtime -ne $nsec_mtime2 ]; then
testcases/kernel/fs/ext4-new-features/ext4_funcs.sh +6 -0
index a9eb54e8d..4de7899e4 100755
@@ -45,6 +45,12 @@ ext4_setup()
 		tst_brkm TCONF "tests need a big block device(5G-10G)"
 	else
 		export EXT4_DEV=$LTP_BIG_DEV
+		if mount | cut -d' ' -f1 | grep -q ^$EXT4_DEV$ ; then
+			umount $EXT4_DEV >/dev/null 2>&1
+			if [ $? -ne 0 ]; then
+				tst_brkm TCONF "umount $EXT4_DEV failed"
+			fi
+		fi
 	fi
 
 	tst_tmpdir
Signature · 5 lines
2.14.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp
Signature · 2 lines
Cyril Hrubis
[email protected]
#2 lore raw

PATCH 1/1 fs/ext4: enhance logic of ext4_nsec_timestamps_test

Hi!

Quoted 5 lines
Quoted 2 lines

2. Add "sleep 1" between mount and umount, to avoid umount failed due to device busy (accessed by system service like systemd-udev)

NACK to this.

Quoted 8 lines

In a case that we need to wait for something like this we are supposed to retry in a loop with exponential backoff, which is usually faster than hardcoded sleep but also more robust.

There is a in-flight patch for adding TST_RETRY_FUNC function to the shell library "[PATCH] tst_test.sh: achieve TST_RETRY_FUNC function in shell" which should make this really easy but we would have to add it to the old shell test library as well so that we can reuse it here.

And I remebered that we already solved this by adding tst_umount function to the shell library. The systemd-udev is not first daemon to probe newly mounted filesystem, we used to have this kind of problems with gvfsd-trash as well.

So fixing this should be as easy as replacing the umount with tst_umount function.

Signature · 2 lines
Cyril Hrubis
[email protected]
#3 lore raw

PATCH 1/1 fs/ext4: enhance logic of ext4_nsec_timestamps_test

Hi,

Quoted 14 lines
Quoted 5 lines
Quoted 2 lines

2. Add "sleep 1" between mount and umount, to avoid umount failed due to device busy (accessed by system service like systemd-udev)

NACK to this.

Quoted 8 lines

In a case that we need to wait for something like this we are supposed to retry in a loop with exponential backoff, which is usually faster than hardcoded sleep but also more robust.

There is a in-flight patch for adding TST_RETRY_FUNC function to the shell library "[PATCH] tst_test.sh: achieve TST_RETRY_FUNC function in shell" which should make this really easy but we would have to add it to the old shell test library as well so that we can reuse it here.

Quoted 7 lines

And I remebered that we already solved this by adding tst_umount function to the shell library. The systemd-udev is not first daemon to probe newly mounted filesystem, we used to have this kind of problems with gvfsd-trash as well.

So fixing this should be as easy as replacing the umount with tst_umount function.

I'm using tst_umount in IMA tests and it works nice :).
BTW It's just that gvfsd-trash TINFO message is a bit too specific (in my case it was that some processes were still using the mounted filesystem - the same issue as Yixin has).

Kind regards,
Petr

#4 lore raw

PATCH 1/1 fs/ext4: enhance logic of ext4_nsec_timestamps_test

Hi!

Quoted 10 lines
Quoted 7 lines

And I remebered that we already solved this by adding tst_umount function to the shell library. The systemd-udev is not first daemon to probe newly mounted filesystem, we used to have this kind of problems with gvfsd-trash as well.

So fixing this should be as easy as replacing the umount with tst_umount function.

I'm using tst_umount in IMA tests and it works nice :).
BTW It's just that gvfsd-trash TINFO message is a bit too specific (in my case it was that some processes were still using the mounted filesystem - the same issue as Yixin has).

Yes, it is, we may want to reword it, but keep that after the release.

Signature · 2 lines
Cyril Hrubis
[email protected]
#5 lore raw

PATCH 1/1 fs/ext4: enhance logic of ext4_nsec_timestamps_test

Thanks Cyril and Petr, the v2 patch is sent with suggested modifications.

Yixin

Quoted 17 lines · Cyril Hrubis

Hi!

Quoted 10 lines
Quoted 7 lines

And I remebered that we already solved this by adding tst_umount function to the shell library. The systemd-udev is not first daemon to probe newly mounted filesystem, we used to have this kind of problems with gvfsd-trash as well.

So fixing this should be as easy as replacing the umount with tst_umount function.

I'm using tst_umount in IMA tests and it works nice :).
BTW It's just that gvfsd-trash TINFO message is a bit too specific (in my case it was that some processes were still using the mounted filesystem - the same issue as Yixin has).

Yes, it is, we may want to reword it, but keep that after the release.

Signature · 2 lines
Cyril Hrubis
[email protected]