[PATCH v5 0/4] Improve test coverage of TTM

[Karolina Stolarek]

On 17.10.2023 14:10, Christian König wrote:
> Am 17.10.23 um 14:06 schrieb Karolina Stolarek:
>> On 17.10.2023 13:57, Christian König wrote:
>>>
>>>
>>> Am 17.10.23 um 13:22 schrieb Somalapuram, Amaranath:
>>>>
>>>> On 10/17/2023 4:35 PM, Karolina Stolarek wrote:
>>>>> Hi Amaranath,
>>>>>
>>>>> On 16.10.2023 15:08, Somalapuram, Amaranath wrote:
>>>>>>
>>>>>> On 10/16/2023 2:22 PM, Karolina Stolarek wrote:
>>>>>>> Add tests for building blocks of the TTM subsystem, such as 
>>>>>>> ttm_resource,
>>>>>>> ttm_resource_manager, ttm_tt and ttm_buffer_object. This series 
>>>>>>> covers
>>>>>>> basic functions such as initialization, allocation and clean-up 
>>>>>>> of each
>>>>>>> struct. Testing of ttm_buffer_object also includes locking and 
>>>>>>> unlocking
>>>>>>> the object for validation, with special scenarios such as an 
>>>>>>> interrupted
>>>>>>> wait or deadlock.
>>>>>>>
>>>>>>> Some of the test cases check the bulk move mechanism and how it 
>>>>>>> interacts
>>>>>>> with pinned buffers. This is to be seen if we want to add 
>>>>>>> dedicated testing
>>>>>>> for bulk move or not. The resource allocation subtests use 
>>>>>>> ttm_sys_manager
>>>>>>> for now. Resources that don't use system memory will be 
>>>>>>> indirectly tested
>>>>>>> via tests for ttm_bo_validate()/ttm_bo_init_validate(), using a mock
>>>>>>> resource manager.
>>>>>>>
>>>>>>> Use kunit_tool script to manually run all the tests:
>>>>>>>
>>>>>>> $ ./tools/testing/kunit/kunit.py run 
>>>>>>> --kunitconfig=drivers/gpu/drm/ttm/tests
>>>>>>>
>>>>>>> To build a kernel with TTM KUnit tests, first enable 
>>>>>>> CONFIG_KUNIT, and
>>>>>>> then CONFIG_DRM_TTM_KUNIT_TEST.
>>>>>> Tested [PATCH v4 0/4] Improve test coverage of TTM patches on AMD 
>>>>>> platform, looks good.
>>>>>
>>>>> Many thanks for taking a look.
>>>>>
>>>>> Do I understand correctly that your Tested-by would apply to all 
>>>>> the patches here?
>>>>>
>>>> Yes, I have tested all the 4 patches together.
>>>
>>> No time for an in deep review, but I'm going to pick this up and push 
>>> it to drm-misc-next.
>>
>> Oh! Could you at least take a look at ttm_bo_reserve_deadlock and/or 
>> interrupted subtests? I'm not 100% sure if my solution is right.
> 
> Than this will have to wait till next week when I have some more time.

That's fine, I think. I still have to tie up loose ends in my 
ttm_bo_validate testing (with no eviction cases), so this will keep me 
busy for a couple more days.

All the best,
Karolina

> 
> Christian.
> 
>>
>> Many thanks,
>> Karolina
>>
>>>
>>> Thanks,
>>> Christian.
>>>
>>>>
>>>> Regards,
>>>> S.Amarnath
>>>>> All the best,
>>>>> Karolina
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> S.Amarnath
>>>>>>> Many thanks,
>>>>>>> Karolina
>>>>>>>
>>>>>>> v5:
>>>>>>>    - Actually use the page_flags parameter in ttm_tt_simple_create()
>>>>>>>
>>>>>>> v4:
>>>>>>>    - First unreserve the object before calling ww_acquire_fini() in
>>>>>>>      ttm_bo_reserve_double_resv subtest
>>>>>>>    - Silence lockdep in ttm_bo_reserve_deadlock subtest (open to 
>>>>>>> suggestions
>>>>>>>      how to fix it in a different way)
>>>>>>>    - Use a genuine GEM object in ttm_buffer_object instead of an 
>>>>>>> empty one
>>>>>>>
>>>>>>> v3:
>>>>>>>    - Instead of modifying the main TTM Makefile, use
>>>>>>>      EXPORT_SYMBOL_FOR_TESTS_ONLY() macro for symbols that are 
>>>>>>> tested but
>>>>>>>      not widely exported. Thanks to this change, TTM tests can be 
>>>>>>> built
>>>>>>>      as modules, even when non-exported functions are used
>>>>>>>    - Change the description of a patch that fixes 
>>>>>>> ttm_pool_pre_populated()
>>>>>>>
>>>>>>> v2:
>>>>>>>    - Remove Makefile for KUnit tests and move the definitions to the
>>>>>>>      TTM's one
>>>>>>>    - Switch on CONFIG_DRM_TTM_KUNIT_TEST=m so the tests and TTM 
>>>>>>> module
>>>>>>>      are built as one. This allows building the tests as a 
>>>>>>> module, even
>>>>>>>      if it uses functions that are not exported
>>>>>>>    - Fix ttm_pool_pre_populated(); a wrong flag was passed to
>>>>>>>      ttm_tt_kunit_init() function
>>>>>>>
>>>>>>> Karolina Stolarek (4):
>>>>>>>    drm/ttm/tests: Add tests for ttm_resource and ttm_sys_man
>>>>>>>    drm/ttm/tests: Add tests for ttm_tt
>>>>>>>    drm/ttm/tests: Add tests for ttm_bo functions
>>>>>>>    drm/ttm/tests: Fix argument in ttm_tt_kunit_init()
>>>>>>>
>>>>>>>   drivers/gpu/drm/ttm/tests/Makefile            |   3 +
>>>>>>>   drivers/gpu/drm/ttm/tests/ttm_bo_test.c       | 619 
>>>>>>> ++++++++++++++++++
>>>>>>>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.c |  51 +-
>>>>>>>   drivers/gpu/drm/ttm/tests/ttm_kunit_helpers.h |   4 +
>>>>>>>   drivers/gpu/drm/ttm/tests/ttm_pool_test.c     |   3 +-
>>>>>>>   drivers/gpu/drm/ttm/tests/ttm_resource_test.c | 335 ++++++++++
>>>>>>>   drivers/gpu/drm/ttm/tests/ttm_tt_test.c       | 295 +++++++++
>>>>>>>   drivers/gpu/drm/ttm/ttm_resource.c            |   3 +
>>>>>>>   drivers/gpu/drm/ttm/ttm_tt.c                  |   3 +
>>>>>>>   9 files changed, 1313 insertions(+), 3 deletions(-)
>>>>>>>   create mode 100644 drivers/gpu/drm/ttm/tests/ttm_bo_test.c
>>>>>>>   create mode 100644 drivers/gpu/drm/ttm/tests/ttm_resource_test.c
>>>>>>>   create mode 100644 drivers/gpu/drm/ttm/tests/ttm_tt_test.c
>>>>>>>
>>>
>