* [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()
@ 2026-06-30 10:03 Brajesh Gupta
2026-06-30 13:53 ` Alessio Belle
0 siblings, 1 reply; 3+ messages in thread
From: Brajesh Gupta @ 2026-06-30 10:03 UTC (permalink / raw)
To: Frank Binns, Matt Coster, Alessio Belle, Alexandru Dadu,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Boris Brezillon
Cc: dri-devel, linux-kernel, stable, Brajesh Gupta
Call sequence of double call:
pvr_context_destroy
pvr_context_kill_queues
pvr_queue_kill
drm_sched_entity_destroy
drm_sched_entity_fini // here
pvr_context_put
kref_put(..., pvr_context_release)
pvr_context_destroy_queues
pvr_queue_destroy
drm_sched_entity_fini // here
Call to drm_sched_entity_destroy() from pvr_context_kill_queues() calls
drm_sched_entity_flush() + drm_sched_entity_fini().
drm_sched_entity_flush() ensures all pending jobs are completed and
drm_sched_entity_fini() ensures no further submission is allowed as
per expectation from pvr_context_kill_queues(). Double call to
drm_sched_entity_fini() is misuse of the API so keep call only in
pvr_context_create() failure path.
Stack trace for issue with addition of refcounting for DRM entity
stats in commit fd177135f0e6 ("drm/sched: Account entity GPU time"):
[ 789.490527] ------------[ cut here ]------------
[ 789.490559] refcount_t: underflow; use-after-free.
[ 789.490657] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xf4/0x144, CPU#0: kworker/u16:1/440
[ 789.490695] Modules linked in: powervr drm_gpuvm drm_exec gpu_sched drm_shmem_helper xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils sa2ul sha512 sha256 dwc3_am62 sha1 authenc rti_wdt libsha512 at24 sch_fq_codel fuse dm_mod ipv6
[ 789.490798] CPU: 0 UID: 0 PID: 440 Comm: kworker/u16:1 Not tainted 7.0.0-rc7-02049-g5e2c0700091b #22 PREEMPT
[ 789.490809] Hardware name: Texas Instruments AM625 SK (DT)
[ 789.490815] Workqueue: powervr-sched pvr_queue_fence_release_work [powervr]
[ 789.490868] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 789.490876] pc : refcount_warn_saturate+0xf4/0x144
[ 789.490884] lr : refcount_warn_saturate+0xf4/0x144
[ 789.490892] sp : ffff8000822cbcc0
[ 789.490895] x29: ffff8000822cbcc0 x28: 0000000000000000 x27: 0000000000000000
[ 789.490909] x26: 0000000000000000 x25: ffff800081b1e338 x24: ffff000004541405
[ 789.490922] x23: ffff000004bea950 x22: ffff00000042e400 x21: ffff000007123e30
[ 789.490935] x20: ffff000007123000 x19: ffff000007a80d50 x18: fffffffffffe7768
[ 789.490948] x17: 74736574202c6e6f x16: 697461746e656d65 x15: ffff800081b269f0
[ 789.490962] x14: 0000000000000030 x13: ffff800081b26a70 x12: 0000000000000211
[ 789.490975] x11: 00000000000000c0 x10: 0000000000000b50 x9 : ffff8000822cbb30
[ 789.490988] x8 : ffff0000014e7bb0 x7 : ffff00007725e780 x6 : 0000000372a05f49
[ 789.491001] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
[ 789.491013] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000014e7000
[ 789.491027] Call trace:
[ 789.491032] refcount_warn_saturate+0xf4/0x144 (P)
[ 789.491043] drm_sched_entity_fini+0x164/0x18c [gpu_sched]
[ 789.491081] pvr_queue_destroy+0x64/0x134 [powervr]
[ 789.491110] pvr_context_destroy_queues+0x34/0x64 [powervr]
[ 789.491138] pvr_context_release+0x70/0xac [powervr]
[ 789.491166] pvr_context_put.part.0+0x5c/0x7c [powervr]
[ 789.491193] pvr_context_put+0x14/0x24 [powervr]
[ 789.491221] pvr_queue_fence_release_work+0x20/0x38 [powervr]
[ 789.491249] process_one_work+0x160/0x4c4
[ 789.491264] worker_thread+0x188/0x310
[ 789.491276] kthread+0x130/0x13c
[ 789.491287] ret_from_fork+0x10/0x20
[ 789.491300] ---[ end trace 0000000000000000 ]---
Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
Cc: stable@vger.kernel.org
Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
---
Changes in v6:
- Fix variable name in pvr_queue.h as per v5.
- Link to v5: https://lore.kernel.org/r/20260630-b4-sched_fix-v5-1-2a84cbf18bfe@imgtec.com
Changes in v5:
- Update description of the issue and added stable tag.
- Modified variable name to align with behaviour.
- Link to v4: https://lore.kernel.org/r/20260619-b4-sched_fix-v4-1-65de5b2fd71d@imgtec.com
Changes in v4:
- Simplify logic in v3 by pushing new flag to pvr_queue_destroy().
- Link to v3: https://lore.kernel.org/r/20260611-b4-sched_fix-v3-1-693beb50ea01@imgtec.com
Changes in v3:
- Fixed a typo.
- Handled missing memory leak for RENDER_CONTEXT.
- Link to v2: https://lore.kernel.org/r/20260611-b4-sched_fix-v2-1-17a93be86fcd@imgtec.com
Changes in v2:
- Fixed memory leak identified in following error path handling of pvr_context_create():
- pvr_context_create()
- ...
- err_destroy_queues:
- pvr_context_destroy_queues()
- pvr_queue_destroy()
- Link to v1: https://lore.kernel.org/r/20260610-b4-sched_fix-v1-1-c5977a6e0b4c@imgtec.com
---
drivers/gpu/drm/imagination/pvr_context.c | 18 ++++++++++--------
drivers/gpu/drm/imagination/pvr_queue.c | 6 ++++--
drivers/gpu/drm/imagination/pvr_queue.h | 2 +-
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
index eba4694400b5..b6f9e078315d 100644
--- a/drivers/gpu/drm/imagination/pvr_context.c
+++ b/drivers/gpu/drm/imagination/pvr_context.c
@@ -161,22 +161,24 @@ ctx_fw_data_init(void *cpu_ptr, void *priv)
/**
* pvr_context_destroy_queues() - Destroy all queues attached to a context.
* @ctx: Context to destroy queues on.
+ * @cleanup_queue_entity: Whether to cleanup the queue entity e.g. context
+ * creation failure path.
*
* Should be called when the last reference to a context object is dropped.
* It releases all resources attached to the queues bound to this context.
*/
-static void pvr_context_destroy_queues(struct pvr_context *ctx)
+static void pvr_context_destroy_queues(struct pvr_context *ctx, bool cleanup_queue_entity)
{
switch (ctx->type) {
case DRM_PVR_CTX_TYPE_RENDER:
- pvr_queue_destroy(ctx->queues.fragment);
- pvr_queue_destroy(ctx->queues.geometry);
+ pvr_queue_destroy(ctx->queues.fragment, cleanup_queue_entity);
+ pvr_queue_destroy(ctx->queues.geometry, cleanup_queue_entity);
break;
case DRM_PVR_CTX_TYPE_COMPUTE:
- pvr_queue_destroy(ctx->queues.compute);
+ pvr_queue_destroy(ctx->queues.compute, cleanup_queue_entity);
break;
case DRM_PVR_CTX_TYPE_TRANSFER_FRAG:
- pvr_queue_destroy(ctx->queues.transfer);
+ pvr_queue_destroy(ctx->queues.transfer, cleanup_queue_entity);
break;
}
}
@@ -240,7 +242,7 @@ static int pvr_context_create_queues(struct pvr_context *ctx,
return -EINVAL;
err_destroy_queues:
- pvr_context_destroy_queues(ctx);
+ pvr_context_destroy_queues(ctx, true);
return err;
}
@@ -349,7 +351,7 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
pvr_fw_object_destroy(ctx->fw_obj);
err_destroy_queues:
- pvr_context_destroy_queues(ctx);
+ pvr_context_destroy_queues(ctx, true);
err_free_ctx_id:
/*
@@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
spin_unlock(&pvr_dev->ctx_list_lock);
xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
- pvr_context_destroy_queues(ctx);
+ pvr_context_destroy_queues(ctx, false);
pvr_fw_object_destroy(ctx->fw_obj);
kfree(ctx->data);
pvr_vm_context_put(ctx->vm_ctx);
diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index 7ed60e1c1a86..941c017399fc 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -1439,11 +1439,12 @@ void pvr_queue_kill(struct pvr_queue *queue)
/**
* pvr_queue_destroy() - Destroy a queue.
* @queue: The queue to destroy.
+ * @cleanup_queue_entity: Whether to cleanup the queue entity.
*
* Cleanup the queue and free the resources attached to it. Should be
* called from the context release function.
*/
-void pvr_queue_destroy(struct pvr_queue *queue)
+void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity)
{
if (!queue)
return;
@@ -1453,7 +1454,8 @@ void pvr_queue_destroy(struct pvr_queue *queue)
mutex_unlock(&queue->ctx->pvr_dev->queues.lock);
drm_sched_fini(&queue->scheduler);
- drm_sched_entity_fini(&queue->entity);
+ if (cleanup_queue_entity)
+ drm_sched_entity_fini(&queue->entity);
if (WARN_ON(queue->last_queued_job_scheduled_fence))
dma_fence_put(queue->last_queued_job_scheduled_fence);
diff --git a/drivers/gpu/drm/imagination/pvr_queue.h b/drivers/gpu/drm/imagination/pvr_queue.h
index 4aa72665ce25..149cc6d124bf 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.h
+++ b/drivers/gpu/drm/imagination/pvr_queue.h
@@ -158,7 +158,7 @@ struct pvr_queue *pvr_queue_create(struct pvr_context *ctx,
void pvr_queue_kill(struct pvr_queue *queue);
-void pvr_queue_destroy(struct pvr_queue *queue);
+void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity);
void pvr_queue_process(struct pvr_queue *queue);
---
base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
change-id: 20260610-b4-sched_fix-ac3b920f475b
Best regards,
--
Brajesh Gupta <brajesh.gupta@imgtec.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()
2026-06-30 10:03 [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini() Brajesh Gupta
@ 2026-06-30 13:53 ` Alessio Belle
2026-06-30 15:42 ` Brajesh Gupta
0 siblings, 1 reply; 3+ messages in thread
From: Alessio Belle @ 2026-06-30 13:53 UTC (permalink / raw)
To: Brajesh Gupta
Cc: tzimmermann, Matt Coster, simona, dri-devel, linux-kernel,
Frank Binns, boris.brezillon, maarten.lankhorst, stable, mripard,
airlied, Alexandru Dadu
Hi Brajesh,
On Tue, 2026-06-30 at 15:33 +0530, Brajesh Gupta wrote:
> Call sequence of double call:
> pvr_context_destroy
> pvr_context_kill_queues
> pvr_queue_kill
> drm_sched_entity_destroy
> drm_sched_entity_fini // here
> pvr_context_put
> kref_put(..., pvr_context_release)
> pvr_context_destroy_queues
> pvr_queue_destroy
> drm_sched_entity_fini // here
>
> Call to drm_sched_entity_destroy() from pvr_context_kill_queues() calls
> drm_sched_entity_flush() + drm_sched_entity_fini().
> drm_sched_entity_flush() ensures all pending jobs are completed and
> drm_sched_entity_fini() ensures no further submission is allowed as
> per expectation from pvr_context_kill_queues(). Double call to
> drm_sched_entity_fini() is misuse of the API so keep call only in
> pvr_context_create() failure path.
>
> Stack trace for issue with addition of refcounting for DRM entity
> stats in commit fd177135f0e6 ("drm/sched: Account entity GPU time"):
Sorry late nit: looking at other kernel dumps in commit descriptions, there
should be an empty line here.
> [ 789.490527] ------------[ cut here ]------------
> [ 789.490559] refcount_t: underflow; use-after-free.
> [ 789.490657] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xf4/0x144, CPU#0: kworker/u16:1/440
> [ 789.490695] Modules linked in: powervr drm_gpuvm drm_exec gpu_sched drm_shmem_helper xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils sa2ul sha512 sha256 dwc3_am62 sha1 authenc rti_wdt libsha512 at24 sch_fq_codel fuse dm_mod ipv6
> [ 789.490798] CPU: 0 UID: 0 PID: 440 Comm: kworker/u16:1 Not tainted 7.0.0-rc7-02049-g5e2c0700091b #22 PREEMPT
> [ 789.490809] Hardware name: Texas Instruments AM625 SK (DT)
> [ 789.490815] Workqueue: powervr-sched pvr_queue_fence_release_work [powervr]
> [ 789.490868] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [ 789.490876] pc : refcount_warn_saturate+0xf4/0x144
> [ 789.490884] lr : refcount_warn_saturate+0xf4/0x144
> [ 789.490892] sp : ffff8000822cbcc0
> [ 789.490895] x29: ffff8000822cbcc0 x28: 0000000000000000 x27: 0000000000000000
> [ 789.490909] x26: 0000000000000000 x25: ffff800081b1e338 x24: ffff000004541405
> [ 789.490922] x23: ffff000004bea950 x22: ffff00000042e400 x21: ffff000007123e30
> [ 789.490935] x20: ffff000007123000 x19: ffff000007a80d50 x18: fffffffffffe7768
> [ 789.490948] x17: 74736574202c6e6f x16: 697461746e656d65 x15: ffff800081b269f0
> [ 789.490962] x14: 0000000000000030 x13: ffff800081b26a70 x12: 0000000000000211
> [ 789.490975] x11: 00000000000000c0 x10: 0000000000000b50 x9 : ffff8000822cbb30
> [ 789.490988] x8 : ffff0000014e7bb0 x7 : ffff00007725e780 x6 : 0000000372a05f49
> [ 789.491001] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
> [ 789.491013] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000014e7000
> [ 789.491027] Call trace:
> [ 789.491032] refcount_warn_saturate+0xf4/0x144 (P)
> [ 789.491043] drm_sched_entity_fini+0x164/0x18c [gpu_sched]
> [ 789.491081] pvr_queue_destroy+0x64/0x134 [powervr]
> [ 789.491110] pvr_context_destroy_queues+0x34/0x64 [powervr]
> [ 789.491138] pvr_context_release+0x70/0xac [powervr]
> [ 789.491166] pvr_context_put.part.0+0x5c/0x7c [powervr]
> [ 789.491193] pvr_context_put+0x14/0x24 [powervr]
> [ 789.491221] pvr_queue_fence_release_work+0x20/0x38 [powervr]
> [ 789.491249] process_one_work+0x160/0x4c4
> [ 789.491264] worker_thread+0x188/0x310
> [ 789.491276] kthread+0x130/0x13c
> [ 789.491287] ret_from_fork+0x10/0x20
> [ 789.491300] ---[ end trace 0000000000000000 ]---
>
> Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
> ---
> Changes in v6:
> - Fix variable name in pvr_queue.h as per v5.
> - Link to v5: https://lore.kernel.org/r/20260630-b4-sched_fix-v5-1-2a84cbf18bfe@imgtec.com
>
> Changes in v5:
> - Update description of the issue and added stable tag.
> - Modified variable name to align with behaviour.
> - Link to v4: https://lore.kernel.org/r/20260619-b4-sched_fix-v4-1-65de5b2fd71d@imgtec.com
>
> Changes in v4:
> - Simplify logic in v3 by pushing new flag to pvr_queue_destroy().
> - Link to v3: https://lore.kernel.org/r/20260611-b4-sched_fix-v3-1-693beb50ea01@imgtec.com
>
> Changes in v3:
> - Fixed a typo.
> - Handled missing memory leak for RENDER_CONTEXT.
> - Link to v2: https://lore.kernel.org/r/20260611-b4-sched_fix-v2-1-17a93be86fcd@imgtec.com
>
> Changes in v2:
> - Fixed memory leak identified in following error path handling of pvr_context_create():
> - pvr_context_create()
> - ...
> - err_destroy_queues:
> - pvr_context_destroy_queues()
> - pvr_queue_destroy()
> - Link to v1: https://lore.kernel.org/r/20260610-b4-sched_fix-v1-1-c5977a6e0b4c@imgtec.com
> ---
> drivers/gpu/drm/imagination/pvr_context.c | 18 ++++++++++--------
> drivers/gpu/drm/imagination/pvr_queue.c | 6 ++++--
> drivers/gpu/drm/imagination/pvr_queue.h | 2 +-
> 3 files changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> index eba4694400b5..b6f9e078315d 100644
> --- a/drivers/gpu/drm/imagination/pvr_context.c
> +++ b/drivers/gpu/drm/imagination/pvr_context.c
> @@ -161,22 +161,24 @@ ctx_fw_data_init(void *cpu_ptr, void *priv)
> /**
> * pvr_context_destroy_queues() - Destroy all queues attached to a context.
> * @ctx: Context to destroy queues on.
> + * @cleanup_queue_entity: Whether to cleanup the queue entity e.g. context
> + * creation failure path.
nit: Could you align the second line of this comment?
With these sorted, in v7 you can add to the end of the trailers:
Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>
Thanks,
Alessio
> *
> * Should be called when the last reference to a context object is dropped.
> * It releases all resources attached to the queues bound to this context.
> */
> -static void pvr_context_destroy_queues(struct pvr_context *ctx)
> +static void pvr_context_destroy_queues(struct pvr_context *ctx, bool cleanup_queue_entity)
> {
> switch (ctx->type) {
> case DRM_PVR_CTX_TYPE_RENDER:
> - pvr_queue_destroy(ctx->queues.fragment);
> - pvr_queue_destroy(ctx->queues.geometry);
> + pvr_queue_destroy(ctx->queues.fragment, cleanup_queue_entity);
> + pvr_queue_destroy(ctx->queues.geometry, cleanup_queue_entity);
> break;
> case DRM_PVR_CTX_TYPE_COMPUTE:
> - pvr_queue_destroy(ctx->queues.compute);
> + pvr_queue_destroy(ctx->queues.compute, cleanup_queue_entity);
> break;
> case DRM_PVR_CTX_TYPE_TRANSFER_FRAG:
> - pvr_queue_destroy(ctx->queues.transfer);
> + pvr_queue_destroy(ctx->queues.transfer, cleanup_queue_entity);
> break;
> }
> }
> @@ -240,7 +242,7 @@ static int pvr_context_create_queues(struct pvr_context *ctx,
> return -EINVAL;
>
> err_destroy_queues:
> - pvr_context_destroy_queues(ctx);
> + pvr_context_destroy_queues(ctx, true);
> return err;
> }
>
> @@ -349,7 +351,7 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
> pvr_fw_object_destroy(ctx->fw_obj);
>
> err_destroy_queues:
> - pvr_context_destroy_queues(ctx);
> + pvr_context_destroy_queues(ctx, true);
>
> err_free_ctx_id:
> /*
> @@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
> spin_unlock(&pvr_dev->ctx_list_lock);
>
> xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
> - pvr_context_destroy_queues(ctx);
> + pvr_context_destroy_queues(ctx, false);
> pvr_fw_object_destroy(ctx->fw_obj);
> kfree(ctx->data);
> pvr_vm_context_put(ctx->vm_ctx);
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> index 7ed60e1c1a86..941c017399fc 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> @@ -1439,11 +1439,12 @@ void pvr_queue_kill(struct pvr_queue *queue)
> /**
> * pvr_queue_destroy() - Destroy a queue.
> * @queue: The queue to destroy.
> + * @cleanup_queue_entity: Whether to cleanup the queue entity.
> *
> * Cleanup the queue and free the resources attached to it. Should be
> * called from the context release function.
> */
> -void pvr_queue_destroy(struct pvr_queue *queue)
> +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity)
> {
> if (!queue)
> return;
> @@ -1453,7 +1454,8 @@ void pvr_queue_destroy(struct pvr_queue *queue)
> mutex_unlock(&queue->ctx->pvr_dev->queues.lock);
>
> drm_sched_fini(&queue->scheduler);
> - drm_sched_entity_fini(&queue->entity);
> + if (cleanup_queue_entity)
> + drm_sched_entity_fini(&queue->entity);
>
> if (WARN_ON(queue->last_queued_job_scheduled_fence))
> dma_fence_put(queue->last_queued_job_scheduled_fence);
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.h b/drivers/gpu/drm/imagination/pvr_queue.h
> index 4aa72665ce25..149cc6d124bf 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.h
> +++ b/drivers/gpu/drm/imagination/pvr_queue.h
> @@ -158,7 +158,7 @@ struct pvr_queue *pvr_queue_create(struct pvr_context *ctx,
>
> void pvr_queue_kill(struct pvr_queue *queue);
>
> -void pvr_queue_destroy(struct pvr_queue *queue);
> +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity);
>
> void pvr_queue_process(struct pvr_queue *queue);
>
>
> ---
> base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
> change-id: 20260610-b4-sched_fix-ac3b920f475b
>
> Best regards,
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini()
2026-06-30 13:53 ` Alessio Belle
@ 2026-06-30 15:42 ` Brajesh Gupta
0 siblings, 0 replies; 3+ messages in thread
From: Brajesh Gupta @ 2026-06-30 15:42 UTC (permalink / raw)
To: Alessio Belle
Cc: tzimmermann, Matt Coster, simona, dri-devel, linux-kernel,
Frank Binns, boris.brezillon, maarten.lankhorst, stable, mripard,
airlied, Alexandru Dadu
On Tue, 2026-06-30 at 13:53 +0000, Alessio Belle wrote:
Hi Alessio,
> *** NOTE: This is an internal email from Imagination Technologies ***
>
>
>
>
> Hi Brajesh,
>
> On Tue, 2026-06-30 at 15:33 +0530, Brajesh Gupta wrote:
> > Call sequence of double call:
> > pvr_context_destroy
> > pvr_context_kill_queues
> > pvr_queue_kill
> > drm_sched_entity_destroy
> > drm_sched_entity_fini // here
> > pvr_context_put
> > kref_put(..., pvr_context_release)
> > pvr_context_destroy_queues
> > pvr_queue_destroy
> > drm_sched_entity_fini // here
> >
> > Call to drm_sched_entity_destroy() from pvr_context_kill_queues() calls
> > drm_sched_entity_flush() + drm_sched_entity_fini().
> > drm_sched_entity_flush() ensures all pending jobs are completed and
> > drm_sched_entity_fini() ensures no further submission is allowed as
> > per expectation from pvr_context_kill_queues(). Double call to
> > drm_sched_entity_fini() is misuse of the API so keep call only in
> > pvr_context_create() failure path.
> >
> > Stack trace for issue with addition of refcounting for DRM entity
> > stats in commit fd177135f0e6 ("drm/sched: Account entity GPU time"):
>
> Sorry late nit: looking at other kernel dumps in commit descriptions, there
> should be an empty line here.
Fixed.
>
> > [ 789.490527] ------------[ cut here ]------------
> > [ 789.490559] refcount_t: underflow; use-after-free.
> > [ 789.490657] WARNING: lib/refcount.c:28 at refcount_warn_saturate+0xf4/0x144, CPU#0: kworker/u16:1/440
> > [ 789.490695] Modules linked in: powervr drm_gpuvm drm_exec gpu_sched drm_shmem_helper xhci_plat_hcd xhci_hcd dwc3 usbcore usb_common snd_soc_simple_card snd_soc_simple_card_utils sa2ul sha512 sha256 dwc3_am62 sha1 authenc rti_wdt libsha512 at24 sch_fq_codel fuse dm_mod ipv6
> > [ 789.490798] CPU: 0 UID: 0 PID: 440 Comm: kworker/u16:1 Not tainted 7.0.0-rc7-02049-g5e2c0700091b #22 PREEMPT
> > [ 789.490809] Hardware name: Texas Instruments AM625 SK (DT)
> > [ 789.490815] Workqueue: powervr-sched pvr_queue_fence_release_work [powervr]
> > [ 789.490868] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [ 789.490876] pc : refcount_warn_saturate+0xf4/0x144
> > [ 789.490884] lr : refcount_warn_saturate+0xf4/0x144
> > [ 789.490892] sp : ffff8000822cbcc0
> > [ 789.490895] x29: ffff8000822cbcc0 x28: 0000000000000000 x27: 0000000000000000
> > [ 789.490909] x26: 0000000000000000 x25: ffff800081b1e338 x24: ffff000004541405
> > [ 789.490922] x23: ffff000004bea950 x22: ffff00000042e400 x21: ffff000007123e30
> > [ 789.490935] x20: ffff000007123000 x19: ffff000007a80d50 x18: fffffffffffe7768
> > [ 789.490948] x17: 74736574202c6e6f x16: 697461746e656d65 x15: ffff800081b269f0
> > [ 789.490962] x14: 0000000000000030 x13: ffff800081b26a70 x12: 0000000000000211
> > [ 789.490975] x11: 00000000000000c0 x10: 0000000000000b50 x9 : ffff8000822cbb30
> > [ 789.490988] x8 : ffff0000014e7bb0 x7 : ffff00007725e780 x6 : 0000000372a05f49
> > [ 789.491001] x5 : 0000000000000000 x4 : 0000000000000001 x3 : 0000000000000010
> > [ 789.491013] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0000014e7000
> > [ 789.491027] Call trace:
> > [ 789.491032] refcount_warn_saturate+0xf4/0x144 (P)
> > [ 789.491043] drm_sched_entity_fini+0x164/0x18c [gpu_sched]
> > [ 789.491081] pvr_queue_destroy+0x64/0x134 [powervr]
> > [ 789.491110] pvr_context_destroy_queues+0x34/0x64 [powervr]
> > [ 789.491138] pvr_context_release+0x70/0xac [powervr]
> > [ 789.491166] pvr_context_put.part.0+0x5c/0x7c [powervr]
> > [ 789.491193] pvr_context_put+0x14/0x24 [powervr]
> > [ 789.491221] pvr_queue_fence_release_work+0x20/0x38 [powervr]
> > [ 789.491249] process_one_work+0x160/0x4c4
> > [ 789.491264] worker_thread+0x188/0x310
> > [ 789.491276] kthread+0x130/0x13c
> > [ 789.491287] ret_from_fork+0x10/0x20
> > [ 789.491300] ---[ end trace 0000000000000000 ]---
> >
> > Fixes: eaf01ee5ba28 ("drm/imagination: Implement job submission and scheduling")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Brajesh Gupta <brajesh.gupta@imgtec.com>
> > ---
> > Changes in v6:
> > - Fix variable name in pvr_queue.h as per v5.
> > - Link to v5: https://lore.kernel.org/r/20260630-b4-sched_fix-v5-1-2a84cbf18bfe@imgtec.com
> >
> > Changes in v5:
> > - Update description of the issue and added stable tag.
> > - Modified variable name to align with behaviour.
> > - Link to v4: https://lore.kernel.org/r/20260619-b4-sched_fix-v4-1-65de5b2fd71d@imgtec.com
> >
> > Changes in v4:
> > - Simplify logic in v3 by pushing new flag to pvr_queue_destroy().
> > - Link to v3: https://lore.kernel.org/r/20260611-b4-sched_fix-v3-1-693beb50ea01@imgtec.com
> >
> > Changes in v3:
> > - Fixed a typo.
> > - Handled missing memory leak for RENDER_CONTEXT.
> > - Link to v2: https://lore.kernel.org/r/20260611-b4-sched_fix-v2-1-17a93be86fcd@imgtec.com
> >
> > Changes in v2:
> > - Fixed memory leak identified in following error path handling of pvr_context_create():
> > - pvr_context_create()
> > - ...
> > - err_destroy_queues:
> > - pvr_context_destroy_queues()
> > - pvr_queue_destroy()
> > - Link to v1: https://lore.kernel.org/r/20260610-b4-sched_fix-v1-1-c5977a6e0b4c@imgtec.com
> > ---
> > drivers/gpu/drm/imagination/pvr_context.c | 18 ++++++++++--------
> > drivers/gpu/drm/imagination/pvr_queue.c | 6 ++++--
> > drivers/gpu/drm/imagination/pvr_queue.h | 2 +-
> > 3 files changed, 15 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/imagination/pvr_context.c b/drivers/gpu/drm/imagination/pvr_context.c
> > index eba4694400b5..b6f9e078315d 100644
> > --- a/drivers/gpu/drm/imagination/pvr_context.c
> > +++ b/drivers/gpu/drm/imagination/pvr_context.c
> > @@ -161,22 +161,24 @@ ctx_fw_data_init(void *cpu_ptr, void *priv)
> > /**
> > * pvr_context_destroy_queues() - Destroy all queues attached to a context.
> > * @ctx: Context to destroy queues on.
> > + * @cleanup_queue_entity: Whether to cleanup the queue entity e.g. context
> > + * creation failure path.
>
> nit: Could you align the second line of this comment?
Fixed
>
> With these sorted, in v7 you can add to the end of the trailers:
>
> Reviewed-by: Alessio Belle <alessio.belle@imgtec.com>
Done
Thanks,
Brajesh
>
> Thanks,
> Alessio
>
> > *
> > * Should be called when the last reference to a context object is dropped.
> > * It releases all resources attached to the queues bound to this context.
> > */
> > -static void pvr_context_destroy_queues(struct pvr_context *ctx)
> > +static void pvr_context_destroy_queues(struct pvr_context *ctx, bool cleanup_queue_entity)
> > {
> > switch (ctx->type) {
> > case DRM_PVR_CTX_TYPE_RENDER:
> > - pvr_queue_destroy(ctx->queues.fragment);
> > - pvr_queue_destroy(ctx->queues.geometry);
> > + pvr_queue_destroy(ctx->queues.fragment, cleanup_queue_entity);
> > + pvr_queue_destroy(ctx->queues.geometry, cleanup_queue_entity);
> > break;
> > case DRM_PVR_CTX_TYPE_COMPUTE:
> > - pvr_queue_destroy(ctx->queues.compute);
> > + pvr_queue_destroy(ctx->queues.compute, cleanup_queue_entity);
> > break;
> > case DRM_PVR_CTX_TYPE_TRANSFER_FRAG:
> > - pvr_queue_destroy(ctx->queues.transfer);
> > + pvr_queue_destroy(ctx->queues.transfer, cleanup_queue_entity);
> > break;
> > }
> > }
> > @@ -240,7 +242,7 @@ static int pvr_context_create_queues(struct pvr_context *ctx,
> > return -EINVAL;
> >
> > err_destroy_queues:
> > - pvr_context_destroy_queues(ctx);
> > + pvr_context_destroy_queues(ctx, true);
> > return err;
> > }
> >
> > @@ -349,7 +351,7 @@ int pvr_context_create(struct pvr_file *pvr_file, struct drm_pvr_ioctl_create_co
> > pvr_fw_object_destroy(ctx->fw_obj);
> >
> > err_destroy_queues:
> > - pvr_context_destroy_queues(ctx);
> > + pvr_context_destroy_queues(ctx, true);
> >
> > err_free_ctx_id:
> > /*
> > @@ -384,7 +386,7 @@ pvr_context_release(struct kref *ref_count)
> > spin_unlock(&pvr_dev->ctx_list_lock);
> >
> > xa_erase(&pvr_dev->ctx_ids, ctx->ctx_id);
> > - pvr_context_destroy_queues(ctx);
> > + pvr_context_destroy_queues(ctx, false);
> > pvr_fw_object_destroy(ctx->fw_obj);
> > kfree(ctx->data);
> > pvr_vm_context_put(ctx->vm_ctx);
> > diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> > index 7ed60e1c1a86..941c017399fc 100644
> > --- a/drivers/gpu/drm/imagination/pvr_queue.c
> > +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> > @@ -1439,11 +1439,12 @@ void pvr_queue_kill(struct pvr_queue *queue)
> > /**
> > * pvr_queue_destroy() - Destroy a queue.
> > * @queue: The queue to destroy.
> > + * @cleanup_queue_entity: Whether to cleanup the queue entity.
> > *
> > * Cleanup the queue and free the resources attached to it. Should be
> > * called from the context release function.
> > */
> > -void pvr_queue_destroy(struct pvr_queue *queue)
> > +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity)
> > {
> > if (!queue)
> > return;
> > @@ -1453,7 +1454,8 @@ void pvr_queue_destroy(struct pvr_queue *queue)
> > mutex_unlock(&queue->ctx->pvr_dev->queues.lock);
> >
> > drm_sched_fini(&queue->scheduler);
> > - drm_sched_entity_fini(&queue->entity);
> > + if (cleanup_queue_entity)
> > + drm_sched_entity_fini(&queue->entity);
> >
> > if (WARN_ON(queue->last_queued_job_scheduled_fence))
> > dma_fence_put(queue->last_queued_job_scheduled_fence);
> > diff --git a/drivers/gpu/drm/imagination/pvr_queue.h b/drivers/gpu/drm/imagination/pvr_queue.h
> > index 4aa72665ce25..149cc6d124bf 100644
> > --- a/drivers/gpu/drm/imagination/pvr_queue.h
> > +++ b/drivers/gpu/drm/imagination/pvr_queue.h
> > @@ -158,7 +158,7 @@ struct pvr_queue *pvr_queue_create(struct pvr_context *ctx,
> >
> > void pvr_queue_kill(struct pvr_queue *queue);
> >
> > -void pvr_queue_destroy(struct pvr_queue *queue);
> > +void pvr_queue_destroy(struct pvr_queue *queue, bool cleanup_queue_entity);
> >
> > void pvr_queue_process(struct pvr_queue *queue);
> >
> >
> > ---
> > base-commit: 61de054a772a1feda6364931ab1baf9038abf1c8
> > change-id: 20260610-b4-sched_fix-ac3b920f475b
> >
> > Best regards,
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-06-30 15:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-30 10:03 [PATCH v6] drm/imagination: Fix double call to drm_sched_entity_fini() Brajesh Gupta
2026-06-30 13:53 ` Alessio Belle
2026-06-30 15:42 ` Brajesh Gupta
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®