+Cc Alessio, Luigi
On Wed, 2026-09-02 at 18:09 +0200, Philipp Stanner wrote:
> Well, that was a quick investigation ;)
>
> On Wed, 2026-09-02 at 23:42 +0900, Jonghyuk Kim(MalHyuk) wrote:
> >
[…]
>
> >
> > Note: detaching the finished fence's ops on signalling also makes
> > to_drm_sched_fence() return NULL for a signalled finished fence. Callers
> > already handle NULL (the normal foreign-fence result), a signalled fence is
> > an already-satisfied dependency so the scheduler's dependency collapsing is
> > unaffected, and it avoids the container_of() on a possibly-freed foreign
> > scheduler that amdgpu_sync_same_dev() and pvr_queue_fence_is_native() would
> > otherwise do. Flagging it explicitly since it touches an exported helper.
>
> That unfortunately does look a bit dangerous.
>
> Isn't pvr here already a race condition?
>
> if (pvr_queue_fence_is_native(uf)) {
> struct drm_sched_fence *s_fence =
> to_drm_sched_fence(uf);
I looked through the code base and it seems no one touches the ops
pointer.
The exception is imagination, which uses it to identify whether a fence
stems from itself. So if we implement a change such as the proposed
one, explosions are thinkable:
bool pvr_queue_fence_is_native(struct dma_fence *f)
{
struct drm_sched_fence *sched_fence = f ? to_drm_sched_fence(f) : NULL;
// <-- ops pointer still valid, sched_fence != NULL
// race: dma_fence_signal(sched_fence->finished) -> sched->ops becomes
NULL
if (sched_fence &&
sched_fence->sched->ops == &pvr_queue_sched_ops)
return true; // might return false now although the fence was
created by imagination
return pvr_queue_fence_is_ufo_backed(f);
}
So depending on when the finished-fence gets signaled, the function
could now sometimes return true, then false, depending on how it's
racing. Not entirely sure, depends probably a bit on when imagination
is signaling its hardware fences and so on.
But I'm not entirely sure to what degree we have a problem here, and if
so how we should best solve it.
P.
