Re: [PATCH blktests v5 2/3] bcache: add bcache/002

[Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>]

On Mar 05, 2026 / 16:01, Daniel Wagner wrote:
> Add test case from Stephen Zhang [1][2], which is
> 
> 1. Wait for fio to complete (wait $fio_pid)
> 2. Wait a few seconds for I/O to drain
> 3. Then check the utilization
> 
> If utilization is still > 0% after I/O completes, then there's a real
> accounting leak.
> 
> [1] https://lore.kernel.org/linux-bcache/CANubcdX7eNbH_bo4-f94DUbdiEbt04Vxy1MPyhm+CZyXB01FuQ@mail.gmail.com/
> [2] https://lore.kernel.org/all/CANubcdU92Uv7wNdZH357LLTnfwPouekXNTPSjHnzYzK99-eZ9w@mail.gmail.com

Thanks for the updates. It's good that we do not need iostat :)

I ran this test case with v7.0-rc2 kernel, and it passes. Good.

I also ran this test case with older kernel without the kernel fix [1]. I
expected failure, but it passed. I ran the Stephen's original test script and it
did not fail either. So I guess my test environment does not fulfill the
conditions to recreate the failure. Daniel, did you observe this test case
failed with any older kernel?

Let me leave nit comments in line. If this patch does not respin, I can fold in
the changes for the nit comments.

> 
> Signed-off-by: Daniel Wagner <dwagner@suse.de>
> ---
>  tests/bcache/002     | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/bcache/002.out |  2 ++
>  2 files changed, 80 insertions(+)
> 
> diff --git a/tests/bcache/002 b/tests/bcache/002
> new file mode 100755
> index 000000000000..a50722ff9e0e
> --- /dev/null
> +++ b/tests/bcache/002
> @@ -0,0 +1,78 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2026 Daniel Wagner, SUSE Labs
> +#
> +# Test based on Stephen Zhang <starzhangzsd@gmail.com> test case
> +# https://lore.kernel.org/linux-bcache/CANubcdX7eNbH_bo4-f94DUbdiEbt04Vxy1MPyhm+CZyXB01FuQ@mail.gmail.com/#t
> +#
> +# Test bcache for bio leaks in clone
> +
> +. tests/bcache/rc
> +
> +DESCRIPTION="test bcache for bio leaks in clone"
> +
> +requires() {
> +	_have_fio

I forgot to mention that we need,

       _have_program bc

here. Let me fold-in this change.

> +}
> +
> +read_ticks() {
> +	local bdev_name="$1"
> +	local stat_file
> +
> +	stat_file="/sys/block/${bdev_name}/stat"
> +	awk '{print $10}' "$stat_file"
> +}
> +
> +get_utilization() {
> +	local bdev_name="$1"
> +	local t1 t2
> +
> +	t1="$(read_ticks ${bdev_name})"

Nit: for shellcheck, the line above should be,

	t1="$(read_ticks "${bdev_name}")"

> +	sleep 1
> +	t2="$(read_ticks ${bdev_name})"

Same here:

	t2="$(read_ticks "${bdev_name}")"

> +
> +	# (t2 - t1) is the ms busy. Over 1000ms, (delta/10) gives percentage.
> +	echo "scale=2; ($t2 - $t1) / 10" | bc
> +}
> +
> +test_device_array() {
> +	echo "Running ${TEST_NAME}"
> +
> +	if [[ ${#TEST_DEV_ARRAY[@]} -lt 2 ]]; then
> +		SKIP_REASONS+=("requires at least 2 devices")
> +		return 1
> +	fi
> +
> +	_setup_bcache "${TEST_DEV_ARRAY[@]}"
> +
> +	local -a bcache_nodes
> +	local bcache_dev bdev_name fio_pid state

Nit: for shellcheck, unused 'fio_pid' should be dropped.

	local bcache_dev bdev_name state

> +
> +	mapfile -t bcache_nodes < <(_create_bcache \
> +				--cache "${TEST_DEV_ARRAY[0]##*/}" \
> +				--bdev "${TEST_DEV_ARRAY[1]##*/}" \
> +				--writeback)
> +
> +	bcache_dev="${bcache_nodes[0]}"
> +	bdev_name="$(basename "${bcache_dev}")"
> +	echo 1 > /sys/block/"${bdev_name}"/bcache/detach
> +
> +	state="$(cat /sys/block/"${bdev_name}"/bcache/state)"
> +	echo "Device state: ${state}"
> +
> +	_run_fio_rand_io --filename="${bcache_dev}" --time_base \
> +			--runtime=10 >> "$FULL" 2>&1 
> +	

Nit: a stray tab and a space in the above two lines.