diff mbox series

tests/throtl: add a new test 006

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

Checks

Context Check Description
shin/vmtest-for-next-PR fail merge-conflict

Commit Message

Ming Lei Feb. 24, 2025, 9:59 a.m. UTC
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

Comments

Yu Kuai Feb. 24, 2025, 12:25 p.m. UTC | #1
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
>
Ming Lei Feb. 24, 2025, 1:54 p.m. UTC | #2
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
Yu Kuai Feb. 25, 2025, 2:07 a.m. UTC | #3
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
Shinichiro Kawasaki March 3, 2025, 12:06 p.m. UTC | #4
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.
Ming Lei March 4, 2025, 2:34 a.m. UTC | #5
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
Yu Kuai March 4, 2025, 2:46 a.m. UTC | #6
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
> 
> .
>
Ming Lei March 4, 2025, 10:02 a.m. UTC | #7
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
Yu Kuai March 4, 2025, 1:08 p.m. UTC | #8
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 mbox series

Patch

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