Re: [PATCH 1/2] of: unittest: option to allow tests that trigger kernel stack dump

[Frank Rowand]

On 3/24/23 16:43, Rob Herring wrote:
> On Wed, Mar 01, 2023 at 10:01:36AM -0600, Frank Rowand wrote:
>> On 2/28/23 22:07, Guenter Roeck wrote:
>>> On 2/28/23 17:21, Frank Rowand wrote:
>>>> Commit 74df14cd301a ("of: unittest: add node lifecycle tests") added
>>>> some tests that trigger a kernel stack dump.  Filtering the boot
>>>> messages with scripts/dtc/of_unittest_expect detects that the stack
>>>> dump is expected instead of being a test error.
>>>>
>>>> Test beds might interpret the stack dumps as errors, resulting in
>>>> needless debugging and error reports.  These test beds are likely
>>>> to remove unittests due to these stack dumps. To avoid these problems,
>>>> have unittest default to skip the tests that trigger a stack dump.
>>>>
>>>> Add a kernel cmdline option to not skip those tests.  This option can
>>>> be used by testers who are able to interpret the stack dumps as not
>>>> an error.
>>>>
>>>> Signed-off-by: Frank Rowand <frowand.list@xxxxxxxxx>
>>>> ---
>>>>   drivers/of/unittest.c | 54 ++++++++++++++++++++++++++++++++++++++++---
>>>>   1 file changed, 51 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>>>> index b5a7a31d8bd2..3a9bc2bc4ba1 100644
>>>> --- a/drivers/of/unittest.c
>>>> +++ b/drivers/of/unittest.c
>>>> @@ -70,6 +70,36 @@ static struct unittest_results {
>>>>   #define EXPECT_NOT_END(level, fmt, ...) \
>>>>       printk(level pr_fmt("EXPECT_NOT / : ") fmt, ##__VA_ARGS__)
>>>>   +/*
>>>> + * Some tests will cause the kernel to emit a stack dump, aka back trace,
>>>> + * when the test is successful.  The tests should make it possible for
>>>> + * test beds to detect that the trace is not an error via EXPECT_BEGIN().
>>>> + *
>>>> + * Most test beds do not process the EXPECT_BEGIN() information and may
>>>> + * flag the stack dump as an error, thus reporting a false failure.  It
>>>> + * is hoped that the KTAP version 4 specification will add the EXPECT_BEGIN()
>>>> + * processing to test beds.
>>>> + *
>>>> + * By default, skip tests that cause a stack dump.  Test beds that process
>>>> + * EXPECT_BEGIN() information should enable these tests via a kernel boot
>>>> + * command line option.
>>>> + */
>>>> +static int stackdump_tests_enabled;
>>>> +
>>>> +static int __init enable_unittest_stackdump(char *str)
>>>> +{
>>>> +    stackdump_tests_enabled = 1;
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int __init disable_unittest_stackdump(char *str)
>>>> +{
>>>> +    stackdump_tests_enabled = 0;
>>>> +    return 0;
>>>> +}
>>>> +early_param("of_unittest_stackdump", enable_unittest_stackdump);
>>>> +early_param("no_of_unittest_stackdump", disable_unittest_stackdump);
>>>
>>> Does no_of_unittest_stackdump have any benefit or value ?
>>
>> I would say no, but it is a common pattern to provide both
>> foo and no_foo.
> 
> It is? I see one documented example. I see numerous ones that are 
> 'no_foo'.

I reconsidered.  I plan to remove the no_of_unittest_stackdump in v2, updated
to the current kernel version.

-Frank

> 
> This doesn't scale well if lots of tests need to disable it. Perhaps it 
> should be more generic (at least documentation/naming wise even if the 
> implmentation lives in DT unittest for now).
> 
> Rob