diff mbox series

[blktests,1/5] tests/throtl: add first test for blk-throttle

Message ID 20240416020042.509291-2-yukuai1@huaweicloud.com (mailing list archive)
State New
Headers show
Series add new tests for blk-throttle | expand

Commit Message

Yu Kuai April 16, 2024, 2 a.m. UTC
From: Yu Kuai <yukuai3@huawei.com>

Test basic functionality.

Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 tests/throtl/001     | 84 ++++++++++++++++++++++++++++++++++++++++++++
 tests/throtl/001.out |  6 ++++
 tests/throtl/rc      | 15 ++++++++
 3 files changed, 105 insertions(+)
 create mode 100755 tests/throtl/001
 create mode 100644 tests/throtl/001.out
 create mode 100644 tests/throtl/rc

Comments

Chaitanya Kulkarni April 16, 2024, 3:02 a.m. UTC | #1
On 4/15/24 19:00, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
>
> Test basic functionality.
>
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   tests/throtl/001     | 84 ++++++++++++++++++++++++++++++++++++++++++++
>   tests/throtl/001.out |  6 ++++
>   tests/throtl/rc      | 15 ++++++++
>   3 files changed, 105 insertions(+)
>   create mode 100755 tests/throtl/001
>   create mode 100644 tests/throtl/001.out
>   create mode 100644 tests/throtl/rc
>
> diff --git a/tests/throtl/001 b/tests/throtl/001
> new file mode 100755
> index 0000000..79ecf07
> --- /dev/null
> +++ b/tests/throtl/001
> @@ -0,0 +1,84 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Yu Kuai
> +#
> +# Test basic functionality of blk-throttle
> +
> +. tests/throtl/rc
> +
> +DESCRIPTION="basic functionality"
> +QUICK=1
> +
> +CG=/sys/fs/cgroup
> +TEST_DIR=$CG/blktests_throtl
> +devname=nullb0
> +dev=""
> +
> +set_up_test() {
> +	if ! _init_null_blk nr_devices=1; then
> +		return 1;
> +	fi
> +
> +	dev=$(cat /sys/block/$devname/dev)
> +	echo +io > $CG/cgroup.subtree_control
> +	mkdir $TEST_DIR
> +

move above to 3 lines to rc with helper instead of repeating the
code for every test ?

> +	return 0;
> +}
> +
> +clean_up_test() {
> +	rmdir $TEST_DIR
> +	echo -io > $CG/cgroup.subtree_control
> +	_exit_null_blk

same here ?

> +}
> +
> +config_throtl() {
> +	echo "$dev $*" > $TEST_DIR/io.max
> +}
> +
> +remove_config() {
> +	echo "$dev rbps=max wbps=max riops=max wiops=max" > $TEST_DIR/io.max
> +}
> +

same here for above two helper ?

> +test_io() {
> +	config_throtl "$1"
> +
> +	{
> +		sleep 0.1
> +		start_time=$(date +%s.%N)
> +
> +		if [ "$2" == "read" ]; then
> +			dd if=/dev/$devname of=/dev/null bs=4k count=256 iflag=direct status=none
> +		elif [ "$2" == "write" ]; then
> +			dd of=/dev/$devname if=/dev/zero bs=4k count=256 oflag=direct status=none
> +		fi

Is there a any specific reason to use dd and not fio ?

> +
> +		end_time=$(date +%s.%N)
> +		elapsed=$(echo "$end_time - $start_time" | bc)
> +		printf "%.0f\n" "$elapsed"
> +	} &
> +
> +	pid=$!
> +	echo $! > $TEST_DIR/cgroup.procs
> +	wait $pid
> +
> +	remove_config
> +}
> +

apparently test_io is also repeated can be moved to rc with right 
parameters ?

> +test() {
> +	echo "Running ${TEST_NAME}"
> +
> +	if ! set_up_test; then
> +		return 1;
> +	fi
> +
> +	_1MB=$((1024 * 1024))

starting variable name with _ seems a but weired, why not just pass
$((1024 *1024)) ?

> +
> +	test_io wbps=$_1MB write
> +	test_io wiops=256 write
> +	test_io rbps=$_1MB read
> +	test_io riops=256 read
> +
> +	clean_up_test
> +	echo "Test complete"
> +}
> diff --git a/tests/throtl/001.out b/tests/throtl/001.out
> new file mode 100644
> index 0000000..a3edfdd
> --- /dev/null
> +++ b/tests/throtl/001.out
> @@ -0,0 +1,6 @@
> +Running throtl/001
> +1
> +1
> +1
> +1
> +Test complete
> diff --git a/tests/throtl/rc b/tests/throtl/rc
> new file mode 100644
> index 0000000..8fa8b58
> --- /dev/null
> +++ b/tests/throtl/rc
> @@ -0,0 +1,15 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2024 Yu Kuai
> +#
> +# Tests for blk-throttle
> +
> +. common/rc
> +. common/null_blk
> +
> +group_requires() {
> +	_have_root
> +	_have_null_blk
> +	_have_kernel_option BLK_DEV_THROTTLING
> +	_have_cgroup2_controller io
> +}

apart from that thanks for the tests ..

-ck
Yu Kuai April 16, 2024, 3:17 a.m. UTC | #2
Hi,

在 2024/04/16 11:02, Chaitanya Kulkarni 写道:
> On 4/15/24 19:00, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> Test basic functionality.
>>
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> ---
>>    tests/throtl/001     | 84 ++++++++++++++++++++++++++++++++++++++++++++
>>    tests/throtl/001.out |  6 ++++
>>    tests/throtl/rc      | 15 ++++++++
>>    3 files changed, 105 insertions(+)
>>    create mode 100755 tests/throtl/001
>>    create mode 100644 tests/throtl/001.out
>>    create mode 100644 tests/throtl/rc
>>
>> diff --git a/tests/throtl/001 b/tests/throtl/001
>> new file mode 100755
>> index 0000000..79ecf07
>> --- /dev/null
>> +++ b/tests/throtl/001
>> @@ -0,0 +1,84 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Yu Kuai
>> +#
>> +# Test basic functionality of blk-throttle
>> +
>> +. tests/throtl/rc
>> +
>> +DESCRIPTION="basic functionality"
>> +QUICK=1
>> +
>> +CG=/sys/fs/cgroup
>> +TEST_DIR=$CG/blktests_throtl
>> +devname=nullb0
>> +dev=""
>> +
>> +set_up_test() {
>> +	if ! _init_null_blk nr_devices=1; then
>> +		return 1;
>> +	fi
>> +
>> +	dev=$(cat /sys/block/$devname/dev)
>> +	echo +io > $CG/cgroup.subtree_control
>> +	mkdir $TEST_DIR
>> +
> 
> move above to 3 lines to rc with helper instead of repeating the
> code for every test ?

Yes, that sounds good, just test 004 is different.
> 
>> +	return 0;
>> +}
>> +
>> +clean_up_test() {
>> +	rmdir $TEST_DIR
>> +	echo -io > $CG/cgroup.subtree_control
>> +	_exit_null_blk
> 
> same here ?
> 
>> +}
>> +
>> +config_throtl() {
>> +	echo "$dev $*" > $TEST_DIR/io.max
>> +}
>> +
>> +remove_config() {
>> +	echo "$dev rbps=max wbps=max riops=max wiops=max" > $TEST_DIR/io.max
>> +}
>> +
> 
> same here for above two helper ?

Yes, of course.
> 
>> +test_io() {
>> +	config_throtl "$1"
>> +
>> +	{
>> +		sleep 0.1
>> +		start_time=$(date +%s.%N)
>> +
>> +		if [ "$2" == "read" ]; then
>> +			dd if=/dev/$devname of=/dev/null bs=4k count=256 iflag=direct status=none
>> +		elif [ "$2" == "write" ]; then
>> +			dd of=/dev/$devname if=/dev/zero bs=4k count=256 oflag=direct status=none
>> +		fi
> 
> Is there a any specific reason to use dd and not fio ?

My thoughts is that I need to make sure the number and the size
of IO that I dispatched, so that I can predict the throttle time,
and we don't need to keep issuing new IO based on time here.
> 
>> +
>> +		end_time=$(date +%s.%N)
>> +		elapsed=$(echo "$end_time - $start_time" | bc)
>> +		printf "%.0f\n" "$elapsed"
>> +	} &
>> +
>> +	pid=$!
>> +	echo $! > $TEST_DIR/cgroup.procs
>> +	wait $pid
>> +
>> +	remove_config
>> +}
>> +
> 
> apparently test_io is also repeated can be moved to rc with right
> parameters ?

There is slight difference, however, the answer is apparently yes.
> 
>> +test() {
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	if ! set_up_test; then
>> +		return 1;
>> +	fi
>> +
>> +	_1MB=$((1024 * 1024))
> 
> starting variable name with _ seems a but weired, why not just pass
> $((1024 *1024)) ?

I'll use the name $bps_limit here, just think to prevent the same code
is better...

Thanks,
Kuai

> 
>> +
>> +	test_io wbps=$_1MB write
>> +	test_io wiops=256 write
>> +	test_io rbps=$_1MB read
>> +	test_io riops=256 read
>> +
>> +	clean_up_test
>> +	echo "Test complete"
>> +}
>> diff --git a/tests/throtl/001.out b/tests/throtl/001.out
>> new file mode 100644
>> index 0000000..a3edfdd
>> --- /dev/null
>> +++ b/tests/throtl/001.out
>> @@ -0,0 +1,6 @@
>> +Running throtl/001
>> +1
>> +1
>> +1
>> +1
>> +Test complete
>> diff --git a/tests/throtl/rc b/tests/throtl/rc
>> new file mode 100644
>> index 0000000..8fa8b58
>> --- /dev/null
>> +++ b/tests/throtl/rc
>> @@ -0,0 +1,15 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2024 Yu Kuai
>> +#
>> +# Tests for blk-throttle
>> +
>> +. common/rc
>> +. common/null_blk
>> +
>> +group_requires() {
>> +	_have_root
>> +	_have_null_blk
>> +	_have_kernel_option BLK_DEV_THROTTLING
>> +	_have_cgroup2_controller io
>> +}
> 
> apart from that thanks for the tests ..
> 
> -ck
> 
>
Chaitanya Kulkarni April 16, 2024, 3:37 a.m. UTC | #3
On 4/15/24 20:17, Yu Kuai wrote:
> Hi,
>
> 在 2024/04/16 11:02, Chaitanya Kulkarni 写道:
>> On 4/15/24 19:00, Yu Kuai wrote:
>>> From: Yu Kuai <yukuai3@huawei.com>
>>>
>>> Test basic functionality.
>>>
>>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>>> ---
>>>    tests/throtl/001     | 84 
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>    tests/throtl/001.out |  6 ++++
>>>    tests/throtl/rc      | 15 ++++++++
>>>    3 files changed, 105 insertions(+)
>>>    create mode 100755 tests/throtl/001
>>>    create mode 100644 tests/throtl/001.out
>>>    create mode 100644 tests/throtl/rc
>>>
>>> diff --git a/tests/throtl/001 b/tests/throtl/001
>>> new file mode 100755
>>> index 0000000..79ecf07
>>> --- /dev/null
>>> +++ b/tests/throtl/001
>>> @@ -0,0 +1,84 @@
>>> +#!/bin/bash
>>> +# SPDX-License-Identifier: GPL-3.0+
>>> +# Copyright (C) 2024 Yu Kuai
>>> +#
>>> +# Test basic functionality of blk-throttle
>>> +
>>> +. tests/throtl/rc
>>> +
>>> +DESCRIPTION="basic functionality"
>>> +QUICK=1
>>> +
>>> +CG=/sys/fs/cgroup
>>> +TEST_DIR=$CG/blktests_throtl
>>> +devname=nullb0
>>> +dev=""
>>> +
>>> +set_up_test() {
>>> +    if ! _init_null_blk nr_devices=1; then
>>> +        return 1;
>>> +    fi
>>> +
>>> +    dev=$(cat /sys/block/$devname/dev)
>>> +    echo +io > $CG/cgroup.subtree_control
>>> +    mkdir $TEST_DIR
>>> +
>>
>> move above to 3 lines to rc with helper instead of repeating the
>> code for every test ?
>
> Yes, that sounds good, just test 004 is different.

indeed, but for future tests we are going need that anyways ..

>>
>>> +    return 0;
>>> +}
>>> +
>>> +clean_up_test() {
>>> +    rmdir $TEST_DIR
>>> +    echo -io > $CG/cgroup.subtree_control
>>> +    _exit_null_blk
>>
>> same here ?
>>
>>> +}
>>> +
>>> +config_throtl() {
>>> +    echo "$dev $*" > $TEST_DIR/io.max
>>> +}
>>> +
>>> +remove_config() {
>>> +    echo "$dev rbps=max wbps=max riops=max wiops=max" > 
>>> $TEST_DIR/io.max
>>> +}
>>> +
>>
>> same here for above two helper ?
>
> Yes, of course.
>>
>>> +test_io() {
>>> +    config_throtl "$1"
>>> +
>>> +    {
>>> +        sleep 0.1
>>> +        start_time=$(date +%s.%N)
>>> +
>>> +        if [ "$2" == "read" ]; then
>>> +            dd if=/dev/$devname of=/dev/null bs=4k count=256 
>>> iflag=direct status=none
>>> +        elif [ "$2" == "write" ]; then
>>> +            dd of=/dev/$devname if=/dev/zero bs=4k count=256 
>>> oflag=direct status=none
>>> +        fi
>>
>> Is there a any specific reason to use dd and not fio ?
>
> My thoughts is that I need to make sure the number and the size
> of IO that I dispatched, so that I can predict the throttle time,
> and we don't need to keep issuing new IO based on time here.

I believe we can do that with fio too, no ? the reason I'm asking
with fio we can specify io_engine and I believe we shuold
be using io_uring in general ? but then for such small I/Os it
may not matter so we can keep it as is ... I'll leave it to you ..

-ck
diff mbox series

Patch

diff --git a/tests/throtl/001 b/tests/throtl/001
new file mode 100755
index 0000000..79ecf07
--- /dev/null
+++ b/tests/throtl/001
@@ -0,0 +1,84 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Yu Kuai
+#
+# Test basic functionality of blk-throttle
+
+. tests/throtl/rc
+
+DESCRIPTION="basic functionality"
+QUICK=1
+
+CG=/sys/fs/cgroup
+TEST_DIR=$CG/blktests_throtl
+devname=nullb0
+dev=""
+
+set_up_test() {
+	if ! _init_null_blk nr_devices=1; then
+		return 1;
+	fi
+
+	dev=$(cat /sys/block/$devname/dev)
+	echo +io > $CG/cgroup.subtree_control
+	mkdir $TEST_DIR
+
+	return 0;
+}
+
+clean_up_test() {
+	rmdir $TEST_DIR
+	echo -io > $CG/cgroup.subtree_control
+	_exit_null_blk
+}
+
+config_throtl() {
+	echo "$dev $*" > $TEST_DIR/io.max
+}
+
+remove_config() {
+	echo "$dev rbps=max wbps=max riops=max wiops=max" > $TEST_DIR/io.max
+}
+
+test_io() {
+	config_throtl "$1"
+
+	{
+		sleep 0.1
+		start_time=$(date +%s.%N)
+
+		if [ "$2" == "read" ]; then
+			dd if=/dev/$devname of=/dev/null bs=4k count=256 iflag=direct status=none
+		elif [ "$2" == "write" ]; then
+			dd of=/dev/$devname if=/dev/zero bs=4k count=256 oflag=direct status=none
+		fi
+
+		end_time=$(date +%s.%N)
+		elapsed=$(echo "$end_time - $start_time" | bc)
+		printf "%.0f\n" "$elapsed"
+	} &
+
+	pid=$!
+	echo $! > $TEST_DIR/cgroup.procs
+	wait $pid
+
+	remove_config
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	if ! set_up_test; then
+		return 1;
+	fi
+
+	_1MB=$((1024 * 1024))
+
+	test_io wbps=$_1MB write
+	test_io wiops=256 write
+	test_io rbps=$_1MB read
+	test_io riops=256 read
+
+	clean_up_test
+	echo "Test complete"
+}
diff --git a/tests/throtl/001.out b/tests/throtl/001.out
new file mode 100644
index 0000000..a3edfdd
--- /dev/null
+++ b/tests/throtl/001.out
@@ -0,0 +1,6 @@ 
+Running throtl/001
+1
+1
+1
+1
+Test complete
diff --git a/tests/throtl/rc b/tests/throtl/rc
new file mode 100644
index 0000000..8fa8b58
--- /dev/null
+++ b/tests/throtl/rc
@@ -0,0 +1,15 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2024 Yu Kuai
+#
+# Tests for blk-throttle
+
+. common/rc
+. common/null_blk
+
+group_requires() {
+	_have_root
+	_have_null_blk
+	_have_kernel_option BLK_DEV_THROTTLING
+	_have_cgroup2_controller io
+}