Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Moving `trace_pvr_job_submit_fw(frag_job)` after the hardware kick introduces a race condition that can result in a Use-After-Free on `frag_job`. --
--- Patch [1]: [PATCH] drm/imagination: Update the trace point pvr_job_submit_fw() --- commit 290fb87e4c730ddd81be1c9d52004f26f109de14 Author: Brajesh Gupta <[email protected]> drm/imagination: Update the trace point pvr_job_submit_fw() This commit moves the pvr_job_submit_fw() trace point after writing the command to the Kernel circular buffer to better represent submission to the firmware. > diff --git a/drivers/gpu/drm/imagination/pvr_queue.c > b/drivers/gpu/drm/imagination/pvr_queue.c > --- a/drivers/gpu/drm/imagination/pvr_queue.c > +++ b/drivers/gpu/drm/imagination/pvr_queue.c [ ... ] > @@ -802,6 +800,9 @@ static struct dma_fence *pvr_queue_run_job(struct > drm_sched_job *sched_job) > job->hwrt, > frag_job->fw_ccb_cmd_type == > > ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR); > + > + trace_pvr_job_submit_fw(geom_job); > + trace_pvr_job_submit_fw(frag_job); [Severity: High] Could this lead to a use-after-free on frag_job if the GPU finishes the combined kick quickly? If pvr_queue_run_job() issues the combined hardware kick via pvr_cccb_send_kccb_combined_kick(), the GPU begins execution immediately. If the thread gets preempted or the GPU simply completes the jobs very quickly, the done_fence could be signaled via interrupts before the tracepoint is reached. If the fragment queue's scheduler thread wakes up, processes the completion, and frees frag_job, then geom_job's scheduler thread would resume and call trace_pvr_job_submit_fw(frag_job) on freed memory, because geom_job doesn't appear to hold a reference on frag_job. Should we take an extra reference to frag_job before the hardware kick, or perhaps move the tracepoint back before the kick? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
