All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/imagination: Update the trace point pvr_job_submit_fw()
@ 2026-07-22 11:25 Brajesh Gupta
  2026-07-22 11:42 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Brajesh Gupta @ 2026-07-22 11:25 UTC (permalink / raw)
  To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Matt Coster,
	Donald Robson, Alexandru Dadu, Frank Binns
  Cc: imagination, dri-devel, linux-kernel, Brajesh Gupta

Trace point pvr_job_submit_fw() is used to trace job submission to
the FW. Currently it is recorded when a command is written to the Client
circular buffer.

Move trace recording after writing command to the Kernel circular buffer to
better represent command submission to the FW.

Fixes: c1079aebb4de ("drm/imagination: Add support for trace points")
Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
 drivers/gpu/drm/imagination/pvr_queue.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index 941c017399fc..d13726401d92 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -726,8 +726,6 @@ static void pvr_queue_submit_job_to_cccb(struct pvr_job *job)
 		cmd->partial_render_geom_frag_fence.value = job->done_fence->seqno - 1;
 	}
 
-	trace_pvr_job_submit_fw(job);
-
 	/* Submit job to FW */
 	pvr_cccb_write_command_with_header(cccb, job->fw_ccb_cmd_type, job->cmd_len, job->cmd,
 					   job->id, job->id);
@@ -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);
 	} else {
 		struct pvr_queue *queue = container_of(job->base.sched,
 						       struct pvr_queue, scheduler);
@@ -809,6 +810,8 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
 		pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
 					pvr_context_get_fw_addr(job->ctx) + queue->ctx_offset,
 					job->hwrt);
+
+		trace_pvr_job_submit_fw(job);
 	}
 
 	return dma_fence_get(job->done_fence);

---
base-commit: e5320be8a585a9286f231305d0d443d59c24e46c
change-id: 20260722-b4-tracepoint-fix-0fc961d7023d

Best regards,
-- 
Brajesh Gupta <brajesh.gupta@imgtec.com>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/imagination: Update the trace point pvr_job_submit_fw()
  2026-07-22 11:25 [PATCH] drm/imagination: Update the trace point pvr_job_submit_fw() Brajesh Gupta
@ 2026-07-22 11:42 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-22 11:42 UTC (permalink / raw)
  To: Brajesh Gupta; +Cc: dri-devel

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 <brajesh.gupta@imgtec.com>

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/20260722-b4-tracepoint-fix-v1-1-f75747fa591a@imgtec.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-22 11:42 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-22 11:25 [PATCH] drm/imagination: Update the trace point pvr_job_submit_fw() Brajesh Gupta
2026-07-22 11:42 ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.