On Fri, 2025-02-21 at 12:09 +0000, Tvrtko Ursulin wrote:
> Add a very simple timeout test which submits a single job and
> verifies
> that the timeout handling will run if the backend failed to complete
> the
> job in time.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
> Cc: Christian König <christian.koe...@amd.com>
> Cc: Danilo Krummrich <d...@kernel.org>
> Cc: Matthew Brost <matthew.br...@intel.com>
> Cc: Philipp Stanner <pha...@kernel.org>
> ---
> .../gpu/drm/scheduler/tests/mock_scheduler.c | 13 +++-
> drivers/gpu/drm/scheduler/tests/sched_tests.h | 6 +-
> drivers/gpu/drm/scheduler/tests/tests_basic.c | 64
> ++++++++++++++++++-
> 3 files changed, 77 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> index d73a9f0337da..427ce29354d0 100644
> --- a/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> +++ b/drivers/gpu/drm/scheduler/tests/mock_scheduler.c
> @@ -193,7 +193,11 @@ static struct dma_fence
> *mock_sched_run_job(struct drm_sched_job *sched_job)
> static enum drm_gpu_sched_stat
> mock_sched_timedout_job(struct drm_sched_job *sched_job)
> {
> - return DRM_GPU_SCHED_STAT_ENODEV;
> + struct drm_mock_sched_job *job =
> drm_sched_job_to_mock_job(sched_job);
> +
> + job->flags |= DRM_MOCK_SCHED_JOB_TIMEDOUT;
> +
> + return DRM_GPU_SCHED_STAT_NOMINAL;
> }
>
> static void mock_sched_free_job(struct drm_sched_job *sched_job)
> @@ -211,17 +215,20 @@ static const struct drm_sched_backend_ops
> drm_mock_scheduler_ops = {
> * drm_mock_new_scheduler - Create a new mock scheduler
> *
> * @test: KUnit test owning the job
> + * @timeout: Job timeout to set
> *
> * Returns: New mock scheduler with allocation managed by the test
> */
> -struct drm_mock_scheduler *drm_mock_new_scheduler(struct kunit
-> …_scheduler_new()
As said in the previous thread, I think that if you want to introduce
the new prefix drm_mock_, then it should always be followed by the DRM
subcomponent, so that it can be congruent across DRM.
> *test)
> +struct drm_mock_scheduler *
> +drm_mock_new_scheduler(struct kunit *test,
> + long timeout)
fits on one line
> {
> struct drm_sched_init_args args = {
> .ops = &drm_mock_scheduler_ops,
> .num_rqs = DRM_SCHED_PRIORITY_COUNT,
> .credit_limit = U32_MAX,
> .hang_limit = UINT_MAX,
> - .timeout = MAX_SCHEDULE_TIMEOUT,
> + .timeout = timeout,
> .name = "drm-mock-scheduler",
> };
> struct drm_mock_scheduler *sched;
> diff --git a/drivers/gpu/drm/scheduler/tests/sched_tests.h
> b/drivers/gpu/drm/scheduler/tests/sched_tests.h
> index 0614bc901dd1..258fec748ca7 100644
> --- a/drivers/gpu/drm/scheduler/tests/sched_tests.h
> +++ b/drivers/gpu/drm/scheduler/tests/sched_tests.h
> @@ -88,6 +88,9 @@ struct drm_mock_sched_entity {
> struct drm_mock_sched_job {
> struct drm_sched_job base;
>
> +#define DRM_MOCK_SCHED_JOB_TIMEDOUT 0x1
Why's that defined here within the struct's body? Better move it above
P.
> + unsigned long flags;
> +
> struct list_head link;
> struct hrtimer timer;
>
> @@ -118,7 +121,8 @@ drm_sched_job_to_mock_job(struct drm_sched_job
> *sched_job)
> return container_of(sched_job, struct drm_mock_sched_job,
> base);
> };
>
> -struct drm_mock_scheduler *drm_mock_new_scheduler(struct kunit
> *test);
> +struct drm_mock_scheduler *drm_mock_new_scheduler(struct kunit
> *test,
> + long timeout);
> void drm_mock_scheduler_fini(struct drm_mock_scheduler *sched);
> unsigned int drm_mock_sched_advance(struct drm_mock_scheduler
> *sched,
> unsigned int num);
> diff --git a/drivers/gpu/drm/scheduler/tests/tests_basic.c
> b/drivers/gpu/drm/scheduler/tests/tests_basic.c
> index c12368a22a39..023143b80d78 100644
> --- a/drivers/gpu/drm/scheduler/tests/tests_basic.c
> +++ b/drivers/gpu/drm/scheduler/tests/tests_basic.c
> @@ -11,7 +11,7 @@
>
> static int drm_sched_basic_init(struct kunit *test)
> {
> - test->priv = drm_mock_new_scheduler(test);
> + test->priv = drm_mock_new_scheduler(test,
> MAX_SCHEDULE_TIMEOUT);
>
> return 0;
> }
> @@ -23,6 +23,13 @@ static void drm_sched_basic_exit(struct kunit
> *test)
> drm_mock_scheduler_fini(sched);
> }
>
> +static int drm_sched_timeout_init(struct kunit *test)
> +{
> + test->priv = drm_mock_new_scheduler(test, HZ);
> +
> + return 0;
> +}
> +
> static void drm_sched_basic_submit(struct kunit *test)
> {
> struct drm_mock_scheduler *sched = test->priv;
> @@ -194,4 +201,57 @@ static struct kunit_suite drm_sched_basic = {
> .test_cases = drm_sched_basic_tests,
> };
>
> -kunit_test_suite(drm_sched_basic);
> +static void drm_sched_basic_timeout(struct kunit *test)
> +{
> + struct drm_mock_scheduler *sched = test->priv;
> + struct drm_mock_sched_entity *entity;
> + struct drm_mock_sched_job *job;
> + bool done;
> +
> + /*
> + * Submit a single job against a scheduler with the timeout
> configured
> + * and verify that the timeout handling will run if the
> backend fails
> + * to complete it in time.
> + */
> +
> + entity = drm_mock_new_sched_entity(test,
> +
> DRM_SCHED_PRIORITY_NORMAL,
> + sched);
> + job = drm_mock_new_sched_job(test, entity);
> +
> + drm_mock_sched_job_submit(job);
> +
> + done = drm_mock_sched_job_wait_scheduled(job, HZ);
> + KUNIT_ASSERT_EQ(test, done, true);
> +
> + done = drm_mock_sched_job_wait_finished(job, HZ / 2);
> + KUNIT_ASSERT_EQ(test, done, false);
> +
> + KUNIT_ASSERT_EQ(test,
> + job->flags & DRM_MOCK_SCHED_JOB_TIMEDOUT,
> + 0);
> +
> + done = drm_mock_sched_job_wait_finished(job, HZ);
> + KUNIT_ASSERT_EQ(test, done, false);
> +
> + KUNIT_ASSERT_EQ(test,
> + job->flags & DRM_MOCK_SCHED_JOB_TIMEDOUT,
> + DRM_MOCK_SCHED_JOB_TIMEDOUT);
> +
> + drm_mock_sched_entity_free(entity);
> +}
> +
> +static struct kunit_case drm_sched_timeout_tests[] = {
> + KUNIT_CASE(drm_sched_basic_timeout),
> + {}
> +};
> +
> +static struct kunit_suite drm_sched_timeout = {
> + .name = "drm_sched_basic_timeout_tests",
> + .init = drm_sched_timeout_init,
> + .exit = drm_sched_basic_exit,
> + .test_cases = drm_sched_timeout_tests,
> +};
> +
> +kunit_test_suites(&drm_sched_basic,
> + &drm_sched_timeout);
