Re: [PATCH v4 06/10] drm/xe/kunit: Restore test->priv when done with fake xe device

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 15.12.2023 23:11, Lucas De Marchi wrote:
> On Fri, Dec 15, 2023 at 05:24:58PM +0100, Michal Wajdeczko wrote:
>> Current KUnit implementation does not reset test->priv in case of
>> parametrized tests and that may lead to wrongly treat our output
>> pointer to fake xe_device from first call as input pointer with
>> xe_pci_fake_data on subsequent calls.  Restore test->priv to
>> original value to avoid invalid access.
> 
> but wouldn't each parameterized test pass a different a data?

test->priv prepared by suit.init() should be the same
only test->param_value will change on each param iteration

> 
> not sure I'm following why this is a xe issue. Are you fixing it in xe
> while should be fixed in kunit?

who said I'm not fixing that in kunit ;)

[1]
https://lore.kernel.org/linux-kselftest/20231215151327.1835-1-michal.wajdeczko@intel.com/

Michal

> 
> Lucas De Marchi
> 
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> ---
>> drivers/gpu/drm/xe/tests/xe_kunit_helpers.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>> b/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>> index 6d72dbf06139..fefe79b3b75a 100644
>> --- a/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>> +++ b/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>> @@ -41,6 +41,13 @@ struct xe_device
>> *xe_kunit_helper_alloc_xe_device(struct kunit *test,
>> }
>> EXPORT_SYMBOL_IF_KUNIT(xe_kunit_helper_alloc_xe_device);
>>
>> +static void kunit_action_restore_priv(void *priv)
>> +{
>> +    struct kunit *test = kunit_get_current_test();
>> +
>> +    test->priv = priv;
>> +}
>> +
>> /**
>>  * xe_kunit_helper_xe_device_test_init - Prepare a &xe_device for a
>> KUnit test.
>>  * @test: the &kunit where this fake &xe_device will be used
>> @@ -74,6 +81,9 @@ int xe_kunit_helper_xe_device_test_init(struct kunit
>> *test)
>>     err = xe_pci_fake_device_init(xe);
>>     KUNIT_ASSERT_EQ(test, err, 0);
>>
>> +    err = kunit_add_action_or_reset(test, kunit_action_restore_priv,
>> test->priv);
>> +    KUNIT_ASSERT_EQ(test, err, 0);
>> +
>>     test->priv = xe;
>>     return 0;
>> }
>> -- 
>> 2.25.1
>>