Skip to contentSkip to conversation

drm/sched: Do not overaccount job runtime

PATCH New Tvrtko Ursulin 1 participant 1 message Nov 11

Subsystems (3)
DRM drivers DRM misc DRM GPU scheduler
lore
Share & tools
lore

Tvrtko Ursulin

author
#0 lore raw

PATCH 1/1 drm/sched: Do not overaccount job runtime

Currently the starting timestamp of the next job on the timeline is adjusted to the end of a preceding job. This works if the next job has been submitted while the preceding job was still executing, but not if it was submitted in the window between it finishing and free job work running the timestamp adjustment.

This can happen because the run job and free job work items share the same ordered workqueue and run job can easily submit the next job before the free job gets a chance to run. This over accounting window is present especially with single credit schedulers, but not limited too.

Fix it by only adjusting the starting timestamp if it is not already after the finish time of the preceding job.

While at it move the logic to a helper to keep indendation at bay.

Signed-off-by Tvrtko Ursulin <[email protected]>
Fixes 1774baa64f93 ("drm/scheduler: Change scheduled fence track v2")
Cc Christian König <[email protected]>
Cc Danilo Krummrich <[email protected]>
Cc Matthew Brost <[email protected]>
Cc Philipp Stanner <[email protected]>
---
 drivers/gpu/drm/scheduler/sched_main.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)
drivers/gpu/drm/scheduler/sched_main.c +19 -5
index a030061d0390..136880fc193d 100644
@@ -898,6 +898,24 @@ drm_sched_select_entity(struct drm_gpu_scheduler *sched)
 	return IS_ERR(entity) ? NULL : entity;
 }
 
+static void
+adjust_next_job_start(struct drm_sched_job *job, struct drm_sched_job *next)
+{
+	struct dma_fence *next_scheduled = &next->s_fence->scheduled;
+	ktime_t ts;
+
+	if (!test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT, &next_scheduled->flags))
+		return;
+
+	/*
+	 * If the next job has been submitted while the previous one was still
+	 * running adjust it's starting timestamp to avoid accounting overlap.
+	 */
+	ts = dma_fence_timestamp(&job->s_fence->finished);
+	if (ktime_after(ts, next_scheduled->timestamp))
+		next_scheduled->timestamp = ts;
+}
+
 /**
  * drm_sched_get_finished_job - fetch the next finished job to be destroyed
  *

@@ -929,11 +947,7 @@ drm_sched_get_finished_job(struct drm_gpu_scheduler *sched)
 						typeof(*next), list);
 		if (next) {
 			/* make the scheduled timestamp more accurate */
-			if (test_bit(DMA_FENCE_FLAG_TIMESTAMP_BIT,
-				     &next->s_fence->scheduled.flags))
-				next->s_fence->scheduled.timestamp =
-					dma_fence_timestamp(&job->s_fence->finished);
-
+			adjust_next_job_start(job, next);
 			/* start TO timer for next job */
 			drm_sched_start_timeout(sched);
 		}
Signature · 1 line
2.56.0