Linux block layer
 help / color / mirror / Atom feed
From: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: Damien Le Moal <dlemoal@kernel.org>, linux-block@vger.kernel.org
Subject: Re: [PATCH blktests] throtl/008: Add a test for the iocost cgroup controller
Date: Thu, 4 Jun 2026 15:03:00 +0900	[thread overview]
Message-ID: <aiESTpWx0-0l9Ppj@shinmob> (raw)
In-Reply-To: <20260603205007.2654971-1-bvanassche@acm.org>

[-- Attachment #1: Type: text/plain, Size: 3597 bytes --]

On Jun 03, 2026 / 13:50, Bart Van Assche wrote:
> Add a test for read and write IOPS throttling. The test implementation
> has been generated by Antigravity and Gemini with a few manual edits.

Hi Bart, thanks for the patch. I think it's nice to add this test case to
cover the iocost controller code paths. It might be the better to note
'the iocost controller' in the commit message also. It will be consistent
with the DESCRIPTION in the test case.

> 
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  tests/throtl/008     | 152 +++++++++++++++++++++++++++++++++++++++++++
>  tests/throtl/008.out |   2 +
>  2 files changed, 154 insertions(+)
>  create mode 100755 tests/throtl/008
>  create mode 100644 tests/throtl/008.out
> 
> diff --git a/tests/throtl/008 b/tests/throtl/008
> new file mode 100755
> index 000000000000..0570fc0188e0
> --- /dev/null
> +++ b/tests/throtl/008
> @@ -0,0 +1,152 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2026 Google LLC
> +#
> +# Test cgroup iocost IOPS limiting.
> +
> +. tests/block/rc

I guess this should be tests/throtl/rc. With this, the two lines below will not
be required.

> +. common/null_blk
> +. common/cgroup
> +. common/fio
> +
> +DESCRIPTION="test cgroup iocost controller limits"
> +
> +requires() {
> +	_have_cgroup2_controller io
> +	_have_null_blk
> +	_have_fio
> +	_have_program bc

group_require() in tsts/throtl/rc covers the 3 lines out of the 4 lines above.

> +	if [[ ! -e "$(_cgroup2_base_dir)/io.cost.qos" ]]; then
> +		SKIP_REASONS+=("iocost controller not supported (CONFIG_BLK_CGROUP_IOCOST)")
> +		return 1
> +	fi
> +}
> +
> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	# Create a null_blk instance
> +	local null_blk_params=(
> +		blocksize=4096
> +		completion_nsec=0
> +		memory_backed=0
> +		size=1024 # MB
> +		submit_queues=1
> +		power=1
> +	)
> +	_init_null_blk nr_devices=0 queue_mode=2 &&
> +	if ! _configure_null_blk nullb0 "${null_blk_params[@]}"; then
> +		echo "Configuring null_blk failed"
> +		return 1
> +	fi
> +
> +	local dev_t
> +	dev_t=$(</sys/block/nullb0/dev)
> +	if [[ -z $dev_t ]]; then
> +		echo "Failed to get major:minor for nullb0"
> +		_exit_null_blk
> +		return 1
> +	fi
> +
> +	# Initialize cgroups
> +	if ! _init_cgroup2; then
> +		echo "Initializing cgroup2 failed"
> +		_exit_null_blk
> +		return 1
> +	fi
> +
> +	# Enable io controller in the subtree
> +	local deactivate_io_ctrlr=false
> +	if ! grep -wq io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
> +		if ! echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
> +			echo "Failed to enable io controller on cgroup root"
> +			_exit_cgroup2
> +			_exit_null_blk
> +			return 1
> +		fi
> +		deactivate_io_ctrlr=true
> +	fi
> +
> +	if ! echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"; then
> +		echo "Failed to enable io controller on $CGROUP2_DIR"
> +		if [[ $deactivate_io_ctrlr == true ]]; then
> +			echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> +		fi
> +		_exit_cgroup2
> +		_exit_null_blk
> +		return 1
> +	fi

The initialization part above has duplications with _set_up_throtl() in
tests/throtl/rc. Is it considered to use _set_up_throtl()? I guess it will
simplify this test case, and make it consisten with other test cases in the
throtl group. It will also allow to run the test case for scsi_debug.

I created a trial patch that applis on top of this patch, which uses
_set_up_throtl(). It is attached to this mail for your reference. It drops
some null_blk options. As far as I did trial runs, it does not look affecting
the run results.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 4582 bytes --]

diff --git a/tests/throtl/008 b/tests/throtl/008
index 0570fc0..b1bcf84 100755
--- a/tests/throtl/008
+++ b/tests/throtl/008
@@ -4,76 +4,36 @@
 #
 # Test cgroup iocost IOPS limiting.
 
-. tests/block/rc
-. common/null_blk
-. common/cgroup
+. tests/throtl/rc
 . common/fio
 
 DESCRIPTION="test cgroup iocost controller limits"
 
 requires() {
-	_have_cgroup2_controller io
-	_have_null_blk
 	_have_fio
-	_have_program bc
 	if [[ ! -e "$(_cgroup2_base_dir)/io.cost.qos" ]]; then
 		SKIP_REASONS+=("iocost controller not supported (CONFIG_BLK_CGROUP_IOCOST)")
 		return 1
 	fi
 }
 
+set_conditions() {
+	_set_throtl_blkdev_type "$@"
+}
+
 test() {
 	echo "Running ${TEST_NAME}"
 
-	# Create a null_blk instance
-	local null_blk_params=(
-		blocksize=4096
-		completion_nsec=0
-		memory_backed=0
-		size=1024 # MB
-		submit_queues=1
-		power=1
-	)
-	_init_null_blk nr_devices=0 queue_mode=2 &&
-	if ! _configure_null_blk nullb0 "${null_blk_params[@]}"; then
-		echo "Configuring null_blk failed"
+	# Set up throtl device and cgroup
+	if ! _set_up_throtl; then
 		return 1
 	fi
 
 	local dev_t
-	dev_t=$(</sys/block/nullb0/dev)
+	dev_t=$(</sys/block/"$THROTL_DEV"/dev)
 	if [[ -z $dev_t ]]; then
-		echo "Failed to get major:minor for nullb0"
-		_exit_null_blk
-		return 1
-	fi
-
-	# Initialize cgroups
-	if ! _init_cgroup2; then
-		echo "Initializing cgroup2 failed"
-		_exit_null_blk
-		return 1
-	fi
-
-	# Enable io controller in the subtree
-	local deactivate_io_ctrlr=false
-	if ! grep -wq io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
-		if ! echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
-			echo "Failed to enable io controller on cgroup root"
-			_exit_cgroup2
-			_exit_null_blk
-			return 1
-		fi
-		deactivate_io_ctrlr=true
-	fi
-
-	if ! echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"; then
-		echo "Failed to enable io controller on $CGROUP2_DIR"
-		if [[ $deactivate_io_ctrlr == true ]]; then
-			echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
-		fi
-		_exit_cgroup2
-		_exit_null_blk
+		echo "Failed to get major:minor for $THROTL_DEV"
+		_clean_up_throtl
 		return 1
 	fi
 
@@ -81,11 +41,7 @@ test() {
 	# min=100.00 max=100.00 forces vrate to be fixed at 100%
 	if ! echo "$dev_t enable=1 min=100.00 max=100.00" > "$(_cgroup2_base_dir)/io.cost.qos"; then
 		echo "Failed to configure io.cost.qos"
-		if [[ $deactivate_io_ctrlr == true ]]; then
-			echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
-		fi
-		_exit_cgroup2
-		_exit_null_blk
+		_clean_up_throtl
 		return 1
 	fi
 
@@ -94,25 +50,21 @@ test() {
 	if ! echo "$dev_t ctrl=user model=linear rbps=0 rseqiops=100 rrandiops=100 wbps=0 wseqiops=10 wrandiops=10" > "$(_cgroup2_base_dir)/io.cost.model"; then
 		echo "Failed to configure io.cost.model"
 		echo "$dev_t enable=0" > "$(_cgroup2_base_dir)/io.cost.qos"
-		if [[ $deactivate_io_ctrlr == true ]]; then
-			echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
-		fi
-		_exit_cgroup2
-		_exit_null_blk
+		_clean_up_throtl
 		return 1
 	fi
 
 	# Create a child cgroup for test
 	local cg_name="testgrp"
-	local cg_path="$CGROUP2_DIR/$cg_name"
+	local cg_path="$CGROUP2_DIR/$THROTL_DIR/$cg_name"
 	mkdir "$cg_path"
 
 	# 1. Run FIO read test
 	local -a FIO_PERF_FIELDS
 	FIO_PERF_FIELDS=("read iops")
-	if ! _fio_perf --bs=4k --rw=randread --name=read-test --filename=/dev/nullb0 \
+	if ! _fio_perf --bs=4k --rw=randread --name=read-test --filename=/dev/"$THROTL_DEV" \
 			--ioengine=libaio --direct=1 --iodepth=64 --time_based --runtime=10 \
-			--cgroup="blktests/$cg_name" > /dev/null 2>&1; then
+			--cgroup="blktests/$THROTL_DIR/$cg_name" > /dev/null 2>&1; then
 		echo "FIO read test failed"
 	else
 		local read_iops=${TEST_RUN["read iops"]}
@@ -125,9 +77,9 @@ test() {
 
 	# 2. Run FIO write test
 	FIO_PERF_FIELDS=("write iops")
-	if ! _fio_perf --bs=4k --rw=randwrite --name=write-test --filename=/dev/nullb0 \
+	if ! _fio_perf --bs=4k --rw=randwrite --name=write-test --filename=/dev/"$THROTL_DEV" \
 			--ioengine=libaio --direct=1 --iodepth=64 --time_based --runtime=10 \
-			--cgroup="blktests/$cg_name" > /dev/null 2>&1; then
+			--cgroup="blktests/$THROTL_DIR/$cg_name" > /dev/null 2>&1; then
 		echo "FIO write test failed"
 	else
 		local write_iops=${TEST_RUN["write iops"]}
@@ -140,13 +92,7 @@ test() {
 
 	# Clean up cgroups
 	rmdir "$cg_path"
-	if [[ $deactivate_io_ctrlr == true ]]; then
-		{ echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"; } &> "${FULL}"
-	fi
-	_exit_cgroup2
-
-	# Clean up null_blk
-	_exit_null_blk
+	_clean_up_throtl
 
 	echo "Test complete"
 }

      reply	other threads:[~2026-06-04  6:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03 20:50 [PATCH blktests] throtl/008: Add a test for the iocost cgroup controller Bart Van Assche
2026-06-04  6:03 ` Shin'ichiro Kawasaki [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aiESTpWx0-0l9Ppj@shinmob \
    --to=shinichiro.kawasaki@wdc.com \
    --cc=bvanassche@acm.org \
    --cc=dlemoal@kernel.org \
    --cc=linux-block@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox