On Aug 23, 2024 / 22:08, Martin Wilck wrote: > Add a test that repeatedly rescans nvme controllers while doing IO > on an nvme namespace connected to these controllers. The purpose > of the test is to make sure that no I/O errors or data corruption > occurs because of the rescan operations. The test uses sub-second > sleeps, which can't be easily accomplished in bash because of > missing floating-point arithmetic (and because usleep(1) isn't > portable). Therefore an awk program is used to trigger the > device rescans. Hello Martin, thanks for the series. I think this test case is good since it looks extending the code coverage. As for the rationale discussion for the v1 series, I suggest to add a "Link:" tag here: Link: https://lore.kernel.org/linux-nvme/20240822201413.112268-1-mwilck@xxxxxxxx/ > > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> > --- > v2: - don't use usleep (Nilay Shroff). Use an awk program to do floating > point arithmetic and achieve more accurate sub-second sleep times. > - add 053.out (Nilay Shroff). > --- > tests/nvme/053 | 70 ++++++++++++++++++++++++++++++++++++++++++++++ > tests/nvme/053.out | 2 ++ > tests/nvme/rc | 18 ++++++++++++ > 3 files changed, 90 insertions(+) > create mode 100755 tests/nvme/053 > create mode 100644 tests/nvme/053.out > > diff --git a/tests/nvme/053 b/tests/nvme/053 > new file mode 100755 > index 0000000..d32484c > --- /dev/null > +++ b/tests/nvme/053 > @@ -0,0 +1,70 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-3.0+ > +# Copyright (C) 2024 Martin Wilck, SUSE LLC I suggest to describe the test content here, something like, # # Repeatedly rescans nvme controllers while doing IO on an nvme namespace # connected to these controllers, and make sure that no I/O errors or data # corruption occurs. > + > +. tests/nvme/rc > + > +DESCRIPTION="test controller rescan under I/O load" > +TIMED=1 > +: "${TIMEOUT:=60}" > + > +rescan_controller() { > + local path > + path="$1/rescan_controller" > + > + [[ -f "$path" ]] || { > + echo "cannot rescan $1" > + return 1 > + } > + > + awk -f "$TMPDIR/rescan.awk" \ > + -v path="$path" -v timeout="$TIMEOUT" -v seed="$2" & > +} > + > +create_rescan_script() { > + cat >"$TMPDIR/rescan.awk" <<EOF > +@load "time" > + > +BEGIN { > + srand(seed); > + finish = gettimeofday() + strtonum(timeout); > + while (gettimeofday() < finish) { > + sleep(0.1 + 5 * rand()); > + printf("1\n") > path; > + close(path); > + } > +} > +EOF > +} > + > +test_device() { > + local -a ctrls > + local i Nit: 'st' can be decalread as the local variable too. I suggest to add another new local variable 'line' below, so I suggset "local i line st" here. > + > + echo "Running ${TEST_NAME}" > + create_rescan_script > + > + ctrls=($(_nvme_get_ctrl_list)) Shellcheck reports a warning here: tests/nvme/053:47:9: warning: Prefer mapfile or read -a to split command output (or quote to avoid splitting). [SC2207] It is a bit lengthy, but let's replace the line above with this: while read -r line ; do ctrls+=("$line"); done < <(_nvme_get_ctrl_list)
