On 04/07/2020 07:17 AM, 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
can we skip the test ? I think Omar added a feature to skip the test.
> + 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
Same here.
> + 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"
> +
Shouldn't we check if new queue count is set here by reading
write_queues ?
> + # reset controller, make it effective
> + file="$sys_dev/reset_controller"
> + if [[ ! -e "$file" ]]; then
> + echo "$file does not exist"
> + return 1
> + fi
I think we need to add a helper to verify all the files and skip the
test required file doesn't exists. Also, please use different variables
representing different files.
> + 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
> + fi
> +
> + # read data from device (may kernel panic)
> + dd if="${TEST_DEV}" of=/dev/null bs=4096 count=1 status=none
> +
This should not lead to the kernel panic. Do you have a patch to fix
the panic ? If not can you please provide information so that we can
fix the panic and make test a test which will not result in panic ?
> + # 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"
> +
> + # 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
Just a write a helper for dd command instead of hard-coding it.
> +
> + 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
>
