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.
>
> 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?
P.
