Re: [PATCH 01/15] dma-buf: cleanup dma_fence_describe

[Tvrtko Ursulin]

On 13/10/2025 14:48, Christian König wrote:
The driver and timeline name are meaningless for signaled fences.

Drop them and also print the context number.

Signed-off-by: Christian König <christian.koe...@amd.com>
---
  drivers/dma-buf/dma-fence.c | 11 ++++++-----
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 3f78c56b58dc..f0539c73ed57 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -1001,17 +1001,18 @@ void dma_fence_describe(struct dma_fence *fence, struct 
seq_file *seq)
  {
        const char __rcu *timeline;
        const char __rcu *driver;
+       const char *signaled = "un";
  
  	rcu_read_lock();
  
  	timeline = dma_fence_timeline_name(fence);
        driver = dma_fence_driver_name(fence);
  
-	seq_printf(seq, "%s %s seq %llu %ssignalled\n",
-                  rcu_dereference(driver),
-                  rcu_dereference(timeline),
-                  fence->seqno,
-                  dma_fence_is_signaled(fence) ? "" : "un");
+       if (dma_fence_is_signaled(fence))
+               timeline = driver = signaled = "";

FWIW you could avoid calling dma_fence_timeline_name() and 
dma_fence_driver_name() since you added the signaled check.

May end up slightly nicer than to override strings returned from helpers 
with a chained assignment.

Or even store the signaled status in a local bool and branch off two 
seq_printfs based on it.

+
+       seq_printf(seq, "%llu %s %s seq %llu %ssignalled\n", fence->context,
+                  timeline, driver, fence->seqno, signaled);

I was initially worried if this string ends up anywhere which could be 
considered ABI but it seems debugfs only so changing the formatting is fine.

How about making dma_fence_describe() conditional on CONFIG_DEBUG_FS to 
set this in stone? (And dma_resv_describe..)

And maybe unify the %llu:%llu context:fence as the tracepoints use?

Altogether something like:

rcu_read_lock();

signaled = dma_fence_is_signaled(fence);

if (signaled)
        seq_printf("%llu:%llu %s %s signalled",
                   fence->context,
                   fence->seqno,
                   dma_fence_timeline_name(fence),
                   dma_fence_timeline_name(fence);
else
        seq_print("%llu:%llu unsignalled",
                  fence->seqno, fence->context);

Maybe more readable but up to you.

Regards,

Tvrtko
        rcu_read_unlock();
  }