tests/throtl: add a new test 009
PATCH 1/1 tests/throtl: add a new test 009
From: Tao Cui <[email protected]>
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.
---
See also the related blkcg "store blkcg in bio" RFC:
https://lore.kernel.org/r/[email protected]
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
tests/throtl/009 new file +67 -0
@@ -0,0 +1,67 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2026 Tao Cui <[email protected]> +# +# 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" +}
tests/throtl/009.out new file +2 -0
@@ -0,0 +1,2 @@ +Running throtl/009 +Test complete
Signature · 1 line
2.43.0
Quoted 8 lines · Tao Cui
From: Tao Cui <[email protected]>
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.
Quoted 20 lines
Signed-off-by Tao Cui <[email protected]>---
See also the related blkcg "store blkcg in bio" RFC:
https://lore.kernel.org/r/[email protected]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.outtests/throtl/009 new file +2
-0@@ -0,0 +1,67 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+
tests/throtl/009 line 12
Quoted 10 lines
tests/throtl/009 new file +10
-0+# Copyright (C) 2026 Tao Cui <[email protected]> +# +# 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.
Hi Shin'ichiro,
Quoted 7 lines · Shin'ichiro Kawasaki
Quoted 6 lines · Tao Cui
From: Tao Cui <[email protected]>
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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 34 lines
Quoted 20 lines
Signed-off-by Tao Cui <[email protected]>---
See also the related blkcg "store blkcg in bio" RFC:
https://lore.kernel.org/r/[email protected]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.outtests/throtl/009 new file +2
-0@@ -0,0 +1,67 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+tests/throtl/009 line 12
Quoted 10 lines
tests/throtl/009 new file +10
-0+# Copyright (C) 2026 Tao Cui <[email protected]> +# +# 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.
Quoted 1 line
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/[email protected]
Thanks,
Tao
Quoted 25 lines · Tao Cui
Hi Shin'ichiro,
Quoted 7 lines · Shin'ichiro Kawasaki
Quoted 6 lines · Tao Cui
From: Tao Cui <[email protected]>
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.
Quoted 7 lines
Quoted 2 lines
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.
Quoted 6 lines
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 thatblkg_destroy()had already removed from the radix tree, while a queued throttled bio still pinned it. RFC v2 fixed it withblkg_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/[email protected]/
Quoted 35 lines
Quoted 34 lines
Quoted 21 lines
Signed-off-by Tao Cui <[email protected]>---
See also the related blkcg "store blkcg in bio" RFC:
https://lore.kernel.org/r/[email protected]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.outtests/throtl/009 new file +2
-0@@ -0,0 +1,67 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+tests/throtl/009 line 12
Quoted 10 lines
tests/throtl/009 new file +10
-0+# Copyright (C) 2026 Tao Cui <[email protected]> +# +# 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.
Quoted 6 lines
Quoted 1 line
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_TYPESis fine with me if you wantscsi_debugcovered 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.