Re: [PATCH blktests] tests/srp/014: Add a test that triggers a SCSI reset while I/O is ongoing

[Omar Sandoval <osandov@osandov.com>]

On Thu, Aug 01, 2019 at 03:09:37PM -0700, Bart Van Assche wrote:

Hi, Bart, a few comments.

> This test triggers the task management function handling code in the SRP
> initiator and target drivers. This test verifies the following kernel
> patch: fd5614124406 ("scsi: RDMA/srp: Fix a sleep-in-invalid-context
> bug") # v5.3.

The commit reference belongs in the test file itself (see, e.g.,
block/001).

> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
> 
> A note regarding the copyright notice: I wrote this patch more than a year
> ago. Hence the copyright attribution to Western Digital.
> 
>  tests/srp/014     | 104 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/srp/014.out |   2 +
>  2 files changed, 106 insertions(+)
>  create mode 100755 tests/srp/014
>  create mode 100644 tests/srp/014.out

> diff --git a/tests/srp/014 b/tests/srp/014
> new file mode 100755
> index 000000000000..bc2e844abdd2
> --- /dev/null
> +++ b/tests/srp/014
> @@ -0,0 +1,104 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2016-2018 Western Digital Corporation or its affiliates
> +
> +. tests/srp/rc
> +
> +DESCRIPTION="Run sg_reset while I/O is ongoing"
> +TIMED=1
> +

[snip]

> +test_sg_reset() {
> +	local dev fio_status m job jobfile
> +
> +	use_blk_mq y y || return $?
> +	dev=$(get_bdev 0) || return $?
> +	sg_reset_loop "$dev" "$TIMEOUT" &

TIMEOUT is only set if the user configured it, so you should set a
default (see block/001).

> +	jobfile=$(mktemp)
> +	# Redirect stderr to suppress the bash "Terminated" message.
> +	(set_running_loop "$dev" 2>/dev/null & echo $! > "$jobfile")

Why is the subshell/jobfile necessary here? The following seems to work,
am I missing something?

	set_running_loop "$dev" 2>/dev/null &
	job=$!

Thanks!

> +	job=$(<"$jobfile")
> +	rm -f "$jobfile"
> +	run_fio --verify=md5 --rw=randwrite --bs=64K --loops=$((10**6)) \
> +		--iodepth=16 --group_reporting --sync=1 --direct=1 \
> +		--ioengine=libaio --runtime="${TIMEOUT}" \
> +		--filename="$dev" --name=sg-reset-test --thread --numjobs=1 \
> +		--output="${RESULTS_DIR}/srp/fio-output-014.txt" \
> +		>>"$FULL"
> +	fio_status=$?
> +	kill "$job"
> +	make_all_running "$dev"
> +	wait
> +	return $fio_status
> +}
> +
> +test() {
> +	trap 'trap "" EXIT; teardown' EXIT
> +	setup && test_sg_reset && echo Passed
> +}
> diff --git a/tests/srp/014.out b/tests/srp/014.out
> new file mode 100644
> index 000000000000..5e25d8e8672d
> --- /dev/null
> +++ b/tests/srp/014.out
> @@ -0,0 +1,2 @@
> +Configured SRP target driver
> +Passed
> -- 
> 2.22.0.770.g0f2c4a37fd-goog
>