diff mbox series

[blktests] throtl: set "io" to subtree_control only if required

Message ID 20241115121224.173285-1-shinichiro.kawasaki@wdc.com (mailing list archive)
State New
Headers show
Series [blktests] throtl: set "io" to subtree_control only if required | expand

Commit Message

Shinichiro Kawasaki Nov. 15, 2024, 12:12 p.m. UTC 
It was reported the thortl test cases fail on the systems, which already
sets "io" in cgourp2 subtree_control files. The fail happens when
writing "-io" to the subtree_control files at clean up.

To avoid the failure, check if the system already sets "io". If so, skip
writing "+io" at set up, and writing "-io" at clean up.

Reported-by: Yi Zhang <yi.zhang@redhat.com>
Link: https://github.com/osandov/blktests/issues/149
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 tests/throtl/rc | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Comments

Yu Kuai Nov. 18, 2024, 11:08 a.m. UTC | #1 
在 2024/11/15 20:12, Shin'ichiro Kawasaki 写道:
> It was reported the thortl test cases fail on the systems, which already
> sets "io" in cgourp2 subtree_control files. The fail happens when
> writing "-io" to the subtree_control files at clean up.
> 
> To avoid the failure, check if the system already sets "io". If so, skip
> writing "+io" at set up, and writing "-io" at clean up.
> 
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Link: https://github.com/osandov/blktests/issues/149
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>   tests/throtl/rc | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
> 

LGTM, thanks for the patch
Reviewed-by: Yu Kuai <yukuai3@huawei.com>

> diff --git a/tests/throtl/rc b/tests/throtl/rc
> index 2e26fd2..9c264bd 100644
> --- a/tests/throtl/rc
> +++ b/tests/throtl/rc
> @@ -10,6 +10,8 @@
>   
>   THROTL_DIR=$(echo "$TEST_NAME" | tr '/' '_')
>   THROTL_DEV=dev_nullb
> +declare THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO
> +declare THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO
>   
>   group_requires() {
>   	_have_root
> @@ -31,8 +33,16 @@ _set_up_throtl() {
>   		return 1
>   	fi
>   
> -	echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> -	echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"
> +	THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO=
> +	THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO=
> +	if ! grep -q io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
> +		echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> +		THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO=true
> +	fi
> +	if ! grep -q io "$CGROUP2_DIR/cgroup.subtree_control"; then
> +		echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"
> +		THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO=true
> +	fi
>   
>   	mkdir -p "$CGROUP2_DIR/$THROTL_DIR"
>   	return 0;
> @@ -40,8 +50,12 @@ _set_up_throtl() {
>   
>   _clean_up_throtl() {
>   	rmdir "$CGROUP2_DIR/$THROTL_DIR"
> -	echo "-io" > "$CGROUP2_DIR/cgroup.subtree_control"
> -	echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> +	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_null_blk
>
Yi Zhang Nov. 19, 2024, 12:29 a.m. UTC | #2 
Thanks for the quick fix.
Tested-by: Yi Zhang <yi.zhang@redhat.com>

On Fri, Nov 15, 2024 at 8:12 PM Shin'ichiro Kawasaki
<shinichiro.kawasaki@wdc.com> wrote:
>
> It was reported the thortl test cases fail on the systems, which already
> sets "io" in cgourp2 subtree_control files. The fail happens when
> writing "-io" to the subtree_control files at clean up.
>
> To avoid the failure, check if the system already sets "io". If so, skip
> writing "+io" at set up, and writing "-io" at clean up.
>
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Link: https://github.com/osandov/blktests/issues/149
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> ---
>  tests/throtl/rc | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/tests/throtl/rc b/tests/throtl/rc
> index 2e26fd2..9c264bd 100644
> --- a/tests/throtl/rc
> +++ b/tests/throtl/rc
> @@ -10,6 +10,8 @@
>
>  THROTL_DIR=$(echo "$TEST_NAME" | tr '/' '_')
>  THROTL_DEV=dev_nullb
> +declare THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO
> +declare THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO
>
>  group_requires() {
>         _have_root
> @@ -31,8 +33,16 @@ _set_up_throtl() {
>                 return 1
>         fi
>
> -       echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> -       echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"
> +       THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO=
> +       THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO=
> +       if ! grep -q io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
> +               echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> +               THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO=true
> +       fi
> +       if ! grep -q io "$CGROUP2_DIR/cgroup.subtree_control"; then
> +               echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"
> +               THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO=true
> +       fi
>
>         mkdir -p "$CGROUP2_DIR/$THROTL_DIR"
>         return 0;
> @@ -40,8 +50,12 @@ _set_up_throtl() {
>
>  _clean_up_throtl() {
>         rmdir "$CGROUP2_DIR/$THROTL_DIR"
> -       echo "-io" > "$CGROUP2_DIR/cgroup.subtree_control"
> -       echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
> +       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_null_blk
> --
> 2.47.0
>


--
Best Regards,
  Yi Zhang
Shinichiro Kawasaki Nov. 19, 2024, 1:51 p.m. UTC | #3 
On Nov 15, 2024 / 21:12, Shin'ichiro Kawasaki wrote:
> It was reported the thortl test cases fail on the systems, which already
> sets "io" in cgourp2 subtree_control files. The fail happens when
> writing "-io" to the subtree_control files at clean up.
> 
> To avoid the failure, check if the system already sets "io". If so, skip
> writing "+io" at set up, and writing "-io" at clean up.

Thanks for the review and the test. I applied this patch.
diff mbox series

Patch

diff --git a/tests/throtl/rc b/tests/throtl/rc
index 2e26fd2..9c264bd 100644
--- a/tests/throtl/rc
+++ b/tests/throtl/rc
@@ -10,6 +10,8 @@ 
 
 THROTL_DIR=$(echo "$TEST_NAME" | tr '/' '_')
 THROTL_DEV=dev_nullb
+declare THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO
+declare THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO
 
 group_requires() {
 	_have_root
@@ -31,8 +33,16 @@  _set_up_throtl() {
 		return 1
 	fi
 
-	echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
-	echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"
+	THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO=
+	THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO=
+	if ! grep -q io "$(_cgroup2_base_dir)/cgroup.subtree_control"; then
+		echo "+io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
+		THROTL_CLEAR_BASE_SUBTREE_CONTROL_IO=true
+	fi
+	if ! grep -q io "$CGROUP2_DIR/cgroup.subtree_control"; then
+		echo "+io" > "$CGROUP2_DIR/cgroup.subtree_control"
+		THROTL_CLEAR_CGROUP2_DIR_CONTROL_IO=true
+	fi
 
 	mkdir -p "$CGROUP2_DIR/$THROTL_DIR"
 	return 0;
@@ -40,8 +50,12 @@  _set_up_throtl() {
 
 _clean_up_throtl() {
 	rmdir "$CGROUP2_DIR/$THROTL_DIR"
-	echo "-io" > "$CGROUP2_DIR/cgroup.subtree_control"
-	echo "-io" > "$(_cgroup2_base_dir)/cgroup.subtree_control"
+	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_null_blk