Message ID | 20250224095945.1994997-1-ming.lei@redhat.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | tests/throtl: add a new test 006 | expand |
Context | Check | Description |
---|---|---|
shin/vmtest-for-next-PR | fail | merge-conflict |
Hi, 在 2025/02/24 17:59, Ming Lei 写道: > Add test for covering prioritized meta IO when throttling, regression > test for commit 29390bb5661d ("blk-throttle: support prioritized processing > of metadata"). > > Cc: Yu Kuai <yukuai1@huaweicloud.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > tests/throtl/006 | 58 ++++++++++++++++++++++++++++++++++++++++++++ > tests/throtl/006.out | 4 +++ > tests/throtl/rc | 19 +++++++++++++++ > 3 files changed, 81 insertions(+) > create mode 100755 tests/throtl/006 > create mode 100644 tests/throtl/006.out > > diff --git a/tests/throtl/006 b/tests/throtl/006 > new file mode 100755 > index 0000000..4baadaf > --- /dev/null > +++ b/tests/throtl/006 > @@ -0,0 +1,58 @@ > +#!/bin/bash > +# SPDX-License-Identifier: GPL-3.0+ > +# Copyright (C) 2025 Ming Lei > +# > +# Test prioritized meta IO when IO throttling, regression test for > +# commit 29390bb5661d ("blk-throttle: support prioritized processing of metadata") > + > +. tests/throtl/rc > + > +DESCRIPTION="test if meta IO has higher priority than data IO" > +QUICK=1 > + > +requires() { > + _have_program mkfs.ext4 > +} > + > +test_meta_io() { > + local path="$1" > + local start_time > + local end_time > + local elapsed > + > + start_time=$(date +%s.%N) > + mkdir "${path}"/xxx > + touch "${path}"/xxx/1 > + sync "${path}"/xxx > + > + end_time=$(date +%s.%N) > + elapsed=$(echo "$end_time - $start_time" | bc) > + printf "%.0f\n" "$elapsed" > +} > + > +test() { > + echo "Running ${TEST_NAME}" > + > + if ! _set_up_throtl memory_backed=1; then > + return 1; > + fi > + > + mkdir -p "${TMPDIR}/mnt" > + mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 -F "/dev/${THROTL_DEV}" >> "$FULL" 2>&1 > + mount "/dev/${THROTL_DEV}" "${TMPDIR}/mnt" >> "$FULL" 2>&1 > + > + _throtl_set_limits wbps=$((1024 * 1024)) > + { > + echo "$BASHPID" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" > + _throtl_issue_fs_io "${TMPDIR}/mnt/test.img" write 64K 64 & > + sleep 2 > + test_meta_io "${TMPDIR}/mnt" Do you run this test without the kernel patch? If I remembered correctly, ext4 issue the meta IO from jbd2 by default, which is from root cgroup, and root cgroup can only throttled from cgroup v1. Thanks, Kuai > + wait > + } & > + wait $! > + > + umount "${TMPDIR}/mnt" || return $? > + _throtl_remove_limits > + _clean_up_throtl > + echo "Test complete" > +} > diff --git a/tests/throtl/006.out b/tests/throtl/006.out > new file mode 100644 > index 0000000..8c3d176 > --- /dev/null > +++ b/tests/throtl/006.out > @@ -0,0 +1,4 @@ > +Running throtl/006 > +0 > +4 > +Test complete > diff --git a/tests/throtl/rc b/tests/throtl/rc > index df54cb9..327084b 100644 > --- a/tests/throtl/rc > +++ b/tests/throtl/rc > @@ -75,6 +75,25 @@ _throtl_get_max_io_size() { > cat "/sys/block/$THROTL_DEV/queue/max_sectors_kb" > } > > +_throtl_issue_fs_io() { > + local path=$1 > + local start_time > + local end_time > + local elapsed > + > + start_time=$(date +%s.%N) > + > + if [ "$2" == "read" ]; then > + dd if="${path}" of=/dev/null bs="$3" count="$4" iflag=direct status=none > + elif [ "$2" == "write" ]; then > + dd of="${path}" if=/dev/zero bs="$3" count="$4" oflag=direct conv=fdatasync status=none > + fi > + > + end_time=$(date +%s.%N) > + elapsed=$(echo "$end_time - $start_time" | bc) > + printf "%.0f\n" "$elapsed" > +} > + > _throtl_issue_io() { > local start_time > local end_time >
On Mon, Feb 24, 2025 at 8:26 PM Yu Kuai <yukuai1@huaweicloud.com> wrote: > > Hi, > > 在 2025/02/24 17:59, Ming Lei 写道: > > Add test for covering prioritized meta IO when throttling, regression > > test for commit 29390bb5661d ("blk-throttle: support prioritized processing > > of metadata"). > > > > Cc: Yu Kuai <yukuai1@huaweicloud.com> > > Signed-off-by: Ming Lei <ming.lei@redhat.com> > > --- > > tests/throtl/006 | 58 ++++++++++++++++++++++++++++++++++++++++++++ > > tests/throtl/006.out | 4 +++ > > tests/throtl/rc | 19 +++++++++++++++ > > 3 files changed, 81 insertions(+) > > create mode 100755 tests/throtl/006 > > create mode 100644 tests/throtl/006.out > > > > diff --git a/tests/throtl/006 b/tests/throtl/006 > > new file mode 100755 > > index 0000000..4baadaf > > --- /dev/null > > +++ b/tests/throtl/006 > > @@ -0,0 +1,58 @@ > > +#!/bin/bash > > +# SPDX-License-Identifier: GPL-3.0+ > > +# Copyright (C) 2025 Ming Lei > > +# > > +# Test prioritized meta IO when IO throttling, regression test for > > +# commit 29390bb5661d ("blk-throttle: support prioritized processing of metadata") > > + > > +. tests/throtl/rc > > + > > +DESCRIPTION="test if meta IO has higher priority than data IO" > > +QUICK=1 > > + > > +requires() { > > + _have_program mkfs.ext4 > > +} > > + > > +test_meta_io() { > > + local path="$1" > > + local start_time > > + local end_time > > + local elapsed > > + > > + start_time=$(date +%s.%N) > > + mkdir "${path}"/xxx > > + touch "${path}"/xxx/1 > > + sync "${path}"/xxx > > + > > + end_time=$(date +%s.%N) > > + elapsed=$(echo "$end_time - $start_time" | bc) > > + printf "%.0f\n" "$elapsed" > > +} > > + > > +test() { > > + echo "Running ${TEST_NAME}" > > + > > + if ! _set_up_throtl memory_backed=1; then > > + return 1; > > + fi > > + > > + mkdir -p "${TMPDIR}/mnt" > > + mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 -F "/dev/${THROTL_DEV}" >> "$FULL" 2>&1 > > + mount "/dev/${THROTL_DEV}" "${TMPDIR}/mnt" >> "$FULL" 2>&1 > > + > > + _throtl_set_limits wbps=$((1024 * 1024)) > > + { > > + echo "$BASHPID" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" > > + _throtl_issue_fs_io "${TMPDIR}/mnt/test.img" write 64K 64 & > > + sleep 2 > > + test_meta_io "${TMPDIR}/mnt" > > Do you run this test without the kernel patch? If I remembered > correctly, ext4 issue the meta IO from jbd2 by default, which is from > root cgroup, and root cgroup can only throttled from cgroup v1. It passes on v6.14-rc1, does META/SWAP IO only route from cgroup v1? static inline bool bio_issue_as_root_blkg(struct bio *bio) { return (bio->bi_opf & (REQ_META | REQ_SWAP)) != 0; } Thanks, Ming
Hi, 在 2025/02/24 21:54, Ming Lei 写道: >> Do you run this test without the kernel patch? If I remembered >> correctly, ext4 issue the meta IO from jbd2 by default, which is from >> root cgroup, and root cgroup can only throttled from cgroup v1. > It passes on v6.14-rc1, does META/SWAP IO only route from cgroup v1? Of course not, it's just ext4 will issue such IO from root cgroup, and there is no IO can be throttled here. You might want to bind jbd2 to the test cgroup as well. Thanks, Kuai > > static inline bool bio_issue_as_root_blkg(struct bio *bio) > { > return (bio->bi_opf & (REQ_META | REQ_SWAP)) != 0; > } > > Thanks, > Ming
Hi, Ming, thank you for the patch. On Feb 24, 2025 / 17:59, Ming Lei wrote: > Add test for covering prioritized meta IO when throttling, regression > test for commit 29390bb5661d ("blk-throttle: support prioritized processing > of metadata"). I ran this test case with the kernel v6.14-rc3, and it passed. Then I reverted the commit 29390bb5661d form the kernel, and still the test case passed. I wonder how can I make the test case fail. The commit was in v6.12-rc1 tag, so do I need to try with v6.11 kernel to see it fails? I have two nit comments in line. If you respin the patch, please consider to fold them in. > > Cc: Yu Kuai <yukuai1@huaweicloud.com> > Signed-off-by: Ming Lei <ming.lei@redhat.com> > --- > tests/throtl/006 | 58 ++++++++++++++++++++++++++++++++++++++++++++ > tests/throtl/006.out | 4 +++ > tests/throtl/rc | 19 +++++++++++++++ > 3 files changed, 81 insertions(+) > create mode 100755 tests/throtl/006 > create mode 100644 tests/throtl/006.out > > diff --git a/tests/throtl/006 b/tests/throtl/006 > new file mode 100755 > index 0000000..4baadaf > --- /dev/null > +++ b/tests/throtl/006 [...] > +test() { > + echo "Running ${TEST_NAME}" > + > + if ! _set_up_throtl memory_backed=1; then > + return 1; > + fi > + > + mkdir -p "${TMPDIR}/mnt" Nit: the long option --parents is preferred to the short option -p. > + mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 -F "/dev/${THROTL_DEV}" >> "$FULL" 2>&1 Nit: this line is a big long, then I suggest to split into two lines with backslash.
On Mon, Mar 03, 2025 at 12:06:44PM +0000, Shinichiro Kawasaki wrote: > Hi, Ming, thank you for the patch. > > On Feb 24, 2025 / 17:59, Ming Lei wrote: > > Add test for covering prioritized meta IO when throttling, regression > > test for commit 29390bb5661d ("blk-throttle: support prioritized processing > > of metadata"). > > I ran this test case with the kernel v6.14-rc3, and it passed. Then I reverted > the commit 29390bb5661d form the kernel, and still the test case passed. I > wonder how can I make the test case fail. The commit was in v6.12-rc1 tag, so > do I need to try with v6.11 kernel to see it fails? > > I have two nit comments in line. If you respin the patch, please consider to > fold them in. The test needs to setup cgroup v1, I guess. Kuai, any idea for setting one test to cover the change of commit 29390bb5661d ("blk-throttle: support prioritized processing of metadata")? Thanks, Ming
Hi, 在 2025/02/25 10:07, Yu Kuai 写道: > Hi, > > 在 2025/02/24 21:54, Ming Lei 写道: >>> Do you run this test without the kernel patch? If I remembered >>> correctly, ext4 issue the meta IO from jbd2 by default, which is from >>> root cgroup, and root cgroup can only throttled from cgroup v1. >> It passes on v6.14-rc1, does META/SWAP IO only route from cgroup v1? > > Of course not, it's just ext4 will issue such IO from root cgroup, and > there is no IO can be throttled here. > > You might want to bind jbd2 to the test cgroup as well. I think the test will be OK this way. Thanks, Kuai > > Thanks, > Kuai > >> >> static inline bool bio_issue_as_root_blkg(struct bio *bio) >> { >> return (bio->bi_opf & (REQ_META | REQ_SWAP)) != 0; >> } >> >> Thanks, >> Ming > > . >
On Tue, Mar 04, 2025 at 10:46:40AM +0800, Yu Kuai wrote: > Hi, > > 在 2025/02/25 10:07, Yu Kuai 写道: > > Hi, > > > > 在 2025/02/24 21:54, Ming Lei 写道: > > > > Do you run this test without the kernel patch? If I remembered > > > > correctly, ext4 issue the meta IO from jbd2 by default, which is from > > > > root cgroup, and root cgroup can only throttled from cgroup v1. > > > It passes on v6.14-rc1, does META/SWAP IO only route from cgroup v1? > > > > Of course not, it's just ext4 will issue such IO from root cgroup, and > > there is no IO can be throttled here. > > > > You might want to bind jbd2 to the test cgroup as well. But the issue still can't be reproduced by adding the following delta change, meantime revert 29390bb5661d ("blk-throttle: support prioritized processing of metadata") on kernel side. diff --git a/tests/throtl/006 b/tests/throtl/006 index 4baadaf..bb09eb2 100755 --- a/tests/throtl/006 +++ b/tests/throtl/006 @@ -43,7 +43,11 @@ test() { _throtl_set_limits wbps=$((1024 * 1024)) { + local jbd2_pid + + jbd2_pid=$(ps -eo pid,comm |grep "jbd2/${THROTL_DEV}" |awk '{print $1}') echo "$BASHPID" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" + echo "$jbd2_pid" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" _throtl_issue_fs_io "${TMPDIR}/mnt/test.img" write 64K 64 & sleep 2 test_meta_io "${TMPDIR}/mnt" Thanks, Ming
Hi, 在 2025/03/04 18:02, Ming Lei 写道: > But the issue still can't be reproduced by adding the following delta > change, meantime revert 29390bb5661d ("blk-throttle: support prioritized > processing of metadata") on kernel side. Because you're issuing 64 64k IO, and dd is issuing next IO after the previous IO is done. Following diff will work for this test: Thanks, Kuai diff --git a/tests/throtl/006 b/tests/throtl/006 index 4baadaf..758293b 100755 --- a/tests/throtl/006 +++ b/tests/throtl/006 @@ -43,8 +43,13 @@ test() { _throtl_set_limits wbps=$((1024 * 1024)) { + local jbd2_pid + + jbd2_pid=$(ps -eo pid,comm |grep "jbd2/${THROTL_DEV}" |awk '{print $1}') echo "$BASHPID" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" - _throtl_issue_fs_io "${TMPDIR}/mnt/test.img" write 64K 64 & + echo "$jbd2_pid" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" + + _throtl_issue_fs_io "${TMPDIR}/mnt/test.img" write 4M 1 & sleep 2 test_meta_io "${TMPDIR}/mnt" wait
diff --git a/tests/throtl/006 b/tests/throtl/006 new file mode 100755 index 0000000..4baadaf --- /dev/null +++ b/tests/throtl/006 @@ -0,0 +1,58 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-3.0+ +# Copyright (C) 2025 Ming Lei +# +# Test prioritized meta IO when IO throttling, regression test for +# commit 29390bb5661d ("blk-throttle: support prioritized processing of metadata") + +. tests/throtl/rc + +DESCRIPTION="test if meta IO has higher priority than data IO" +QUICK=1 + +requires() { + _have_program mkfs.ext4 +} + +test_meta_io() { + local path="$1" + local start_time + local end_time + local elapsed + + start_time=$(date +%s.%N) + mkdir "${path}"/xxx + touch "${path}"/xxx/1 + sync "${path}"/xxx + + end_time=$(date +%s.%N) + elapsed=$(echo "$end_time - $start_time" | bc) + printf "%.0f\n" "$elapsed" +} + +test() { + echo "Running ${TEST_NAME}" + + if ! _set_up_throtl memory_backed=1; then + return 1; + fi + + mkdir -p "${TMPDIR}/mnt" + mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 -F "/dev/${THROTL_DEV}" >> "$FULL" 2>&1 + mount "/dev/${THROTL_DEV}" "${TMPDIR}/mnt" >> "$FULL" 2>&1 + + _throtl_set_limits wbps=$((1024 * 1024)) + { + echo "$BASHPID" > "$CGROUP2_DIR/$THROTL_DIR/cgroup.procs" + _throtl_issue_fs_io "${TMPDIR}/mnt/test.img" write 64K 64 & + sleep 2 + test_meta_io "${TMPDIR}/mnt" + wait + } & + wait $! + + umount "${TMPDIR}/mnt" || return $? + _throtl_remove_limits + _clean_up_throtl + echo "Test complete" +} diff --git a/tests/throtl/006.out b/tests/throtl/006.out new file mode 100644 index 0000000..8c3d176 --- /dev/null +++ b/tests/throtl/006.out @@ -0,0 +1,4 @@ +Running throtl/006 +0 +4 +Test complete diff --git a/tests/throtl/rc b/tests/throtl/rc index df54cb9..327084b 100644 --- a/tests/throtl/rc +++ b/tests/throtl/rc @@ -75,6 +75,25 @@ _throtl_get_max_io_size() { cat "/sys/block/$THROTL_DEV/queue/max_sectors_kb" } +_throtl_issue_fs_io() { + local path=$1 + local start_time + local end_time + local elapsed + + start_time=$(date +%s.%N) + + if [ "$2" == "read" ]; then + dd if="${path}" of=/dev/null bs="$3" count="$4" iflag=direct status=none + elif [ "$2" == "write" ]; then + dd of="${path}" if=/dev/zero bs="$3" count="$4" oflag=direct conv=fdatasync status=none + fi + + end_time=$(date +%s.%N) + elapsed=$(echo "$end_time - $start_time" | bc) + printf "%.0f\n" "$elapsed" +} + _throtl_issue_io() { local start_time local end_time
Add test for covering prioritized meta IO when throttling, regression test for commit 29390bb5661d ("blk-throttle: support prioritized processing of metadata"). Cc: Yu Kuai <yukuai1@huaweicloud.com> Signed-off-by: Ming Lei <ming.lei@redhat.com> --- tests/throtl/006 | 58 ++++++++++++++++++++++++++++++++++++++++++++ tests/throtl/006.out | 4 +++ tests/throtl/rc | 19 +++++++++++++++ 3 files changed, 81 insertions(+) create mode 100755 tests/throtl/006 create mode 100644 tests/throtl/006.out