On 14/01/2026 11:39, Christian König wrote:
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 <[email protected]>
---
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.
Flags sounds okay-ish. I mean it is a bit roundabout to track the state
and export API only useful for some driver flows, not least that zero
alloc of the object itself is not mandatory to begin with, but I guess
it is passable.
Hm... it could even be solved with "DMA_FENCE_FLAG_USER_BITS+1". Since
only two drivers need this maybe something to consider.
Regards,
Tvrtko
