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

[Sagi Grimberg]

On 22/05/2024 3:54, Chaitanya Kulkarni wrote:
> 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

I think its better that the ns comes as an argument, maybe someone may 
make this
function a generic utility in the future.

>
>
>> +	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 ?

At first I used NVME_NUM_ITERS, but that made the test too long, I can
make it hard-coded.