Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Calling `dma_fence_is_signaled(s_job->s_fence->parent)` can result in a NULL pointer dereference because the parent fence may not be initialized when the timeout handler is invoked. --
commit 3504f98145fc1193c182b77188d8026b264fe31f Author: Brajesh Gupta <[email protected]> drm/imagination: Don't timeout job if its fence has been signaled This commit adds a check in the timeout handler to verify if the job's fence has been signaled. This prevents unnecessary hang recovery when the job has already completed. Link: https://patch.msgid.link/[email protected] > diff --git a/drivers/gpu/drm/imagination/pvr_queue.c > b/drivers/gpu/drm/imagination/pvr_queue.c > index fc415dd0d7a73..8d5629ea3e520 100644 > --- a/drivers/gpu/drm/imagination/pvr_queue.c > +++ b/drivers/gpu/drm/imagination/pvr_queue.c [ ... ] > @@ -814,6 +816,9 @@ pvr_queue_timedout_job(struct drm_sched_job *s_job) > struct pvr_job *job; > u32 job_count = 0; > > + if (dma_fence_is_signaled(s_job->s_fence->parent)) > + return DRM_GPU_SCHED_STAT_NO_HANG; > + [Severity: High] Can this result in a NULL pointer dereference? In drivers/gpu/drm/imagination/pvr_queue.c:pvr_queue_timedout_job(), if the timeout worker executes concurrently while the driver's run_job() callback is still blocked (e.g., due to power management or firmware delays), drm_sched_fence_scheduled() will not have executed yet. In that scenario, s_job->s_fence->parent is uninitialized and remains NULL. Passing this uninitialized parent pointer directly to dma_fence_is_signaled() would unconditionally dereference it, which could cause a regression if a timeout fires before job submission completes. > dev_err(sched->dev, "Job timeout\n"); > > /* Before we stop the scheduler, make sure the queue is out of any > list, so -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
