[PATCH 05/12] drm/xe/tests: Allow deferred function call during KUnit test

Lucas De Marchi lucas.demarchi at intel.com
Tue Aug 20 13:27:08 UTC 2024


On Tue, Aug 20, 2024 at 12:23:43PM GMT, Michal Wajdeczko wrote:
>
>
>On 19.08.2024 23:38, Lucas De Marchi wrote:
>> On Fri, Aug 09, 2024 at 06:51:52PM GMT, Michal Wajdeczko wrote:
>>> For some advanced test cases we might want to simulate external
>>> activity that will stimulate function under test. Add set of
>>> helper functions to implement such requirement. Usage example:
>>>
>>>  static void foo(struct xe_device *xe)
>>>  {
>>>      WRITE_ONCE(xe->foo, 1);
>>>  }
>>>
>>>  static void foo_test(struct kunit *test)
>>>  {
>>>      struct xe_device *xe = test->priv;
>>>
>>>      xe_kunit_helper_delayed_call(test, HZ / 2, foo, xe);
>>>
>>>      KUNIT_EXPECT_EQ(test, 0, READ_ONCE(xe->foo));
>>>      schedule_timeout_uninterruptible(HZ);
>>>      KUNIT_EXPECT_EQ(test, 1, READ_ONCE(xe->foo));
>>>  }
>>>
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
>>> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>>> ---
>>> drivers/gpu/drm/xe/tests/xe_kunit_helpers.c | 148 ++++++++++++++++++++
>>> drivers/gpu/drm/xe/tests/xe_kunit_helpers.h |  38 +++++
>>> 2 files changed, 186 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>>> b/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>>> index bc5156966ce9..8fe1a0918b32 100644
>>> --- a/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>>> +++ b/drivers/gpu/drm/xe/tests/xe_kunit_helpers.c
>>> @@ -127,3 +127,151 @@ int
>>> xe_kunit_helper_xe_device_live_test_init(struct kunit *test)
>>>     return 0;
>>> }
>>> EXPORT_SYMBOL_IF_KUNIT(xe_kunit_helper_xe_device_live_test_init);
>>> +
>>> +struct xe_kunit_call {
>>> +    struct delayed_work work;
>>> +    struct kunit *test;
>>> +    struct {
>>> +        int (*int_gt)(struct xe_gt *gt);
>>> +        int (*int_gt_uint)(struct xe_gt *gt, unsigned int n);
>>> +        void (*void_gt)(struct xe_gt *gt);
>>> +        void (*void_gt_uint)(struct xe_gt *gt, unsigned int n);
>>> +        void (*void_xe)(struct xe_device *xe);
>>
>> I think the only reason why you're adding this in xe is because you are
>> using xe-specific types. This could rather be done generically by
>> always using int delayed_func(void *data). Then you can simply add it to
>> kunit and get it reviewed there.
>
>while some generic functionality could be always extracted and moved to
>the kunit, for easier use in Xe tests likely we will still need some
>wrappers with xe-specific types, so do we really always need to start
>with trying enhance generic kunit first and then wait for acceptance
>until we can use such extension in our test code?

the other option is that we keep creating a mini-subsystem inside xe and
if we don't push back, the generic part is never done as people move on
to other tasks.

>
>IMO this could be in parallel, so in kunit review we can point how this
>is already being used..

Yes, sometimes it's better to do the concrete thing and
use that to advocate for the generic feature, but we should balance on
how much we want to maintain in the mid/long term.

>
>anyway, will try to split that feature into 'generic' and 'xe'

if the generic part is acceptable to kunit maintainers, we can always
ask to merge that via drm-xe with their acks.

Lucas De Marchi

>
>>
>> Also thinking that the example you added in the commit message is likely
>> suggesting the wrong type of test, that a) sleep too much and b) tend to
>> be noisy
>
>only tests longer than 2s are considered SLOW ;)
>
>and HZ was just picked to have an example small
>
>and you will not see any noise if test PASSES
>
>>
>> Lucas De Marchi
>>
>>> +    } func;
>>> +    struct {
>>> +        struct xe_device *xe;
>>> +        struct xe_gt *gt;
>>> +        unsigned int n;
>>> +    } args;
>>> +};
>>> +
>>> +static void call_work_func(struct work_struct *work)
>>> +{
>>> +    struct xe_kunit_call *call = container_of(work, typeof(*call),
>>> work.work);
>>> +
>>> +    if (call->func.void_xe) {
>>> +        kunit_info(call->test, "calling %ps(xe)\n", call->func.void_xe);
>>> +        call->func.void_xe(call->args.xe);
>>> +        kunit_info(call->test, "%ps(xe) completed\n",
>>> call->func.void_xe);
>>> +    } else if (call->func.void_gt) {
>>> +        kunit_info(call->test, "%ps(gt)\n", call->func.void_gt);
>>> +        call->func.void_gt(call->args.gt);
>>> +        kunit_info(call->test, "%ps(gt) completed\n",
>>> call->func.void_gt);
>>> +    } else if (call->func.int_gt_uint) {
>>> +        int ret;
>>> +
>>> +        kunit_info(call->test, "calling %ps(gt,%u)\n",
>>> +               call->func.int_gt_uint, call->args.n);
>>> +        ret = call->func.int_gt_uint(call->args.gt, call->args.n);
>>> +        if (ret < 0) {
>>> +            kunit_info(call->test, "%ps(gt,%u) reported error %pe\n",
>>> +                   call->func.int_gt_uint, call->args.n, ERR_PTR(ret));
>>> +        } else {
>>> +            kunit_info(call->test, "%ps(gt,%u) returned %d\n",
>>> +                   call->func.int_gt_uint, call->args.n, ret);
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static struct xe_kunit_call *prepare_call(struct kunit *test)
>>> +{
>>> +    struct xe_kunit_call *call;
>>> +
>>> +    call = kunit_kzalloc(test, sizeof(*call), GFP_KERNEL);
>>> +    KUNIT_ASSERT_NOT_NULL(test, call);
>>> +
>>> +    INIT_DELAYED_WORK(&call->work, call_work_func);
>>> +    call->test = test;
>>> +
>>> +    return call;
>>> +}
>>> +
>>> +KUNIT_DEFINE_ACTION_WRAPPER(cancel_call, cancel_delayed_work_sync,
>>> struct delayed_work *);
>>> +
>>> +static void queue_call(struct kunit *test, struct xe_kunit_call
>>> *call, unsigned long delay)
>>> +{
>>> +    queue_delayed_work(system_wq, &call->work, delay);
>>> +    KUNIT_ASSERT_EQ(test, 0, kunit_add_action_or_reset(test,
>>> cancel_call, &call->work));
>>> +}
>>> +
>>> +/**
>>> + * xe_kunit_helper_delayed_call_void_xe_device - Queue a function
>>> call during KUnit test
>>> + * @test: the &kunit test case
>>> + * @delay: number of jiffies to wait before queueing
>>> + * @func: the &xe_device function to call
>>> + * @xe: the &xe_device argument
>>> + *
>>> + * This function uses KUNIT_ASSERT to detect any failures.
>>> + */
>>> +void xe_kunit_helper_delayed_call_void_xe_device(struct kunit *test,
>>> +                         unsigned long delay,
>>> +                         void (*func)(struct xe_device *xe),
>>> +                         struct xe_device *xe)
>>> +{
>>> +    struct xe_kunit_call *call = prepare_call(test);
>>> +
>>> +    KUNIT_ASSERT_NOT_NULL(test, func);
>>> +    KUNIT_ASSERT_NOT_NULL(test, xe);
>>> +
>>> +    call->func.void_xe = func;
>>> +    call->args.xe = xe;
>>> +
>>> +    return queue_call(test, call, delay);
>>> +}
>>> +EXPORT_SYMBOL_IF_KUNIT(xe_kunit_helper_delayed_call_void_xe_device);
>>> +
>>> +/**
>>> + * xe_kunit_helper_delayed_call_void_xe_gt - Queue a function call
>>> during KUnit test
>>> + * @test: the &kunit test case
>>> + * @delay: number of jiffies to wait before queueing
>>> + * @func: the &xe_gt function to call
>>> + * @gt: the &xe_gt function argument
>>> + *
>>> + * This function uses KUNIT_ASSERT to detect any failures.
>>> + */
>>> +void xe_kunit_helper_delayed_call_void_xe_gt(struct kunit *test,
>>> +                         unsigned long delay,
>>> +                         void (*func)(struct xe_gt *gt),
>>> +                         struct xe_gt *gt)
>>> +{
>>> +    struct xe_kunit_call *call = prepare_call(test);
>>> +
>>> +    KUNIT_ASSERT_NOT_NULL(test, func);
>>> +    KUNIT_ASSERT_NOT_NULL(test, gt);
>>> +
>>> +    call->func.void_gt = func;
>>> +    call->args.gt = gt;
>>> +
>>> +    return queue_call(test, call, delay);
>>> +}
>>> +EXPORT_SYMBOL_IF_KUNIT(xe_kunit_helper_delayed_call_void_xe_gt);
>>> +
>>> +/**
>>> + * xe_kunit_helper_delayed_call_int_xe_gt_uint - Queue a function
>>> call during KUnit test
>>> + * @test: the &kunit test case
>>> + * @delay: number of jiffies to wait before queueing
>>> + * @func: the &xe_gt function to call
>>> + * @gt: the &xe_gt function argument
>>> + * @n: the function argument
>>> + *
>>> + * This function uses KUNIT_ASSERT to detect any failures.
>>> + */
>>> +void xe_kunit_helper_delayed_call_int_xe_gt_uint(struct kunit *test,
>>> +                         unsigned long delay,
>>> +                         int (*func)(struct xe_gt *gt,
>>> +                                 unsigned int vfid),
>>> +                         struct xe_gt *gt,
>>> +                         unsigned int n)
>>> +{
>>> +    struct xe_kunit_call *call = prepare_call(test);
>>> +
>>> +    KUNIT_ASSERT_NOT_NULL(test, func);
>>> +    KUNIT_ASSERT_NOT_NULL(test, gt);
>>> +
>>> +    call->func.int_gt_uint = func;
>>> +    call->args.gt = gt;
>>> +    call->args.n = n;
>>> +
>>> +    return queue_call(test, call, delay);
>>> +}
>>> +EXPORT_SYMBOL_IF_KUNIT(xe_kunit_helper_delayed_call_int_xe_gt_uint);
>>> diff --git a/drivers/gpu/drm/xe/tests/xe_kunit_helpers.h
>>> b/drivers/gpu/drm/xe/tests/xe_kunit_helpers.h
>>> index 83665f7b1254..ec5287947ee4 100644
>>> --- a/drivers/gpu/drm/xe/tests/xe_kunit_helpers.h
>>> +++ b/drivers/gpu/drm/xe/tests/xe_kunit_helpers.h
>>> @@ -9,6 +9,7 @@
>>> struct device;
>>> struct kunit;
>>> struct xe_device;
>>> +struct xe_gt;
>>>
>>> struct xe_device *xe_kunit_helper_alloc_xe_device(struct kunit *test,
>>>                           struct device *dev);
>>> @@ -16,4 +17,41 @@ int xe_kunit_helper_xe_device_test_init(struct
>>> kunit *test);
>>>
>>> int xe_kunit_helper_xe_device_live_test_init(struct kunit *test);
>>>
>>> +void xe_kunit_helper_delayed_call_void_xe_device(struct kunit *test,
>>> +                         unsigned long delay,
>>> +                         void (*func)(struct xe_device *xe),
>>> +                         struct xe_device *xe);
>>> +
>>> +void xe_kunit_helper_delayed_call_void_xe_gt(struct kunit *test,
>>> +                         unsigned long delay,
>>> +                         void (*func)(struct xe_gt *gt),
>>> +                         struct xe_gt *gt);
>>> +
>>> +void xe_kunit_helper_delayed_call_int_xe_gt_uint(struct kunit *test,
>>> +                         unsigned long delay,
>>> +                         int (*func)(struct xe_gt *gt,
>>> +                                 unsigned int vfid),
>>> +                         struct xe_gt *gt,
>>> +                         unsigned int n);
>>> +
>>> +/**
>>> + * xe_kunit_helper_delayed_call - Queue a function call during KUnit
>>> test
>>> + * @test: the &kunit test case
>>> + * @delay: number of jiffies to wait before queueing
>>> + * @func: the &xe_device or &xe_gt function to call
>>> + * @args: the &xe_device or &xe_gt and other function arguments
>>> + *
>>> + * This is a helper macro that compiles into dedicated function call
>>> based on
>>> + * the provided argument types.
>>> + */
>>> +#define xe_kunit_helper_delayed_call(test, delay, func, args...)    \
>>> +    _Generic(func,                            \
>>> +         void (*)(struct xe_device *) :                \
>>> +            xe_kunit_helper_delayed_call_void_xe_device,    \
>>> +         void (*)(struct xe_gt *) :                \
>>> +            xe_kunit_helper_delayed_call_void_xe_gt,    \
>>> +         int (*)(struct xe_gt *, unsigned int) :        \
>>> +            xe_kunit_helper_delayed_call_int_xe_gt_uint    \
>>> +    )(test, delay, func, args)
>>> +
>>> #endif
>>> -- 
>>> 2.43.0
>>>


More information about the Intel-xe mailing list