On 20/10/2026 07:47, Philipp Stanner wrote:
On Fri, 2026-10-16 at 09:23 +0100, Tvrtko Ursulin wrote:
On 15/10/2026 20:30, Olivier Kaloudoff wrote:
The hole was latent-but-safe until commit 2df5efb45425 ("drm/sched:
Remove redundant entity->rq initialization and checks") removed the
if (!entity->rq) early return that commit 1decbf6bb0b4 ("drm/sched:
Fix entities with 0 rqs.") had put in drm_sched_entity_flush(), on the
grounds that entity->rq can never be NULL. That holds only for
entities that completed drm_sched_entity_init() — not for the
never-initialized entity this teardown path reaches.
[…]
- drm_sched_entity_destroy(&adev->uvd.entity);
+ if (adev->uvd.entity.rq)
+ drm_sched_entity_destroy(&adev->uvd.entity);
for (j = 0; j < adev->uvd.num_uvd_inst; ++j) {
if (adev->uvd.harvest_config & (1 << j))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 1f7a98cdc..3952af9ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -253,7 +253,8 @@ int amdgpu_vce_sw_fini(struct amdgpu_device *adev)
if (adev->vce.vcpu_bo == NULL)
return 0;
- drm_sched_entity_destroy(&adev->vce.entity);
+ if (adev->vce.entity.rq)
+ drm_sched_entity_destroy(&adev->vce.entity);
for (i = 0; i < adev->vce.num_rings; i++)
amdgpu_ring_fini(&adev->vce.ring[i]);
Dang, it looks like currently the ownership of entity initialization is
broken between the IP agnostic phase which does the init, and per-IP
phase which tears it down.
The fix will get in the way of me re-trying to embed the run-queue into
the scheduler but it does look correct.
Couldn't the status be tracked through the vce struct?
if (vce.entity_initialized)
Otherwise you rely on API-internals (again) which is what enabled that
bug AFAICS. If it's about amdgpu having called foo_init() or not, then
the most robust solution is to track within amdgpu "did I call that
function already?" Same applies to all constructors and destructors in
the kernel AFAIK.
Yep, it is such a horrible pattern to rely on being able to call
destructors on non-initialized objects. I am looking at some flavour of
"track in amdgpu" as priority options. Either by re-arranging the probe
steps, or by tracking explicitly.
Reviewed-by: Tvrtko Ursulin <[email protected]>
I will see if I can re-work the init/cleanup phases so this can be
removed, or if not, try to add a proper DRM scheduler API to query the
status of an entity.
An API with which you can query whether a data object that you
allocated and that you own had the constructor run by yourself? Or am I
misunderstanding something?
Yeah it would be bad. I don't think it will come to it since there is at
least one better option.
Regards,
Tvrtko
