On 03/03/2025 13:48, Philipp Stanner wrote:
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.
To be concluded in that sub-thread.
*test) +struct drm_mock_scheduler * +drm_mock_new_scheduler(struct kunit *test, + long timeout)fits on one line
Oops, done.
{ 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 0x1Why's that defined here within the struct's body? Better move it above
Makes it clear to which field flags apply. I can move if you insist but then I would rename it to something like DRM_MOCK_SCHED_FLAGS_JOB_TIMEDOUT.
Regards, Tvrtko
+ 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);
