Skip to content

Commit 7e549c6

Browse files
mairacanalpopcornmix
authored andcommitted
drm/v3d: Add job to pending list if the reset was skipped
When a CL/CSD job times out, we check if the GPU has made any progress since the last timeout. If so, instead of resetting the hardware, we skip the reset and let the timer get rearmed. This gives long-running jobs a chance to complete. However, when `timedout_job()` is called, the job in question is removed from the pending list, which means it won't be automatically freed through `free_job()`. Consequently, when we skip the reset and keep the job running, the job won't be freed when it finally completes. This situation leads to a memory leak, as exposed in [1]. Similarly to commit 704d3d6 ("drm/etnaviv: don't block scheduler when GPU is still active"), this patch ensures the job is put back on the pending list when extending the timeout. Cc: [email protected] # 6.0 Link: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12227 [1] Reported-by: Daivik Bhatia <[email protected]> Signed-off-by: Maíra Canal <[email protected]>
1 parent a9b3481 commit 7e549c6

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

drivers/gpu/drm/v3d/v3d_sched.c

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -744,11 +744,6 @@ v3d_gpu_reset_for_timeout(struct v3d_dev *v3d, struct drm_sched_job *sched_job)
744744
return DRM_GPU_SCHED_STAT_NOMINAL;
745745
}
746746

747-
/* If the current address or return address have changed, then the GPU
748-
* has probably made progress and we should delay the reset. This
749-
* could fail if the GPU got in an infinite loop in the CL, but that
750-
* is pretty unlikely outside of an i-g-t testcase.
751-
*/
752747
static enum drm_gpu_sched_stat
753748
v3d_cl_job_timedout(struct drm_sched_job *sched_job, enum v3d_queue q,
754749
u32 *timedout_ctca, u32 *timedout_ctra)
@@ -758,9 +753,16 @@ v3d_cl_job_timedout(struct drm_sched_job *sched_job, enum v3d_queue q,
758753
u32 ctca = V3D_CORE_READ(0, V3D_CLE_CTNCA(q));
759754
u32 ctra = V3D_CORE_READ(0, V3D_CLE_CTNRA(q));
760755

756+
/* If the current address or return address have changed, then the GPU
757+
* has probably made progress and we should delay the reset. This
758+
* could fail if the GPU got in an infinite loop in the CL, but that
759+
* is pretty unlikely outside of an i-g-t testcase.
760+
*/
761761
if (*timedout_ctca != ctca || *timedout_ctra != ctra) {
762762
*timedout_ctca = ctca;
763763
*timedout_ctra = ctra;
764+
765+
list_add(&sched_job->list, &sched_job->sched->pending_list);
764766
return DRM_GPU_SCHED_STAT_NOMINAL;
765767
}
766768

@@ -800,11 +802,13 @@ v3d_csd_job_timedout(struct drm_sched_job *sched_job)
800802
struct v3d_dev *v3d = job->base.v3d;
801803
u32 batches = V3D_CORE_READ(0, V3D_CSD_CURRENT_CFG4(v3d->ver));
802804

803-
/* If we've made progress, skip reset and let the timer get
804-
* rearmed.
805+
/* If we've made progress, skip reset, add the job to the pending
806+
* list, and let the timer get rearmed.
805807
*/
806808
if (job->timedout_batches != batches) {
807809
job->timedout_batches = batches;
810+
811+
list_add(&sched_job->list, &sched_job->sched->pending_list);
808812
return DRM_GPU_SCHED_STAT_NOMINAL;
809813
}
810814

0 commit comments

Comments
 (0)