Re: [RFC 00/10] drm: selftests: Convert to KUnit

["Michał Winiarski" <michal.winiarski@intel.com>]

On 19.01.2022 00:58, Daniel Latypov wrote:
>   change
> On Mon, Jan 17, 2022 at 3:24 PM Michał Winiarski
> <michal.winiarski@intel.com> wrote:
>>
>> KUnit unifies the test structure and provides helper tools that simplify
>> the development. Basic use case allows running tests as regular processes,
>> leveraging User Mode Linux.
>> For example, to execute all DRM unit tests:
>> ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm
>> (the tool also allows using QEMU instead of UML by adding e.g. --arch=x86_64)
>>
>> For developers - it means that it's easier to run unit tests on the development
>> machine, tightening the feedback loop. When using UML, it also simplifies using
> 
> Nice, it's neat to see --kunitconfig being useful for having
> one-liners for running tests.
> 
>> gdb for debug (since the kernel is just a regular process).
> 
> Anecdotally, I hear from davidgow@google.com that using gdb and UML
> together isn't the nicest experience because of all the SIGSEGV flying
> around when emulating paging.
> So I'm a bit doubtful about this particular statement, but if you have
> tried it out and it works well, then that's good too.
> 
> I just think the primary benefit of UML is faster compilation and it
> being somewhat lighter than bringing up a VM.

It was good enough to debug any problems that I accidentally introduced 
during the conversion, but perhaps that was simple enough usecase to not 
encounter SIGSEGVs.

> 
>> For CI systems - DRM tests can be moved from being executed on device under
>> test (that's also running IGTs and so on) to being executed on buildsystem
>> during build (just like checkpatch.pl).
>>
>> All tests were renamed - IGT prefix is no longer used.
>>
>> Compared to selftests executed by CI using IGT, there's one functional
>> regression - KUnit test runner is not catching WARNs.
>> To solve this, we could either go in the similar direction that UBSAN went in:
>> 1195505 ("kunit: ubsan integration")
> 
> Is the suggestion that all WARN's fail the current KUnit test?
> I don't think we can do that.

Yes - that's the suggestion.
The CI used for DRM has a separate "WARN" test state, even if the test 
is PASSing, any WARNs cause the test to end up in this state.
For example:
https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11112/bat-adlp-4/igt@kms_addfb_basic@addfb25-framebuffer-vs-set-tiling.html

> 
> Some KUnit tests will explicitly want to trigger error paths, so we
> could have a lot of false positives.
> 
> An alternative is that we can apply the 1195505 to the code paths
> we're interested in, e.g.
> 
> #include <kunit/test-bug.h>
> if (bad_thing()) {
>    kunit_fail_current_test("bad_thing happened");
> }
> 
> 
> I don't have the context to know how cumbersome this would be for the
> DRM tests though.
> If the answer is we want to catch any and all warnings, then we'd
> perhaps want to add something to the tests themselves. And maybe we
> should implement that as a KUnit library helper function so that other
> tests can use it as well.

I don't thing that would work. IIUC, currently the runner is operating 
on taint - there's a subset of taints that it considers "fatal" 
(including TAINT_WARN).

If we have tests that WARN, perhaps we could introduce extra flag to the 
test state on per-test or per-testsuite level, to mark that the test 
wants to fail-on-warn?
Then we would only fail if the test opted in (or other way around? if 
opt-out makes more sense and we have more tests that actually don't WARN 
as part of their normal test logic).

> 
>> Or we could expand the test runner to catch WARN signature in dmesg.
> 
> Ditto from the above, I think we'd wrongly mark some tests as failed
> for intentional warnings.
> 
>>
>> Pastebin to preview the output and execution times:
>> https://gitlab.freedesktop.org/-/snippets/4139
> 
> I see these take 17-18s to exec the tests in the example snippets.
> FYI, if you're not already trying this on top of 5.16, there's recent
> changes to make the parsed output more fully realtime as well.
> So hopefully that should increase the usability of these tests a bit further.
> 
> I only mention that since I wasn't able to apply this series without
> conflicts on top of v5.16.

It's applied on top of DRM subsystem integration tree (drm-tip):
https://cgit.freedesktop.org/drm-tip
At that time it was already based on v5.16.

Most of that 17-18s is taken by two subtest of drm_mm_tests:

[22:17:19] ============================================================
[22:17:19] ================= drm_mm_tests (1 subtest) =================
[22:17:27] [PASSED] test_insert
[22:17:27] ================== [PASSED] drm_mm_tests ===================
[22:17:27] ============================================================
[22:17:27] Testing complete. Passed: 1, Failed: 0, Crashed: 0, Skipped: 
0, Errors: 0
[22:17:27] Elapsed time: 10.400s total, 0.001s configuring, 2.419s 
building, 7.947s running

[22:17:42] ============================================================
[22:17:42] ================= drm_mm_tests (1 subtest) =================
[22:17:50] [PASSED] test_replace
[22:17:50] ================== [PASSED] drm_mm_tests ===================
[22:17:50] ============================================================
[22:17:50] Testing complete. Passed: 1, Failed: 0, Crashed: 0, Skipped: 
0, Errors: 0
[22:17:50] Elapsed time: 10.272s total, 0.001s configuring, 2.492s 
building, 7.776s running

Their runtime can be controlled with max_prime and max_iterations 
modparams - I left the default values intact, but we can tweak them to 
speed things up if needed.

Thanks!
-Michał

> 
>>
>> -Michał
>>
>> Michał Winiarski (10):
>>    drm: test-drm_cmdline_parser: Convert to KUnit
>>    drm: test-drm_plane_helper: Convert to KUnit
>>    drm: test-drm_format: Convert to KUnit
>>    drm: test-drm_framebuffer: Convert to KUnit
>>    drm: test-drm_damage_helper: Convert to KUnit
>>    drm: test-drm_dp_mst_helper: Convert to KUnit
>>    drm: test-drm_rect: Convert to KUnit
>>    drm: test-drm_mm: Convert to KUnit
>>    drm: selftests: Convert to KUnit
>>    drm: test: Simplify testing on UML with kunit.py
>>
>>   drivers/gpu/drm/.kunitconfig                  |    3 +
>>   drivers/gpu/drm/Kconfig                       |   22 +-
>>   drivers/gpu/drm/Makefile                      |    2 +-
>>   drivers/gpu/drm/i915/Kconfig.debug            |    1 -
>>   drivers/gpu/drm/selftests/Makefile            |    7 -
>>   .../gpu/drm/selftests/drm_cmdline_selftests.h |   68 -
>>   drivers/gpu/drm/selftests/drm_mm_selftests.h  |   28 -
>>   .../gpu/drm/selftests/drm_modeset_selftests.h |   40 -
>>   drivers/gpu/drm/selftests/drm_selftest.c      |  109 -
>>   drivers/gpu/drm/selftests/drm_selftest.h      |   41 -
>>   .../drm/selftests/test-drm_cmdline_parser.c   | 1141 --------
>>   .../drm/selftests/test-drm_damage_helper.c    |  667 -----
>>   .../drm/selftests/test-drm_dp_mst_helper.c    |  273 --
>>   drivers/gpu/drm/selftests/test-drm_format.c   |  280 --
>>   drivers/gpu/drm/selftests/test-drm_mm.c       | 2487 -----------------
>>   .../drm/selftests/test-drm_modeset_common.c   |   32 -
>>   .../drm/selftests/test-drm_modeset_common.h   |   52 -
>>   .../gpu/drm/selftests/test-drm_plane_helper.c |  223 --
>>   drivers/gpu/drm/selftests/test-drm_rect.c     |  223 --
>>   drivers/gpu/drm/test/Makefile                 |    7 +
>>   .../gpu/drm/test/test-drm_cmdline_parser.c    | 1027 +++++++
>>   drivers/gpu/drm/test/test-drm_damage_helper.c |  667 +++++
>>   drivers/gpu/drm/test/test-drm_dp_mst_helper.c |  429 +++
>>   drivers/gpu/drm/test/test-drm_format.c        |  356 +++
>>   .../test-drm_framebuffer.c                    |  109 +-
>>   drivers/gpu/drm/test/test-drm_mm.c            | 2426 ++++++++++++++++
>>   drivers/gpu/drm/test/test-drm_plane_helper.c  |  312 +++
>>   drivers/gpu/drm/test/test-drm_rect.c          |  249 ++
>>   drivers/video/Kconfig                         |    4 +
>>   29 files changed, 5558 insertions(+), 5727 deletions(-)
>>   create mode 100644 drivers/gpu/drm/.kunitconfig
>>   delete mode 100644 drivers/gpu/drm/selftests/Makefile
>>   delete mode 100644 drivers/gpu/drm/selftests/drm_cmdline_selftests.h
>>   delete mode 100644 drivers/gpu/drm/selftests/drm_mm_selftests.h
>>   delete mode 100644 drivers/gpu/drm/selftests/drm_modeset_selftests.h
>>   delete mode 100644 drivers/gpu/drm/selftests/drm_selftest.c
>>   delete mode 100644 drivers/gpu/drm/selftests/drm_selftest.h
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_cmdline_parser.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_damage_helper.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_format.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_mm.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_modeset_common.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_modeset_common.h
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_plane_helper.c
>>   delete mode 100644 drivers/gpu/drm/selftests/test-drm_rect.c
>>   create mode 100644 drivers/gpu/drm/test/Makefile
>>   create mode 100644 drivers/gpu/drm/test/test-drm_cmdline_parser.c
>>   create mode 100644 drivers/gpu/drm/test/test-drm_damage_helper.c
>>   create mode 100644 drivers/gpu/drm/test/test-drm_dp_mst_helper.c
>>   create mode 100644 drivers/gpu/drm/test/test-drm_format.c
>>   rename drivers/gpu/drm/{selftests => test}/test-drm_framebuffer.c (91%)
>>   create mode 100644 drivers/gpu/drm/test/test-drm_mm.c
>>   create mode 100644 drivers/gpu/drm/test/test-drm_plane_helper.c
>>   create mode 100644 drivers/gpu/drm/test/test-drm_rect.c
>>
>> --
>> 2.34.1
>>