| Message ID | 20240416020042.509291-2-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add new tests for blk-throttle | expand |
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
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 > >
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 --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 +}