[PATCH blktests] nvme: add regression test for concurrently enable/disable nvmet ns

[Chaitanya Kulkarni]

On 5/21/24 01:56, Sagi Grimberg wrote:
> Reproduce a hang in nvmet when concurrently disabling/enabling
> an nvmet namespace.
>
> Signed-off-by: Sagi Grimberg <sagi at grimberg.me>

thanks for the test  ...

> ---
>   tests/nvme/051     | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/051.out |  2 ++
>   2 files changed, 50 insertions(+)
>   create mode 100755 tests/nvme/051
>   create mode 100644 tests/nvme/051.out
>
> diff --git a/tests/nvme/051 b/tests/nvme/051
> new file mode 100755
> index 000000000000..ddc097310dd7
> --- /dev/null
> +++ b/tests/nvme/051
> @@ -0,0 +1,48 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Sagi Grimberg
> +#
> +# Regression test for kernel hang when enabling/disabling nvmet
> +# namespace concurrently
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="test nvmet concurrent ns enable/disable"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +set_conditions() {
> +	_set_nvme_trtype "$@"
> +}
> +
> +ns_enable_disable_loop() {
> +	local ns="$1"

nit: how about :-

	local ns="${NVMET_CFS}subsystems/${def_subsysnqn}/namespaces/1"

and get rid of the argument at the call site ? unless there is
a plan to use different namespaces


> +	local iterations=200

nit: not sure if we need iterations as variable, unless there is
      a plan to make it configurable in the future maybe we can
      remove that ?

> +	for ((i = 1; i <= ${iterations}; i++)); do
> +		echo 0 > $ns/enable
> +		echo 1 > $ns/enable
> +	done
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	_setup_nvmet
> +
> +	_nvmet_target_setup
> +	ns="${NVMET_CFS}subsystems/${def_subsysnqn}/namespaces/1"
> +
> +	# fire off two enable/disable loops concurrently and wait
> +	# for them to complete...
> +	ns_enable_disable_loop $ns &
> +	ns_enable_disable_loop $ns &
> +	wait
> +
> +	_nvmet_target_cleanup
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/nvme/051.out b/tests/nvme/051.out
> new file mode 100644
> index 000000000000..156f0687aab2
> --- /dev/null
> +++ b/tests/nvme/051.out
> @@ -0,0 +1,2 @@
> +Running nvme/051
> +Test complete

irrespective of couple of nits, looks good.

Reviewed-by: Chaitanya Kulkarni <kch at nvidia.com>

-ck