[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aGenhKOjXq6FTsTo@pc636>
Date: Fri, 4 Jul 2025 12:05:56 +0200
From: Uladzislau Rezki <urezki@...il.com>
To: Raghavendra K T <raghavendra.kt@....com>
Cc: urezki@...il.com, akpm@...ux-foundation.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
Dev Jain <dev.jain@....com>
Subject: Re: [PATCH] lib/test_vmalloc.c: introduce xfail for failing tests
Hello, Raghavendra!
>
> On 7/2/2025 1:43 PM, Dev Jain wrote:
> >
> > On 02/07/25 1:38 pm, Raghavendra K T wrote:
> > >
> > >
> > > On 7/2/2025 12:18 PM, Dev Jain wrote:
> > > >
> > > > On 02/07/25 12:13 pm, Raghavendra K T wrote:
> > > > > The test align_shift_alloc_test is expected to fail.
> > > > > Reporting the test as fail confuses to be a genuine failure.
> > > > > Introduce widely used xfail sematics to address the issue.
> > > > >
> > > > > Note: a warn_alloc dump similar to below is still expected:
> > > > >
> > > > > Call Trace:
> > > > > <TASK>
> > > > > dump_stack_lvl+0x64/0x80
> > > > > warn_alloc+0x137/0x1b0
> > > > > ? __get_vm_area_node+0x134/0x140
> > > > >
> > > > > Snippet of dmesg after change:
> > > > >
> > > > > Summary: random_size_align_alloc_test passed: 1 failed: 0 xfailed: 0 ..
> > > > > Summary: align_shift_alloc_test passed: 0 failed: 0 xfailed: 1 ..
> > > > > Summary: pcpu_alloc_test passed: 1 failed: 0 xfailed: 0 ..
> > > > >
> > > > > Signed-off-by: Raghavendra K T <raghavendra.kt@....com>
> > > > > ---
> > > >
> > > > Thanks for doing this, been thinking about this for so long but
> > > > I'm lazy : )
> > >
> > > :)
> > >
> > > >
> > > > > lib/test_vmalloc.c | 36 +++++++++++++++++++++---------------
> > > > > 1 file changed, 21 insertions(+), 15 deletions(-)
> > > > >
> > > > > diff --git a/lib/test_vmalloc.c b/lib/test_vmalloc.c
> > > > > index 1b0b59549aaf..649f352e2046 100644
> > > > > --- a/lib/test_vmalloc.c
> > > > > +++ b/lib/test_vmalloc.c
> > > > > @@ -396,25 +396,27 @@ vm_map_ram_test(void)
> > > > > struct test_case_desc {
> > > > > const char *test_name;
> > > > > int (*test_func)(void);
> > > > > + bool xfail;
> > > > > };
> > > > > static struct test_case_desc test_case_array[] = {
> > > > > - { "fix_size_alloc_test", fix_size_alloc_test },
> > > > > - { "full_fit_alloc_test", full_fit_alloc_test },
> > > > > - { "long_busy_list_alloc_test", long_busy_list_alloc_test },
> > > > > - { "random_size_alloc_test", random_size_alloc_test },
> > > > > - { "fix_align_alloc_test", fix_align_alloc_test },
> > > > > - { "random_size_align_alloc_test", random_size_align_alloc_test },
> > > > > - { "align_shift_alloc_test", align_shift_alloc_test },
> > > > > - { "pcpu_alloc_test", pcpu_alloc_test },
> > > > > - { "kvfree_rcu_1_arg_vmalloc_test",
> > > > > kvfree_rcu_1_arg_vmalloc_test },
> > > > > - { "kvfree_rcu_2_arg_vmalloc_test",
> > > > > kvfree_rcu_2_arg_vmalloc_test },
> > > > > - { "vm_map_ram_test", vm_map_ram_test },
> > > > > + { "fix_size_alloc_test", fix_size_alloc_test, },
> > > > > + { "full_fit_alloc_test", full_fit_alloc_test, },
> > > > > + { "long_busy_list_alloc_test", long_busy_list_alloc_test, },
> > > > > + { "random_size_alloc_test", random_size_alloc_test, },
> > > > > + { "fix_align_alloc_test", fix_align_alloc_test, },
> > > > > + { "random_size_align_alloc_test", random_size_align_alloc_test, },
> > > > > + { "align_shift_alloc_test", align_shift_alloc_test, true },
> > > > > + { "pcpu_alloc_test", pcpu_alloc_test, },
> > > > > + { "kvfree_rcu_1_arg_vmalloc_test",
> > > > > kvfree_rcu_1_arg_vmalloc_test, },
> > > > > + { "kvfree_rcu_2_arg_vmalloc_test",
> > > > > kvfree_rcu_2_arg_vmalloc_test, },
> > > > > + { "vm_map_ram_test", vm_map_ram_test, },
> > > > > /* Add a new test case here. */
> > > > > };
> > > >
> > > > Why this change?
> > >
> > > Perhaps not entirely necessary except for align_shift_alloc_test line,
> > > still updated the field since one more bool field added. But let me know
> > > if you are okay with current state OR need a respin for that?
> >
> > Oh now I saw the "true", I thought you were adding commas for no reason.
> >
> > I think that's fine then, but will let Uladzislau decide.
> >
> >
>
> Uladzislau,
>
> Do you think this patch would be useful? and above change is okay?
>
Sorry, i missed this. Yes, i think it makes sense since it confuses
people.
--
Uladzislau Rezki
Powered by blists - more mailing lists