Re: [PATCH blktests 2/2] Add a test that triggers blk_mq_update_nr_hw_queues()

[Omar Sandoval <osandov@osandov.com>]

On Mon, Oct 21, 2019 at 03:57:19PM -0700, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  tests/block/029     | 63 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/block/029.out |  1 +
>  2 files changed, 64 insertions(+)
>  create mode 100755 tests/block/029
>  create mode 100644 tests/block/029.out
> 
> diff --git a/tests/block/029 b/tests/block/029
> new file mode 100755
> index 000000000000..1999168603c1
> --- /dev/null
> +++ b/tests/block/029
> @@ -0,0 +1,63 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2019 Google Inc.
> +#
> +# Trigger blk_mq_update_nr_hw_queues().
> +
> +. tests/block/rc
> +. common/null_blk
> +
> +DESCRIPTION="trigger blk_mq_update_nr_hw_queues()"
> +QUICK=1
> +
> +requires() {
> +	_have_null_blk
> +}
> +
> +# Configure one null_blk instance.
> +configure_null_blk() {
> +	(
> +		cd /sys/kernel/config/nullb || return $?
> +		(
> +			mkdir -p nullb0 &&
> +				cd nullb0 &&
> +				echo 0 > completion_nsec &&
> +				echo 512 > blocksize &&
> +				echo 16 > size &&
> +				echo 1 > memory_backed &&
> +				echo 1 > power
> +		)
> +	) &&
> +	ls -l /dev/nullb* &>>"$FULL"

What's the point of these nested subshells? Can't this just be:

configure_null_blk() {
	cd /sys/kernel/config/nullb &&
	mkdir -p nullb0 &&
	cd nullb0 &&
	echo 0 > completion_nsec &&
	echo 512 > blocksize &&
	echo 16 > size &&
	echo 1 > memory_backed &&
	echo 1 > power &&
	ls -l /dev/nullb* &>>"$FULL"
}

> +}
> +
> +modify_nr_hw_queues() {
> +	local deadline num_cpus
> +
> +	deadline=$(($(_uptime_s) + TIMEOUT))
> +	num_cpus=$(find /sys/devices/system/cpu -maxdepth 1 -name 'cpu[0-9]*' |
> +			   wc -l)

Please just use nproc. Or even better, can you just read the original
value of /sys/kernel/config/nullb/nullb0/submit_queues? Or does that
start at 1?

> +	while [ "$(_uptime_s)" -lt "$deadline" ]; do
> +		sleep .1
> +		echo 1 > /sys/kernel/config/nullb/nullb0/submit_queues
> +		sleep .1
> +		echo "$num_cpus" > /sys/kernel/config/nullb/nullb0/submit_queues
> +	done
> +}
> +
> +test() {
> +	: "${TIMEOUT:=30}"
> +	_init_null_blk nr_devices=0 queue_mode=2 &&
> +	configure_null_blk
> +	modify_nr_hw_queues &
> +	fio --rw=randwrite --bs=4K --loops=$((10**6)) \
> +		--iodepth=64 --group_reporting --sync=1 --direct=1 \
> +		--ioengine=libaio --filename="/dev/nullb0" \
> +		--runtime="${TIMEOUT}" --name=nullb0 \
> +		--output="${RESULTS_DIR}/block/fio-output-029.txt" \
> +		>>"$FULL"
> +	wait
> +	rmdir /sys/kernel/config/nullb/nullb0
> +	_exit_null_blk
> +	echo Passed
> +}
> diff --git a/tests/block/029.out b/tests/block/029.out
> new file mode 100644
> index 000000000000..863339fb8ced
> --- /dev/null
> +++ b/tests/block/029.out
> @@ -0,0 +1 @@
> +Passed
> -- 
> 2.23.0.866.gb869b98d4c-goog
>