On Tue, Apr 07, 2020 at 10:16:33PM +0800, Weiping Zhang wrote:
> Modify nvme module parameter write_queues to change hardware
> queue count, then reset nvme controller to reinitialize nvme
> with different queue count.
>
> Attention, this test case may trigger a kernel panic.
>
> Signed-off-by: Weiping Zhang <zhangweiping@xxxxxxxxxxxxxx>
> ---
> tests/nvme/033 | 87 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/nvme/033.out | 2 ++
> 2 files changed, 89 insertions(+)
> create mode 100755 tests/nvme/033
> create mode 100644 tests/nvme/033.out
>
> diff --git a/tests/nvme/033 b/tests/nvme/033
> new file mode 100755
> index 0000000..e3b9211
> --- /dev/null
> +++ b/tests/nvme/033
> @@ -0,0 +1,87 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2020 Weiping Zhang <zwp10758@xxxxxxxxx>
> +#
> +# Test nvme update hardware queue count larger than system cpu count
> +#
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="test nvme update hardware queue count larger than system cpu count"
> +QUICK=1
> +
> +requires() {
> + _have_program dd
> +}
> +
> +device_requires() {
> + _test_dev_is_nvme
> +}
> +
> +test_device() {
> + echo "Running ${TEST_NAME}"
> +
> + local old_write_queues
> + local cur_hw_io_queues
> + local file
> + local sys_dev=$TEST_DEV_SYSFS/device
> +
> + # backup old module parameter: write_queues
> + file=/sys/module/nvme/parameters/write_queues
> + if [[ ! -e "$file" ]]; then
> + echo "$file does not exist"
> + return 1
> + fi
Tests shouldn't fail if kernel support is missing. You can check this
with `_have_module_param nvme write_queues` in requires().
> + old_write_queues="$(cat $file)"
> +
> + # get current hardware queue count
> + file="$sys_dev/queue_count"
> + if [[ ! -e "$file" ]]; then
> + echo "$file does not exist"
> + return 1
> + fi
Again, this should be checked before the test is run (device_requires(),
in this case). Something like
if [[ ! -e "$TEST_DEV_SYSFS/device/queue_count" ]]; then
SKIP_REASON="device does have queue_count sysfs attribute"
return
fi
> + cur_hw_io_queues="$(cat "$file")"
> + # minus admin queue
> + cur_hw_io_queues=$((cur_hw_io_queues - 1))
> +
> + # set write queues count to increase more hardware queues
> + file=/sys/module/nvme/parameters/write_queues
> + echo "$cur_hw_io_queues" > "$file"
> +
> + # reset controller, make it effective
> + file="$sys_dev/reset_controller"
> + if [[ ! -e "$file" ]]; then
> + echo "$file does not exist"
> + return 1
> + fi
Same thing here.
> + echo 1 > "$file"
> +
> + # wait nvme reinitialized
> + for ((m = 0; m < 10; m++)); do
> + if [[ -b "${TEST_DEV}" ]]; then
> + break
> + fi
> + sleep 0.5
> + done
> + if (( m > 9 )); then
> + echo "nvme still not reinitialized after 5 seconds!"
> + return 1
You're not resetting the write_queues parameter here.
> + fi
> +
> + # read data from device (may kernel panic)
> + dd if="${TEST_DEV}" of=/dev/null bs=4096 count=1 status=none
> +
> + # If all work well restore hardware queue to default
> + file=/sys/module/nvme/parameters/write_queues
> + echo "$old_write_queues" > "$file"
> +
> + # reset controller
> + file="$sys_dev/reset_controller"
> + echo 1 > "$file"
Does this need the same logic to wait for the device to reappear?
> + # read data from device (may kernel panic)
> + dd if="${TEST_DEV}" of=/dev/null bs=4096 count=1 iflag=direct status=none
> + dd if=/dev/zero of="${TEST_DEV}" bs=4096 count=1 oflag=direct status=none
> +
> + echo "Test complete"
> +}
> diff --git a/tests/nvme/033.out b/tests/nvme/033.out
> new file mode 100644
> index 0000000..9648c73
> --- /dev/null
> +++ b/tests/nvme/033.out
> @@ -0,0 +1,2 @@
> +Running nvme/033
> +Test complete
> --
> 2.18.1
>
