[PATCH blktests v4 0/4] Test the blk_mq_realloc_hw_ctxs() error path

[PATCH blktests v4 0/4] Test the blk_mq_realloc_hw_ctxs() error path

[Bart Van Assche]

Hi Omar,

Please consider this patch series for the official blktests repository.

Thanks,

Bart.

Changes compared to v3:
- Addressed Daniel Wagner's review comments.

Changes compared to v2:
- Addressed Omar's review comments.

Changes between v1 and v2:
- Added three patches that refactor null_blk loading, unloading and
  configuration (Chaitanya).

Bart Van Assche (4):
  Make _exit_null_blk remove all null_blk device instances
  Use _{init,exit}_null_blk instead of open-coding these functions
  Introduce the function _configure_null_blk()
  Add a test that triggers the blk_mq_realloc_hw_ctxs() error path

 common/multipath-over-rdma | 29 ++++++---------------
 common/null_blk            | 21 ++++++++++++++-
 tests/block/022            |  3 ---
 tests/block/029            | 17 ++-----------
 tests/block/030            | 52 ++++++++++++++++++++++++++++++++++++++
 tests/block/030.out        |  1 +
 tests/nvmeof-mp/rc         |  2 +-
 7 files changed, 84 insertions(+), 41 deletions(-)
 create mode 100755 tests/block/030
 create mode 100644 tests/block/030.out

[PATCH blktests v4 1/4] Make _exit_null_blk remove all null_blk device instances

[Bart Van Assche]

Instead of making every test remove null_blk device instances before calling
_exit_null_blk(), move the null_blk device instance removal code into
_exit_null_blk().

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 common/null_blk | 7 ++++++-
 tests/block/022 | 3 ---
 tests/block/029 | 1 -
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/common/null_blk b/common/null_blk
index 2e300c20bbc7..a4140e365955 100644
--- a/common/null_blk
+++ b/common/null_blk
@@ -8,11 +8,15 @@ _have_null_blk() {
 	_have_modules null_blk
 }
 
-_init_null_blk() {
+_remove_null_blk_devices() {
 	if [[ -d /sys/kernel/config/nullb ]]; then
 		find /sys/kernel/config/nullb -mindepth 1 -maxdepth 1 \
 		     -type d -delete
 	fi
+}
+
+_init_null_blk() {
+	_remove_null_blk_devices
 
 	local zoned=""
 	if (( RUN_FOR_ZONED )); then zoned="zoned=1"; fi
@@ -26,6 +30,7 @@ _init_null_blk() {
 }
 
 _exit_null_blk() {
+	_remove_null_blk_devices
 	udevadm settle
 	modprobe -r null_blk
 }
diff --git a/tests/block/022 b/tests/block/022
index 1404aacef295..b2c53e266d81 100755
--- a/tests/block/022
+++ b/tests/block/022
@@ -50,9 +50,6 @@ test() {
 		wait $pid1
 	} 2>/dev/null
 
-	rmdir /sys/kernel/config/nullb/1
-	rmdir /sys/kernel/config/nullb/0
-
 	_exit_null_blk
 	echo "Test complete"
 }
diff --git a/tests/block/029 b/tests/block/029
index d298bac8db5c..0d521edb0cf6 100755
--- a/tests/block/029
+++ b/tests/block/029
@@ -58,7 +58,6 @@ test() {
 	else
 		echo "Skipping test because $sq cannot be modified" >>"$FULL"
 	fi
-	rmdir /sys/kernel/config/nullb/nullb0
 	_exit_null_blk
 	echo Passed
 }

[PATCH blktests v4 2/4] Use _{init,exit}_null_blk instead of open-coding these functions

[Bart Van Assche]

This patch reduces code duplication.

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 common/multipath-over-rdma | 12 +++---------
 tests/nvmeof-mp/rc         |  2 +-
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
index 40efc4b3aa2e..a56e7a8269db 100644
--- a/common/multipath-over-rdma
+++ b/common/multipath-over-rdma
@@ -5,6 +5,7 @@
 # Functions and global variables used by both the srp and nvmeof-mp tests.
 
 . common/shellcheck
+. common/null_blk
 
 debug=
 filesystem_type=ext4
@@ -634,13 +635,6 @@ configure_null_blk() {
 	ls -l /dev/nullb* &>>"$FULL"
 }
 
-unload_null_blk() {
-	local d
-
-	for d in /sys/kernel/config/nullb/*; do [ -d "$d" ] && rmdir "$d"; done
-	unload_module null_blk
-}
-
 setup_rdma() {
 	start_soft_rdma
 	(
@@ -662,7 +656,7 @@ teardown_uncond() {
 	rm -f /etc/multipath.conf
 	stop_target
 	stop_soft_rdma
-	unload_null_blk
+	_exit_null_blk
 }
 
 teardown() {
@@ -698,7 +692,7 @@ setup_test() {
 		[ -e "/sys/module/$m" ] || modprobe "$m" || return $?
 	done
 
-	modprobe null_blk nr_devices=0 || return $?
+	_init_null_blk nr_devices=0 || return $?
 
 	configure_null_blk || return $?
 
diff --git a/tests/nvmeof-mp/rc b/tests/nvmeof-mp/rc
index 1fd631445921..136163bc73ad 100755
--- a/tests/nvmeof-mp/rc
+++ b/tests/nvmeof-mp/rc
@@ -278,7 +278,7 @@ stop_nvme_target() {
 	)
 	unload_module nvmet_rdma &&
 		unload_module nvmet &&
-		unload_null_blk
+		_exit_null_blk
 }
 
 start_target() {

[PATCH blktests v4 3/4] Introduce the function _configure_null_blk()

[Bart Van Assche]

Introduce a function for creating a null_blk device instance through
configfs.

Suggested-by: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 common/multipath-over-rdma | 17 +++++------------
 common/null_blk            | 14 ++++++++++++++
 tests/block/029            | 16 ++--------------
 3 files changed, 21 insertions(+), 26 deletions(-)

diff --git a/common/multipath-over-rdma b/common/multipath-over-rdma
index a56e7a8269db..e689a9633d13 100644
--- a/common/multipath-over-rdma
+++ b/common/multipath-over-rdma
@@ -620,18 +620,11 @@ run_fio() {
 configure_null_blk() {
 	local i
 
-	(
-		cd /sys/kernel/config/nullb || return $?
-		for i in nullb0 nullb1; do (
-			{ mkdir -p $i &&
-				  cd $i &&
-				  echo 0 > completion_nsec &&
-				  echo 512 > blocksize &&
-				  echo $((ramdisk_size>>20)) > size &&
-				  echo 1 > memory_backed &&
-				  echo 1 > power; } || exit $?
-		) done
-	)
+	for i in nullb0 nullb1; do
+		_configure_null_blk $i completion_nsec=0 blocksize=512 \
+				    size=$((ramdisk_size>>20)) memory_backed=1 \
+				    power=1 || return $?
+	done
 	ls -l /dev/nullb* &>>"$FULL"
 }
 
diff --git a/common/null_blk b/common/null_blk
index a4140e365955..6611db03a00e 100644
--- a/common/null_blk
+++ b/common/null_blk
@@ -29,6 +29,20 @@ _init_null_blk() {
 	return 0
 }
 
+# Configure one null_blk instance with name $1 and parameters $2..${$#}.
+_configure_null_blk() {
+	local nullb=/sys/kernel/config/nullb/$1 param val
+
+	shift
+	mkdir "$nullb" || return $?
+	while [[ $# -gt 0 ]]; do
+		param="${1%%=*}"
+		val="${1#*=}"
+		shift
+		echo "$val" > "$nullb/$param" || return $?
+	done
+}
+
 _exit_null_blk() {
 	_remove_null_blk_devices
 	udevadm settle
diff --git a/tests/block/029 b/tests/block/029
index 0d521edb0cf6..dbb582eab473 100755
--- a/tests/block/029
+++ b/tests/block/029
@@ -14,19 +14,6 @@ requires() {
 	_have_null_blk
 }
 
-# Configure one null_blk instance.
-configure_null_blk() {
-	local nullb0="/sys/kernel/config/nullb/nullb0"
-
-	mkdir "$nullb0" &&
-	echo 0 > "$nullb0/completion_nsec" &&
-	echo 512 > "$nullb0/blocksize" &&
-	echo 16 > "$nullb0/size" &&
-	echo 1 > "$nullb0/memory_backed" &&
-	echo 1 > "$nullb0/power" &&
-	ls -l /dev/nullb* &>>"$FULL"
-}
-
 modify_nr_hw_queues() {
 	local deadline num_cpus
 
@@ -45,7 +32,8 @@ test() {
 
 	: "${TIMEOUT:=30}"
 	_init_null_blk nr_devices=0 queue_mode=2 &&
-	configure_null_blk
+	_configure_null_blk nullb0 completion_nsec=0 blocksize=512 \
+			    size=16 memory_backed=1 power=1 &&
 	if { echo 1 >$sq; } 2>/dev/null; then
 		modify_nr_hw_queues &
 		fio --rw=randwrite --bs=4K --loops=$((10**6)) \

[PATCH blktests v4 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path

[Bart Van Assche]

Add a test that triggers the code touched by commit d0930bb8f46b ("blk-mq:
Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()"). This
test only runs if a recently added fault injection feature is available,
namely commit 596444e75705 ("null_blk: Add support for init_hctx() fault
injection").

Cc: Ming Lei <ming.lei@redhat.com>
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 tests/block/030     | 52 +++++++++++++++++++++++++++++++++++++++++++++
 tests/block/030.out |  1 +
 2 files changed, 53 insertions(+)
 create mode 100755 tests/block/030
 create mode 100644 tests/block/030.out

diff --git a/tests/block/030 b/tests/block/030
new file mode 100755
index 000000000000..2b159510e586
--- /dev/null
+++ b/tests/block/030
@@ -0,0 +1,52 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2020 Google LLC
+#
+# Trigger the blk_mq_realloc_hw_ctxs() error path.
+
+. tests/block/rc
+. common/null_blk
+
+DESCRIPTION="trigger the blk_mq_realloc_hw_ctxs() error path"
+QUICK=1
+
+requires() {
+	_have_null_blk || return $?
+	_have_module_param null_blk init_hctx || return $?
+}
+
+test() {
+	local i sq=/sys/kernel/config/nullb/nullb0/submit_queues
+
+	: "${TIMEOUT:=30}"
+	# Legend: init_hctx=<interval>,<probability>,<space>,<times>
+	# Set <space> to $(nproc) + 1 to make loading of null_blk succeed.
+	if ! _init_null_blk nr_devices=0 queue_mode=2 \
+	     "init_hctx=$(nproc),100,$(($(nproc)+1)),-1"; then
+		echo "Loading null_blk failed"
+		return 1
+	fi
+	if ! _configure_null_blk nullb0 completion_nsec=0 blocksize=512 size=16\
+	     submit_queues="$(nproc)" memory_backed=1 power=1; then
+		echo "Configuring null_blk failed"
+		return 1
+	fi
+	# Since older null_blk versions do not allow "submit_queues" to be
+	# modified, check first whether that configs attribute is writeable.
+	# Each iteration of the loop below triggers $(nproc) + 1
+	# null_init_hctx() calls. Since <interval>=$(nproc), all possible
+	# blk_mq_realloc_hw_ctxs() error paths will be triggered. Whether or
+	# not this test succeeds depends on whether or not _check_dmesg()
+	# detects a kernel warning.
+	if { echo "$(<"$sq")" >$sq; } 2>/dev/null; then
+		for ((i=0;i<100;i++)); do
+			echo 1 >$sq
+			nproc >$sq
+		done
+	else
+		SKIP_REASON="Skipping test because $sq cannot be modified"
+	fi
+	rmdir /sys/kernel/config/nullb/nullb0
+	_exit_null_blk
+	echo Passed
+}
diff --git a/tests/block/030.out b/tests/block/030.out
new file mode 100644
index 000000000000..863339fb8ced
--- /dev/null
+++ b/tests/block/030.out
@@ -0,0 +1 @@
+Passed

Re: [PATCH blktests v4 4/4] Add a test that triggers the blk_mq_realloc_hw_ctxs() error path

[Daniel Wagner]

Hi Bart,

On Sat, Mar 28, 2020 at 11:22:51AM -0700, Bart Van Assche wrote:
> Add a test that triggers the code touched by commit d0930bb8f46b ("blk-mq:
> Fix a recently introduced regression in blk_mq_realloc_hw_ctxs()"). This
> test only runs if a recently added fault injection feature is available,
> namely commit 596444e75705 ("null_blk: Add support for init_hctx() fault
> injection").
> 
> Cc: Ming Lei <ming.lei@redhat.com>
> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>

Reviewed-by: Daniel Wagner <dwagner@suse.de>

Thanks,
Daniel

Re: [PATCH blktests v4 0/4] Test the blk_mq_realloc_hw_ctxs() error path

[Omar Sandoval]

On Sat, Mar 28, 2020 at 11:22:47AM -0700, Bart Van Assche wrote:
> Hi Omar,
> 
> Please consider this patch series for the official blktests repository.
> 
> Thanks,
> 
> Bart.
> 
> Changes compared to v3:
> - Addressed Daniel Wagner's review comments.
> 
> Changes compared to v2:
> - Addressed Omar's review comments.
> 
> Changes between v1 and v2:
> - Added three patches that refactor null_blk loading, unloading and
>   configuration (Chaitanya).
> 
> Bart Van Assche (4):
>   Make _exit_null_blk remove all null_blk device instances
>   Use _{init,exit}_null_blk instead of open-coding these functions
>   Introduce the function _configure_null_blk()
>   Add a test that triggers the blk_mq_realloc_hw_ctxs() error path
> 
>  common/multipath-over-rdma | 29 ++++++---------------
>  common/null_blk            | 21 ++++++++++++++-
>  tests/block/022            |  3 ---
>  tests/block/029            | 17 ++-----------
>  tests/block/030            | 52 ++++++++++++++++++++++++++++++++++++++
>  tests/block/030.out        |  1 +
>  tests/nvmeof-mp/rc         |  2 +-
>  7 files changed, 84 insertions(+), 41 deletions(-)
>  create mode 100755 tests/block/030
>  create mode 100644 tests/block/030.out

Merged, thanks, Bart!