diff mbox series

ext4/032: add a new testcase in online resize tests

Message ID 20241108134817.128078-1-aleksandr.mikhalitsyn@canonical.com
State Not Applicable
Headers show
Series ext4/032: add a new testcase in online resize tests | expand

Commit Message

Aleksandr Mikhalitsyn Nov. 8, 2024, 1:48 p.m. UTC
Add a new testcase for [1] commit in ext4 online resize testsuite.

Link: https://lore.kernel.org/linux-ext4/20240927133329.1015041-1-libaokun@huaweicloud.com [1]
Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
---
 tests/ext4/032 | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Baokun Li Nov. 9, 2024, 3:32 a.m. UTC | #1
Hi Alexander,

Thanks for the patch.

On 2024/11/8 21:48, Alexander Mikhalitsyn wrote:
> Add a new testcase for [1] commit in ext4 online resize testsuite.
>
> Link: https://lore.kernel.org/linux-ext4/20240927133329.1015041-1-libaokun@huaweicloud.com [1]
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---
>   tests/ext4/032 | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/tests/ext4/032 b/tests/ext4/032
> index 6bc3b61b..77d592f4 100755
> --- a/tests/ext4/032
> +++ b/tests/ext4/032
> @@ -97,6 +97,10 @@ mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
>   # Check if online resizing with bigalloc is supported by the kernel
>   ext4_online_resize 4096 8192 1
>   
> +_fixed_by_kernel_commit 6121258c2b33 \
> +	"ext4: fix off by one issue in alloc_flex_gd()"
> +ext4_online_resize $(c2b 6400) $(c2b 786432)
> +
I think this test would be better placed in the loop below. Then add some
comments describing the scenario being tested.

There are two current scenarios for off by one:
  * The above test is to expand from the first block group of a flex_bg to
    the next flex_bg;
  * Another scenario is to expand from the first block group of a flex_bg
    to the last block group of this flex_bg. For example,
      `ext4_online_resize $(c2b 6400) $(c2b 524288)`

In addition, we need to modify the tests/ext4/032.out or the use cases
will fail due to inconsistent output.


Regards,
Baokun
>   ## We perform resizing to various multiples of block group sizes to
>   ## ensure that we cover maximum edge cases in the kernel code.
>   for CLUSTER_SIZ in 4096 16384 65536; do
Zorro Lang Nov. 9, 2024, 12:01 p.m. UTC | #2
On Fri, Nov 08, 2024 at 02:48:17PM +0100, Alexander Mikhalitsyn wrote:
> Add a new testcase for [1] commit in ext4 online resize testsuite.
> 
> Link: https://lore.kernel.org/linux-ext4/20240927133329.1015041-1-libaokun@huaweicloud.com [1]
> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> ---
>  tests/ext4/032 | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tests/ext4/032 b/tests/ext4/032
> index 6bc3b61b..77d592f4 100755
> --- a/tests/ext4/032
> +++ b/tests/ext4/032
> @@ -97,6 +97,10 @@ mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
>  # Check if online resizing with bigalloc is supported by the kernel
>  ext4_online_resize 4096 8192 1
>  
> +_fixed_by_kernel_commit 6121258c2b33 \
> +	"ext4: fix off by one issue in alloc_flex_gd()"

We generally mark this at the beginning of the test, not in the middle of test
running. Refer to ext4/058.

Thanks,
Zorro

> +ext4_online_resize $(c2b 6400) $(c2b 786432)
> +
>  ## We perform resizing to various multiples of block group sizes to
>  ## ensure that we cover maximum edge cases in the kernel code.
>  for CLUSTER_SIZ in 4096 16384 65536; do
> -- 
> 2.43.0
> 
>
Aleksandr Mikhalitsyn Nov. 11, 2024, 3:22 p.m. UTC | #3
On Sat, Nov 9, 2024 at 1:02 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Fri, Nov 08, 2024 at 02:48:17PM +0100, Alexander Mikhalitsyn wrote:
> > Add a new testcase for [1] commit in ext4 online resize testsuite.
> >
> > Link: https://lore.kernel.org/linux-ext4/20240927133329.1015041-1-libaokun@huaweicloud.com [1]
> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> > ---
> >  tests/ext4/032 | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/tests/ext4/032 b/tests/ext4/032
> > index 6bc3b61b..77d592f4 100755
> > --- a/tests/ext4/032
> > +++ b/tests/ext4/032
> > @@ -97,6 +97,10 @@ mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
> >  # Check if online resizing with bigalloc is supported by the kernel
> >  ext4_online_resize 4096 8192 1
> >
> > +_fixed_by_kernel_commit 6121258c2b33 \
> > +     "ext4: fix off by one issue in alloc_flex_gd()"
>
> We generally mark this at the beginning of the test, not in the middle of test
> running. Refer to ext4/058.

Hi Zorro,

have fixed it. Thanks!

Kind regards,
Alex

>
> Thanks,
> Zorro
>
> > +ext4_online_resize $(c2b 6400) $(c2b 786432)
> > +
> >  ## We perform resizing to various multiples of block group sizes to
> >  ## ensure that we cover maximum edge cases in the kernel code.
> >  for CLUSTER_SIZ in 4096 16384 65536; do
> > --
> > 2.43.0
> >
> >
>
Aleksandr Mikhalitsyn Nov. 11, 2024, 3:25 p.m. UTC | #4
On Sat, Nov 9, 2024 at 4:32 AM Baokun Li <libaokun1@huawei.com> wrote:
>
> Hi Alexander,
>
> Thanks for the patch.
>
> On 2024/11/8 21:48, Alexander Mikhalitsyn wrote:
> > Add a new testcase for [1] commit in ext4 online resize testsuite.
> >
> > Link: https://lore.kernel.org/linux-ext4/20240927133329.1015041-1-libaokun@huaweicloud.com [1]
> > Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
> > ---
> >   tests/ext4/032 | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/tests/ext4/032 b/tests/ext4/032
> > index 6bc3b61b..77d592f4 100755
> > --- a/tests/ext4/032
> > +++ b/tests/ext4/032
> > @@ -97,6 +97,10 @@ mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
> >   # Check if online resizing with bigalloc is supported by the kernel
> >   ext4_online_resize 4096 8192 1
> >
> > +_fixed_by_kernel_commit 6121258c2b33 \
> > +     "ext4: fix off by one issue in alloc_flex_gd()"
> > +ext4_online_resize $(c2b 6400) $(c2b 786432)
> > +

Hi Baokun,

> I think this test would be better placed in the loop below. Then add some
> comments describing the scenario being tested.

Have done. Thanks!

>
> There are two current scenarios for off by one:
>   * The above test is to expand from the first block group of a flex_bg to
>     the next flex_bg;
>   * Another scenario is to expand from the first block group of a flex_bg
>     to the last block group of this flex_bg. For example,
>       `ext4_online_resize $(c2b 6400) $(c2b 524288)`

This test does not fail for me when I test without "ext4: fix off by
one issue in alloc_flex_gd()" fix, so I decided not to take it.

>
> In addition, we need to modify the tests/ext4/032.out or the use cases
> will fail due to inconsistent output.

Of course, my bad, I forgot to add this file to the commit... stupid mistake.

I have fixed it in v2:
https://lore.kernel.org/fstests/20241111152100.152924-1-aleksandr.mikhalitsyn@canonical.com/T/#u

Kind regards,
Alex

>
>
> Regards,
> Baokun
> >   ## We perform resizing to various multiples of block group sizes to
> >   ## ensure that we cover maximum edge cases in the kernel code.
> >   for CLUSTER_SIZ in 4096 16384 65536; do
>
>
Baokun Li Nov. 14, 2024, 3:04 a.m. UTC | #5
Hi Alexander,

On 2024/11/11 23:25, Aleksandr Mikhalitsyn wrote:
> On Sat, Nov 9, 2024 at 4:32 AM Baokun Li <libaokun1@huawei.com> wrote:
>> Hi Alexander,
>>
>> Thanks for the patch.
>>
>> On 2024/11/8 21:48, Alexander Mikhalitsyn wrote:
>>> Add a new testcase for [1] commit in ext4 online resize testsuite.
>>>
>>> Link: https://lore.kernel.org/linux-ext4/20240927133329.1015041-1-libaokun@huaweicloud.com [1]
>>> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@canonical.com>
>>> ---
>>>    tests/ext4/032 | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/tests/ext4/032 b/tests/ext4/032
>>> index 6bc3b61b..77d592f4 100755
>>> --- a/tests/ext4/032
>>> +++ b/tests/ext4/032
>>> @@ -97,6 +97,10 @@ mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
>>>    # Check if online resizing with bigalloc is supported by the kernel
>>>    ext4_online_resize 4096 8192 1
>>>
>>> +_fixed_by_kernel_commit 6121258c2b33 \
>>> +     "ext4: fix off by one issue in alloc_flex_gd()"
>>> +ext4_online_resize $(c2b 6400) $(c2b 786432)
>>> +
> Hi Baokun,
>
>> I think this test would be better placed in the loop below. Then add some
>> comments describing the scenario being tested.
> Have done. Thanks!
Okay.
>> There are two current scenarios for off by one:
>>    * The above test is to expand from the first block group of a flex_bg to
>>      the next flex_bg;
>>    * Another scenario is to expand from the first block group of a flex_bg
>>      to the last block group of this flex_bg. For example,
>>        `ext4_online_resize $(c2b 6400) $(c2b 524288)`
> This test does not fail for me when I test without "ext4: fix off by
> one issue in alloc_flex_gd()" fix, so I decided not to take it.
Well, since we didn't check the off-by-one case directly, the latter case
really didn't cause the test case to fail before, and it doesn't appear
to have any effect at the moment, other than using some more memory.


Cheers,
Baokun
diff mbox series

Patch

diff --git a/tests/ext4/032 b/tests/ext4/032
index 6bc3b61b..77d592f4 100755
--- a/tests/ext4/032
+++ b/tests/ext4/032
@@ -97,6 +97,10 @@  mkdir -p $IMG_MNT || _fail "cannot create loopback mount point"
 # Check if online resizing with bigalloc is supported by the kernel
 ext4_online_resize 4096 8192 1
 
+_fixed_by_kernel_commit 6121258c2b33 \
+	"ext4: fix off by one issue in alloc_flex_gd()"
+ext4_online_resize $(c2b 6400) $(c2b 786432)
+
 ## We perform resizing to various multiples of block group sizes to
 ## ensure that we cover maximum edge cases in the kernel code.
 for CLUSTER_SIZ in 4096 16384 65536; do