On Jun 18, 2024 / 01:35, Chaitanya Kulkarni wrote: > On 6/17/24 02:17, Nilay Shroff wrote: > > I think subject line should start with nvme ? > > nvme: add test for creating/deleting file-ns > > > This is regression test for commit be647e2c76b2 > > (nvme: use srcu for iterating namespace list) > > > > This test uses a regulare file backed loop device > > for creating and then deleting an NVMe namespace > > in a loop. > > > s/regulare/regular/ ? > > nit:- commit log looks a bit short :- > > This is regression test for commit be647e2c76b2 > (nvme: use srcu for iterating namespace list) > > This test uses a regulare file backed loop device for creating and > then deleting an NVMe namespace in a loop. > > > Signed-off-by: Nilay Shroff <nilay@xxxxxxxxxxxxx> > > --- > > This regression was first reported[1], and now it's > > fixed in 6.10-rc4[2] > > > > [1] https://lore.kernel.org/all/2312e6c3-a069-4388-a863-df7e261b9d70@xxxxxxxxxxxxxxxxxx/ > > [2] commit ff0ffe5b7c3c (nvme: fix namespace removal list) > > it will be helpful in long run to add above information > into the commit log, Shinichiro any thoughts ? Agreed. It is helpful to record the kernel side fix commit and the link to the discussion threads in the blktests side commit log. > > > --- > > tests/nvme/051 | 65 ++++++++++++++++++++++++++++++++++++++++++++++ > > tests/nvme/051.out | 2 ++ > > 2 files changed, 67 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 0000000..0de5c56 > > --- /dev/null > > +++ b/tests/nvme/051 > > @@ -0,0 +1,65 @@ > > +#!/bin/bash > > +# SPDX-License-Identifier: GPL-3.0+ > > +# Copyright (C) 2024 Nilay Shroff(nilay@xxxxxxxxxxxxx) > > not sure we need to have email address here as it's a part of > commit log anyways ... > > > +# > > +# Regression test for commit be647e2c76b2(nvme: use srcu for iterating > > +# namespace list) It is also good to enrich this header comment section. Especially, the kernel side fix commit will be helpful. > > + > > +. tests/nvme/rc > > + > > +DESCRIPTION="Test file-ns creation/deletion under one subsystem" > > + > > +requires() { > > + _nvme_requires > > + _have_loop > > + _require_nvme_trtype_is_loop > > +} > > + > > +set_conditions() { > > + _set_nvme_trtype "$@" > > +} > > + > > +test() { > > + echo "Running ${TEST_NAME}" > > + > > + _setup_nvmet > > + > > + local subsys="blktests-subsystem-1" > > + local iterations="${NVME_NUM_ITER}" > > no need for above var, I think direct use of NVME_NUM_ITER is > clear here ... I ran this test case on my baremetal test node and QEMU test node using the kernel without the fix. On the baremetal test node, the kernel Oops was created soon. Good. On the QEMU test node, the Oops was not recreated, and the test case passed. For this pass case, it took more than 15 minutes to complete the test case. I think the default NVME_NUM_ITER=1000 is too much. Can we reduce it to 10 or 20? > > > + local loop_dev > > + local port > > + > > + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path)" > > + > > + loop_dev="$(losetup -f --show "$(_nvme_def_file_path)")" > > + > > + port="$(_create_nvmet_port "${nvme_trtype}")" > > + > > + _nvmet_target_setup --subsysnqn "${subsys}" --blkdev "${loop_dev}" > > + > > + _nvme_connect_subsys --subsysnqn "${subsys}" > > + > > + for ((i = 2; i <= iterations; i++)); do { > > small comment would be useful to explain why are starting at 2 ... > > > + truncate -s "${NVME_IMG_SIZE}" "$(_nvme_def_file_path).$i" > > + _create_nvmet_ns "${subsys}" "${i}" "$(_nvme_def_file_path).$i" > > + > > + # allow async request to be processed > > + sleep 1 > > + > > + _remove_nvmet_ns "${subsys}" "${i}" > > + rm "$(_nvme_def_file_path).$i" > > + } > > + done > > + > > + _nvme_disconnect_subsys --subsysnqn "${subsys}" >> "${FULL}" 2>&1 > > + > > + _nvmet_target_cleanup --subsysnqn "${subsys}" --blkdev "${loop_dev}" > > + > > + _remove_nvmet_port "${port}" > > + > > + losetup -d "$loop_dev" > > + > > + rm "$(_nvme_def_file_path)" > > + > > + echo "Test complete" > > +} > > diff --git a/tests/nvme/051.out b/tests/nvme/051.out > > new file mode 100644 > > index 0000000..156f068 > > --- /dev/null > > +++ b/tests/nvme/051.out > > @@ -0,0 +1,2 @@ > > +Running nvme/051 > > +Test complete > > thanks for the test, I think this is much needed test especially with > recent reported issues ... I also appreciate this patch. Thanks! One more request, recent commit added a test case to the nvme group and it has the number nvme/051. Could you renumber this test case to nvme/052?
