Skip to contentSkip to conversation

nvme: add nvme pci timeout testcase

PATCH Superseded Chaitanya Kulkarni 3 participants 4 messages Jan 17, 2024 – Jan 22, 2024

Subsystems (2)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover nvme: add nvme pci timeout testcase

Hi,

Add a test to trigger nvme pci timeout handler function

Below is the test log.

-ck

Change log :-

1. Use test_device() and TEST_DEV.
2. Make tesctcase pci only just like in nvme/032.
3. Remove . from copyright and test description.
4. Add _have_kernel_option FAIL_IO_TIMEOUT.
5. Save and restore fault injection settings.
6. Use "$(nproc)" to get rid of shellcheck annotation.
7. Don't remove the device keep it as it is, add module remove testcase later.

Chaitanya Kulkarni (1):
nvme: add nvme pci timeout testcase

 tests/nvme/050     | 74 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/050.out |  2 ++
 2 files changed, 76 insertions(+)
 create mode 100755 tests/nvme/050
 create mode 100644 tests/nvme/050.out

blktests (master) # modprobe -r nvme
blktests (master) # modprobe nvme
blktests (master) # TEST_DEVS=/dev/nvme0n1 nvme_trtype=pci ./check nvme/050
nvme/050 => nvme0n1 (test nvme-pci timeout with fio jobs) [passed]

    runtime  90.854s  ...  91.087s

blktests (master) #

Signature · 1 line
2.40.0
#1 lore raw

PATCH v2 1/1 nvme: add nvme pci timeout testcase

Trigger and test nvme-pci timeout with concurrent fio jobs.

---
 tests/nvme/050     | 74 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/050.out |  2 ++
 2 files changed, 76 insertions(+)
 create mode 100755 tests/nvme/050
 create mode 100644 tests/nvme/050.out
tests/nvme/050 new file +74 -0
new file mode 100755
index 0000000..6c44b43
@@ -0,0 +1,74 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Chaitanya Kulkarni
+#
+# Test NVMe-PCI timeout with FIO jobs by triggering the nvme_timeout function.
+#
+
+. tests/nvme/rc
+
+DESCRIPTION="test nvme-pci timeout with fio jobs"
+
+sysfs_path="/sys/kernel/debug/fail_io_timeout/"
+#restrict test to nvme-pci only
+nvme_trtype=pci
+
+# fault injection config array
+declare -A fi_array
+
+requires() {
+        _require_nvme_trtype pci
+        _have_fio
+        _nvme_requires
+        _have_kernel_option FAIL_IO_TIMEOUT
+}
+
+device_requires() {
+	_require_test_dev_is_nvme
+}
+
+save_fi_settings() {
+	for fi_attr in probability interval times space verbose
+	do
+		fi_array["${fi_attr}"]=$(cat "${sysfs_path}/${fi_attr}")
+	done
+}
+
+restore_fi_settings() {
+	for fi_attr in probability interval times space verbose
+	do
+		echo "${fi_array["${fi_attr}"]}" > "${sysfs_path}/${fi_attr}"
+	done
+}
+
+test_device() {
+	local nvme_ns
+	local io_fimeout_fail
+
+	echo "Running ${TEST_NAME}"
+
+	nvme_ns="$(basename "${TEST_DEV}")"
+	io_fimeout_fail="$(cat /sys/block/"${nvme_ns}"/io-timeout-fail)"
+	save_fi_settings
+	echo 1 > /sys/block/"${nvme_ns}"/io-timeout-fail
+
+	echo 99 > /sys/kernel/debug/fail_io_timeout/probability
+	echo 10 > /sys/kernel/debug/fail_io_timeout/interval
+	echo -1 > /sys/kernel/debug/fail_io_timeout/times
+	echo  0 > /sys/kernel/debug/fail_io_timeout/space
+	echo  1 > /sys/kernel/debug/fail_io_timeout/verbose
+
+	fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \
+	    --name=reads --direct=1 --filename="${TEST_DEV}" --group_reporting \
+	    --time_based --runtime=1m 2>&1 | grep -q "Input/output error"
+
+	# shellcheck disable=SC2181
+	if [ $? -eq 0 ]; then
+		echo "Test complete"
+	else
+		echo "Test failed"
+	fi
+
+	restore_fi_settings
+	echo "${io_fimeout_fail}" > /sys/block/"${nvme_ns}"/io-timeout-fail
+}
tests/nvme/050.out new file +2 -0
new file mode 100644
index 0000000..b78b05f
@@ -0,0 +1,2 @@
+Running nvme/050
+Test complete
Signature · 1 line
2.40.0
#2 lore raw

Hi Chaitanya, thanks for this v2 patch.

I ran this new test case with several devices and observed these two:

1) The test case fails with my QEMU NMVE devices. It looks all of the inject failures get recovered by error handler, then no I/O error is reported to the fio process. I modified two fail_io_timeout parameters as follows to inject more failures, and saw the test case fails with the device. I suggest these parameters.

@@ -49,8 +50,8 @@ test_device() {
        save_fi_settings
        echo 1 > /sys/block/"${nvme_ns}"/io-timeout-fail
-       echo 99 > /sys/kernel/debug/fail_io_timeout/probability
-       echo 10 > /sys/kernel/debug/fail_io_timeout/interval
+       echo 100 > /sys/kernel/debug/fail_io_timeout/probability
+       echo 1 > /sys/kernel/debug/fail_io_timeout/interval

echo -1 > /sys/kernel/debug/fail_io_timeout/times echo 0 > /sys/kernel/debug/fail_io_timeout/space echo 1 > /sys/kernel/debug/fail_io_timeout/verbose

2) After I ran the test case, I see the test target device left with zero size. I think this is consistent as your report [*]. I still think device remove and rescan at the test case end is required to avoid impacts on following test cases. It will depend on the discussion for your report, though.

[*] https://lore.kernel.org/linux-nvme/[email protected]/

Please find my some more comments in line:

Quoted 16 lines · Chaitanya Kulkarni

Trigger and test nvme-pci timeout with concurrent fio jobs.

---
 tests/nvme/050     | 74 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/050.out |  2 ++
 2 files changed, 76 insertions(+)
 create mode 100755 tests/nvme/050
 create mode 100644 tests/nvme/050.out
tests/nvme/050 new file +0 -0
new file mode 100755
index 0000000..6c44b43

tests/nvme/050 line 10

Quoted 10 lines
tests/nvme/050 new file +10 -0
@@ -0,0 +1,74 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Chaitanya Kulkarni
+#
+# Test NVMe-PCI timeout with FIO jobs by triggering the nvme_timeout function.
+#
+
+. tests/nvme/rc
+
+DESCRIPTION="test nvme-pci timeout with fio jobs"

I ran this test case a ZNS drive today, and it looks working. I suggest to add:

CAN_BE_ZONED=1

tests/nvme/050 line 20

Quoted 10 lines
tests/nvme/050 new file +10 -0
+
+sysfs_path="/sys/kernel/debug/fail_io_timeout/"
+#restrict test to nvme-pci only
+nvme_trtype=pci
+
+# fault injection config array
+declare -A fi_array
+
+requires() {
+        _require_nvme_trtype pci

We can remove this line, since we added "nvme_trtype=pci" above.

tests/nvme/050 line 23

Quoted 3 lines
tests/nvme/050 new file +3 -0
+        _have_fio
+        _nvme_requires
+        _have_kernel_option FAIL_IO_TIMEOUT

Today, I found that this test case depends on another kernel option. Let's add,

_have_kernel_option FAULT_INJECTION_DEBUG_FS

tests/nvme/050 line 28

Quoted 5 lines
tests/nvme/050 new file +5 -0
+}
+
+device_requires() {
+	_require_test_dev_is_nvme
+}

Actually, this check is not required here, since it is already done in group_device_requires() in tests/nvme/rc. It is a small left work to remove the same device_requires() in nvme/032.

Quoted 25 lines
tests/nvme/050 new file +25 -0
+
+save_fi_settings() {
+	for fi_attr in probability interval times space verbose
+	do
+		fi_array["${fi_attr}"]=$(cat "${sysfs_path}/${fi_attr}")
+	done
+}
+
+restore_fi_settings() {
+	for fi_attr in probability interval times space verbose
+	do
+		echo "${fi_array["${fi_attr}"]}" > "${sysfs_path}/${fi_attr}"
+	done
+}
+
+test_device() {
+	local nvme_ns
+	local io_fimeout_fail
+
+	echo "Running ${TEST_NAME}"
+
+	nvme_ns="$(basename "${TEST_DEV}")"
+	io_fimeout_fail="$(cat /sys/block/"${nvme_ns}"/io-timeout-fail)"
+	save_fi_settings
+	echo 1 > /sys/block/"${nvme_ns}"/io-timeout-fail

tests/nvme/050 line 63

Quoted 10 lines
tests/nvme/050 new file +10 -0
+
+	echo 99 > /sys/kernel/debug/fail_io_timeout/probability
+	echo 10 > /sys/kernel/debug/fail_io_timeout/interval
+	echo -1 > /sys/kernel/debug/fail_io_timeout/times
+	echo  0 > /sys/kernel/debug/fail_io_timeout/space
+	echo  1 > /sys/kernel/debug/fail_io_timeout/verbose
+
+	fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \
+	    --name=reads --direct=1 --filename="${TEST_DEV}" --group_reporting \
+	    --time_based --runtime=1m 2>&1 | grep -q "Input/output error"

When I investigated the failure cause of this test case, I found fio output is useful. So, I suggest to keep the output in $FULL.

            --time_based --runtime=1m >&"$FULL"

tests/nvme/050 line 66

Quoted 3 lines
tests/nvme/050 new file +3 -0
+
+	# shellcheck disable=SC2181
+	if [ $? -eq 0 ]; then

With that $FULL file, the if statement can be as follows. And we don't need to disable SC2181 either.

if grep -q "Input/output error" "$FULL"; then

Quoted 19 lines
tests/nvme/050 new file +8 -0
+		echo "Test complete"
+	else
+		echo "Test failed"
+	fi
+
+	restore_fi_settings
+	echo "${io_fimeout_fail}" > /sys/block/"${nvme_ns}"/io-timeout-fail
+}
tests/nvme/050.out new file +2 -0
new file mode 100644
index 0000000..b78b05f
@@ -0,0 +1,2 @@
+Running nvme/050
+Test complete
Signature · 1 line
2.40.0
#3 lore raw
Quoted 12 lines · Shinichiro Kawasaki

Hi Chaitanya, thanks for this v2 patch.

I ran this new test case with several devices and observed these two:

1) The test case fails with my QEMU NMVE devices. It looks all of the inject failures get recovered by error handler, then no I/O error is reported to the fio process. I modified two fail_io_timeout parameters as follows to inject more failures, and saw the test case fails with the device. I suggest these parameters.

Review test_device() { · line 57

Quoted 10 lines
(quoted diff) +2 -2
@@ -49,8 +50,8 @@ test_device() {
         save_fi_settings
         echo 1 > /sys/block/"${nvme_ns}"/io-timeout-fail
 
-       echo 99 > /sys/kernel/debug/fail_io_timeout/probability
-       echo 10 > /sys/kernel/debug/fail_io_timeout/interval
+       echo 100 > /sys/kernel/debug/fail_io_timeout/probability
+       echo 1 > /sys/kernel/debug/fail_io_timeout/interval
         echo -1 > /sys/kernel/debug/fail_io_timeout/times
         echo  0 > /sys/kernel/debug/fail_io_timeout/space
         echo  1 > /sys/kernel/debug/fail_io_timeout/verbose

Let me test these and add to the V3. But it is really strange, current code pass testcase 100% of the time without above changes ...

Quoted 6 lines

2) After I ran the test case, I see the test target device left with zero size. I think this is consistent as your report [*]. I still think device remove and rescan at the test case end is required to avoid impacts on following test cases. It will depend on the discussion for your report, though.

Yes, still waiting on that, hopefully we will get some information to move this forward. The reason I didn't add remove/rescan deviec coz it helped me debug the state of the device post timeout handler which is turned out to be not consistent for sure ..

Quoted 34 lines

[*] https://lore.kernel.org/linux-nvme/[email protected]/

Please find my some more comments in line:

Quoted 16 lines · Chaitanya Kulkarni

Trigger and test nvme-pci timeout with concurrent fio jobs.

---
  tests/nvme/050     | 74 ++++++++++++++++++++++++++++++++++++++++++++++
  tests/nvme/050.out |  2 ++
  2 files changed, 76 insertions(+)

create mode 100755 tests/nvme/050
create mode 100644 tests/nvme/050.out

tests/nvme/050 new file +0 -0
new file mode 100755
index 0000000..6c44b43

tests/nvme/050 line 10

Quoted 10 lines
tests/nvme/050 new file +10 -0
@@ -0,0 +1,74 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Chaitanya Kulkarni
+#
+# Test NVMe-PCI timeout with FIO jobs by triggering the nvme_timeout function.
+#
+
+. tests/nvme/rc
+
+DESCRIPTION="test nvme-pci timeout with fio jobs"

I ran this test case a ZNS drive today, and it looks working. I suggest to add:

Quoted 2 lines

CAN_BE_ZONED=1

okay ..

Quoted 4 lines
Quoted 4 lines
tests/nvme/050 new file +4 -0
+
+sysfs_path="/sys/kernel/debug/fail_io_timeout/"
+#restrict test to nvme-pci only
+nvme_trtype=pci
Quoted 8 lines

tests/nvme/050 line 20

Quoted 6 lines
tests/nvme/050 new file +6 -0
+
+# fault injection config array
+declare -A fi_array
+
+requires() {
+        _require_nvme_trtype pci

We can remove this line, since we added "nvme_trtype=pci" above.

yeah ..

Quoted 8 lines

tests/nvme/050 line 23

Quoted 4 lines
tests/nvme/050 new file +3 -0
+        _have_fio
+        _nvme_requires
+        _have_kernel_option FAIL_IO_TIMEOUT

Today, I found that this test case depends on another kernel option. Let's add,

_have_kernel_option FAULT_INJECTION_DEBUG_FS

indeed good catch I'll add this ..

Quoted 11 lines

tests/nvme/050 line 28

Quoted 6 lines
tests/nvme/050 new file +5 -0
+}
+
+device_requires() {
+	_require_test_dev_is_nvme
+}

Actually, this check is not required here, since it is already done in group_device_requires() in tests/nvme/rc. It is a small left work to remove the same device_requires() in nvme/032.

okay will remove that ..

Quoted 39 lines
Quoted 25 lines
tests/nvme/050 new file +25 -0
+
+save_fi_settings() {
+	for fi_attr in probability interval times space verbose
+	do
+		fi_array["${fi_attr}"]=$(cat "${sysfs_path}/${fi_attr}")
+	done
+}
+
+restore_fi_settings() {
+	for fi_attr in probability interval times space verbose
+	do
+		echo "${fi_array["${fi_attr}"]}" > "${sysfs_path}/${fi_attr}"
+	done
+}
+
+test_device() {
+	local nvme_ns
+	local io_fimeout_fail
+
+	echo "Running ${TEST_NAME}"
+
+	nvme_ns="$(basename "${TEST_DEV}")"
+	io_fimeout_fail="$(cat /sys/block/"${nvme_ns}"/io-timeout-fail)"
+	save_fi_settings
+	echo 1 > /sys/block/"${nvme_ns}"/io-timeout-fail

tests/nvme/050 line 63

Quoted 10 lines
tests/nvme/050 new file +10 -0
+
+	echo 99 > /sys/kernel/debug/fail_io_timeout/probability
+	echo 10 > /sys/kernel/debug/fail_io_timeout/interval
+	echo -1 > /sys/kernel/debug/fail_io_timeout/times
+	echo  0 > /sys/kernel/debug/fail_io_timeout/space
+	echo  1 > /sys/kernel/debug/fail_io_timeout/verbose
+
+	fio --bs=4k --rw=randread --norandommap --numjobs="$(nproc)" \
+	    --name=reads --direct=1 --filename="${TEST_DEV}" --group_reporting \
+	    --time_based --runtime=1m 2>&1 | grep -q "Input/output error"

When I investigated the failure cause of this test case, I found fio output is useful. So, I suggest to keep the output in $FULL.

Quoted 2 lines
             --time_based --runtime=1m >&"$FULL"

okay and will modify the testcase for that ..

Quoted 8 lines

tests/nvme/050 line 66

Quoted 3 lines
tests/nvme/050 new file +3 -0
+
+	# shellcheck disable=SC2181
+	if [ $? -eq 0 ]; then

With that $FULL file, the if statement can be as follows. And we don't need to disable SC2181 either.

if grep -q "Input/output error" "$FULL"; then

agree ..

Quoted 11 lines
Quoted 11 lines
tests/nvme/050 new file +8 -0
+		echo "Test complete"
+	else
+		echo "Test failed"
+	fi
+
+	restore_fi_settings
+	echo "${io_fimeout_fail}" > /sys/block/"${nvme_ns}"/io-timeout-fail
+}
tests/nvme/050.out new file +0 -0
new file mode 100644
Quoted 8 lines
Quoted 8 lines

index 0000000..b78b05f

tests/nvme/050.out new file +2 -0
@@ -0,0 +1,2 @@
+Running nvme/050
+Test complete
Signature · 1 line
2.40.0

Thanks for the review comments, I'll send out V3 with your suggestions ..

-ck