[PATCH blktests] tests/throtl: add a new test 009

[PATCH blktests] tests/throtl: add a new test 009

[Tao Cui]

From: Tao Cui <cuitao@kylinos.cn>

Test the boundary condition of removing a cgroup while it still has IO
queued in the blk-throttle queue.  The queued bios must be flushed through
without any blk-cgroup or blk-throttle warning.

The dmesg check matches any blk-cgroup / blk-throttle warning rather than a
single call site, so it is not tied to one symbol.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
See also the related blkcg "store blkcg in bio" RFC:
https://lore.kernel.org/r/20260804065313.2092022-1-yukuai@kernel.org

 tests/throtl/009     | 67 ++++++++++++++++++++++++++++++++++++++++++++
 tests/throtl/009.out |  2 ++
 2 files changed, 69 insertions(+)
 create mode 100755 tests/throtl/009
 create mode 100644 tests/throtl/009.out

diff --git a/tests/throtl/009 b/tests/throtl/009
new file mode 100755
index 0000000..1f4ed67
--- /dev/null
+++ b/tests/throtl/009
@@ -0,0 +1,67 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2026 Tao Cui <cuitao@kylinos.cn>
+#
+# Regression test for removing a cgroup while IO is still queued in the
+# blk-throttle queue.  The queued bios must be flushed without any
+# blk-cgroup or blk-throttle warning, and without leaking the blkg
+# reference they pin.
+
+. tests/throtl/rc
+
+THROTL_BLKDEV_TYPES="nullb"
+
+DESCRIPTION="queued throttled IO vs. cgroup removal must not warn"
+QUICK=1
+
+set_conditions() {
+	_set_throtl_blkdev_type "$@"
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	if ! _set_up_throtl; then
+		return 1
+	fi
+
+	# Throttle reads so the bio queues in the throttle queue.
+	_throtl_set_limits rbps=4096
+
+	(
+		echo "$BASHPID" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs"
+		exec dd if=/dev/"$THROTL_DEV" of=/dev/null bs=1M count=1 status=none
+	) &
+	local io_pid=$!
+	sleep 2
+
+	# Move the reader to the cgroup2 root so the throttle cgroup can be
+	# removed while its bio is still queued.  $CGROUP2_DIR has +io in its
+	# subtree_control and so cannot hold processes.
+	echo "$io_pid" > "$(_cgroup2_base_dir)/cgroup.procs"
+
+	# Remove the throttle cgroup while its bio is still queued.
+	rmdir "$CGROUP2_DIR/$THROTL_DIR"
+	sleep 3
+
+	kill "$io_pid" 2>/dev/null
+	wait "$io_pid" 2>/dev/null
+
+	# $THROTL_DIR is already gone; tear down the rest by hand.
+	if [[ $THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO == true ]]; then
+		echo "-io" > "$CGROUP2_DIR/cgroup.subtree_control"
+	fi
+	if [[ $THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO == true ]]; then
+		echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
+	fi
+	_exit_cgroup2
+	_exit_throtl_blkdev
+
+	if _dmesg_since_test_start | grep -qE "WARNING:.*(blk-cgroup|blk-throttle)"; then
+		echo "WARNING: blkcg/blk-throttle warning on cgroup removal with queued IO"
+		_dmesg_since_test_start | grep -A 12 -E "WARNING:.*(blk-cgroup|blk-throttle)" \
+			>> "$FULL"
+	fi
+
+	echo "Test complete"
+}
diff --git a/tests/throtl/009.out b/tests/throtl/009.out
new file mode 100644
index 0000000..be675dd
--- /dev/null
+++ b/tests/throtl/009.out
@@ -0,0 +1,2 @@
+Running throtl/009
+Test complete
-- 
2.43.0

Re: [PATCH blktests] tests/throtl: add a new test 009

[Shin'ichiro Kawasaki]

On Aug 13, 2026 / 22:40, Tao Cui wrote:
> From: Tao Cui <cuitao@kylinos.cn>
> 
> Test the boundary condition of removing a cgroup while it still has IO
> queued in the blk-throttle queue.  The queued bios must be flushed through
> without any blk-cgroup or blk-throttle warning.
> 
> The dmesg check matches any blk-cgroup / blk-throttle warning rather than a
> single call site, so it is not tied to one symbol.

Tao, thanks for the patch. Does this test case have corresponding kernel side
fix? I'm guessing that your recent kernel patch is the fix. If so, I suggest
to mention the kernel commit in here and the test case header comment below.

> 
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> ---
> See also the related blkcg "store blkcg in bio" RFC:
> https://lore.kernel.org/r/20260804065313.2092022-1-yukuai@kernel.org
> 
>  tests/throtl/009     | 67 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/throtl/009.out |  2 ++
>  2 files changed, 69 insertions(+)
>  create mode 100755 tests/throtl/009
>  create mode 100644 tests/throtl/009.out
> 
> diff --git a/tests/throtl/009 b/tests/throtl/009
> new file mode 100755
> index 0000000..1f4ed67
> --- /dev/null
> +++ b/tests/throtl/009
> @@ -0,0 +1,67 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2026 Tao Cui <cuitao@kylinos.cn>
> +#
> +# Regression test for removing a cgroup while IO is still queued in the
> +# blk-throttle queue.  The queued bios must be flushed without any
> +# blk-cgroup or blk-throttle warning, and without leaking the blkg
> +# reference they pin.
> +
> +. tests/throtl/rc
> +
> +THROTL_BLKDEV_TYPES="nullb"

Is there any reason to run this test case only for nullb? If not, I suggest to
remove the line above to run it for both nullb and scsi_debug.

Re: [PATCH blktests] tests/throtl: add a new test 009

[Tao Cui]

Hi Shin'ichiro,

在 2026/8/18 20:42, Shin'ichiro Kawasaki 写道:
> On Aug 13, 2026 / 22:40, Tao Cui wrote:
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> Test the boundary condition of removing a cgroup while it still has IO
>> queued in the blk-throttle queue.  The queued bios must be flushed through
>> without any blk-cgroup or blk-throttle warning.
>>
>> The dmesg check matches any blk-cgroup / blk-throttle warning rather than a
>> single call site, so it is not tied to one symbol.
> 
> Tao, thanks for the patch. Does this test case have corresponding kernel side
> fix? I'm guessing that your recent kernel patch is the fix. If so, I suggest
> to mention the kernel commit in here and the test case header comment below.
> 


Thanks for the review.

No.  Christoph asked me to add the reproducer to blktests in that RFC
thread [1], that's how this patch came about.

The WARN it catches came from yukuai's "store blkcg in bio" RFC v1, which
is not merged: bio_pinned_blkg() re-looked up a blkg that blkg_destroy()
had already removed from the radix tree, while a queued throttled bio
still pinned it.  RFC v2 fixed it with blkg_lookup_any(), and mainline
passes this test too, since the blkg stays findable until queued bios
drain.

>>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>> See also the related blkcg "store blkcg in bio" RFC:
>> https://lore.kernel.org/r/20260804065313.2092022-1-yukuai@kernel.org
>>
>>  tests/throtl/009     | 67 ++++++++++++++++++++++++++++++++++++++++++++
>>  tests/throtl/009.out |  2 ++
>>  2 files changed, 69 insertions(+)
>>  create mode 100755 tests/throtl/009
>>  create mode 100644 tests/throtl/009.out
>>
>> diff --git a/tests/throtl/009 b/tests/throtl/009
>> new file mode 100755
>> index 0000000..1f4ed67
>> --- /dev/null
>> +++ b/tests/throtl/009
>> @@ -0,0 +1,67 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2026 Tao Cui <cuitao@kylinos.cn>
>> +#
>> +# Regression test for removing a cgroup while IO is still queued in the
>> +# blk-throttle queue.  The queued bios must be flushed without any
>> +# blk-cgroup or blk-throttle warning, and without leaking the blkg
>> +# reference they pin.
>> +
>> +. tests/throtl/rc
>> +
>> +THROTL_BLKDEV_TYPES="nullb"
> 
> Is there any reason to run this test case only for nullb? If not, I suggest to
> remove the line above to run it for both nullb and scsi_debug.

> Is there any reason to run this test case only for nullb?

The test just exercises the blk-throttle/blk-cgroup path on cgroup
removal and doesn't depend on the device type, nullb is simply the most
convenient.  Dropping THROTL_BLKDEV_TYPES is fine with me if you want
scsi_debug covered too.

[1] https://lore.kernel.org/r/20260804065313.2092022-1-yukuai@kernel.org

Thanks,
Tao

Re: [PATCH blktests] tests/throtl: add a new test 009

[Shin'ichiro Kawasaki]

On Aug 20, 2026 / 16:43, Tao Cui wrote:
> 
> Hi Shin'ichiro,
> 
> 在 2026/8/18 20:42, Shin'ichiro Kawasaki 写道:
> > On Aug 13, 2026 / 22:40, Tao Cui wrote:
> >> From: Tao Cui <cuitao@kylinos.cn>
> >>
> >> Test the boundary condition of removing a cgroup while it still has IO
> >> queued in the blk-throttle queue.  The queued bios must be flushed through
> >> without any blk-cgroup or blk-throttle warning.
> >>
> >> The dmesg check matches any blk-cgroup / blk-throttle warning rather than a
> >> single call site, so it is not tied to one symbol.
> > 
> > Tao, thanks for the patch. Does this test case have corresponding kernel side
> > fix? I'm guessing that your recent kernel patch is the fix. If so, I suggest
> > to mention the kernel commit in here and the test case header comment below.
> > 
> 
> 
> Thanks for the review.
> 
> No.  Christoph asked me to add the reproducer to blktests in that RFC
> thread [1], that's how this patch came about.
> 
> The WARN it catches came from yukuai's "store blkcg in bio" RFC v1, which
> is not merged: bio_pinned_blkg() re-looked up a blkg that blkg_destroy()
> had already removed from the radix tree, while a queued throttled bio
> still pinned it.  RFC v2 fixed it with blkg_lookup_any(), and mainline
> passes this test too, since the blkg stays findable until queued bios
> drain.

I see, thanks for the clarification. I suggest to add a "Link:" tag to this
patch's commit message, to refer to the background discussion [*].

[*] https://lore.kernel.org/all/20260804133208.GB8078@lst.de/

> 
> >>
> >> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
> >> ---
> >> See also the related blkcg "store blkcg in bio" RFC:
> >> https://lore.kernel.org/r/20260804065313.2092022-1-yukuai@kernel.org
> >>
> >>  tests/throtl/009     | 67 ++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/throtl/009.out |  2 ++
> >>  2 files changed, 69 insertions(+)
> >>  create mode 100755 tests/throtl/009
> >>  create mode 100644 tests/throtl/009.out
> >>
> >> diff --git a/tests/throtl/009 b/tests/throtl/009
> >> new file mode 100755
> >> index 0000000..1f4ed67
> >> --- /dev/null
> >> +++ b/tests/throtl/009
> >> @@ -0,0 +1,67 @@
> >> +#!/bin/bash
> >> +# SPDX-License-Identifier: GPL-3.0+
> >> +# Copyright (C) 2026 Tao Cui <cuitao@kylinos.cn>
> >> +#
> >> +# Regression test for removing a cgroup while IO is still queued in the
> >> +# blk-throttle queue.  The queued bios must be flushed without any
> >> +# blk-cgroup or blk-throttle warning, and without leaking the blkg
> >> +# reference they pin.
> >> +
> >> +. tests/throtl/rc
> >> +
> >> +THROTL_BLKDEV_TYPES="nullb"
> > 
> > Is there any reason to run this test case only for nullb? If not, I suggest to
> > remove the line above to run it for both nullb and scsi_debug.
> 
> > Is there any reason to run this test case only for nullb?
> 
> The test just exercises the blk-throttle/blk-cgroup path on cgroup
> removal and doesn't depend on the device type, nullb is simply the most
> convenient.  Dropping THROTL_BLKDEV_TYPES is fine with me if you want
> scsi_debug covered too.

I see, thanks. The other tests cases in throtl group run for both nullb and
scsi_debug, so I suggest to do the same for this test case also for consistency.

Once there was a failure in throtl group that was observed only with scsi_debug.
IIRC, the test case does not depend on the device type, but the failure was
timing dependent and observed only with scsi_debug. Then, I think it is useful
to run the throtl test cases for both null_blk and scsi_debug in general.