On 1/14/26 10:53, Tvrtko Ursulin wrote:
\ On 13/01/2026 15:16, Christian König wrote:
Some driver use fence->ops to test if a fence was initialized or not. The problem is that this utilizes internal behavior of the dma_fence implementation.
So better abstract that into a function.
Signed-off-by: Christian König christian.koenig@amd.com
drivers/gpu/drm/amd/amdgpu/amdgpu_job.c | 13 +++++++------ drivers/gpu/drm/qxl/qxl_release.c | 2 +- include/linux/dma-fence.h | 12 ++++++++++++ 3 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c index 0a0dcbf0798d..b97f90bbe8b9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c @@ -278,9 +278,10 @@ void amdgpu_job_free_resources(struct amdgpu_job *job) unsigned i; /* Check if any fences were initialized */ - if (job->base.s_fence && job->base.s_fence->finished.ops) + if (job->base.s_fence && + dma_fence_is_initialized(&job->base.s_fence->finished)) f = &job->base.s_fence->finished; - else if (job->hw_fence && job->hw_fence->base.ops) + else if (dma_fence_is_initialized(&job->hw_fence->base)) f = &job->hw_fence->base; else f = NULL; @@ -297,11 +298,11 @@ static void amdgpu_job_free_cb(struct drm_sched_job *s_job) amdgpu_sync_free(&job->explicit_sync); - if (job->hw_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_fence->base)) dma_fence_put(&job->hw_fence->base); else kfree(job->hw_fence); - if (job->hw_vm_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_vm_fence->base)) dma_fence_put(&job->hw_vm_fence->base); else kfree(job->hw_vm_fence); @@ -335,11 +336,11 @@ void amdgpu_job_free(struct amdgpu_job *job) if (job->gang_submit != &job->base.s_fence->scheduled) dma_fence_put(job->gang_submit); - if (job->hw_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_fence->base)) dma_fence_put(&job->hw_fence->base); else kfree(job->hw_fence); - if (job->hw_vm_fence->base.ops) + if (dma_fence_is_initialized(&job->hw_vm_fence->base)) dma_fence_put(&job->hw_vm_fence->base); else kfree(job->hw_vm_fence); diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index 7b3c9a6016db..b38ae0b25f3c 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -146,7 +146,7 @@ qxl_release_free(struct qxl_device *qdev, idr_remove(&qdev->release_idr, release->id); spin_unlock(&qdev->release_idr_lock); - if (release->base.ops) { + if (dma_fence_is_initialized(&release->base)) { WARN_ON(list_empty(&release->bos)); qxl_release_free_list(release); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index eea674acdfa6..371aa8ecf18e 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -274,6 +274,18 @@ void dma_fence_release(struct kref *kref); void dma_fence_free(struct dma_fence *fence); void dma_fence_describe(struct dma_fence *fence, struct seq_file *seq); +/**
- dma_fence_is_initialized - test if fence was initialized
- @fence: fence to test
- Return: True if fence was initialized, false otherwise. Works correctly only
- when memory backing the fence structure is zero initialized on allocation.
- */
+static inline bool dma_fence_is_initialized(struct dma_fence *fence) +{ + return fence && !!fence->ops;
This patch should precede the one adding RCU protection to fence->ops. And that one then needs to add a rcu_dereference() here.
Good point.
At which point however it would start exploding?
When we start setting the ops pointer to NULL in the next patch.
Which also means the new API is racy by definition and can give false positives if fence would be to be signaled as someone is checking.
Oh, that is a really really good point. I haven't thought about that because all current users would check the fence only after it is signaled.
Hmm.. is the new API too weak, being able to only be called under very limited circumstances?
Yes, exactly that. All callers use this only to decide on the correct cleanup path.
So the fence is either fully signaled or was never initialized in the first place.
Would it be better to solve it in the drivers by tracking state?
The alternative I had in mind was to use another DMA_FENCE_FLAG_... for that.
I will probably use that approach instead, just to make it extra defensive.
Thanks, Christian.
Regards,
Tvrtko
+}
/** * dma_fence_put - decreases refcount of the fence * @fence: fence to reduce refcount of