Re: [PATCH] lib/test_vmalloc.c: introduce xfail for failing tests

[Dev Jain <dev.jain@arm.com>]

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@amd.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.


>
> [...]