On Tue, Oct 14, 2025 at 03:37:03PM +0100, Tvrtko Ursulin wrote:
>
> 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 <[email protected]>
> > ---
> > 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.
>
+1 to avoid calling dma_fence_timeline_name / dma_fence_driver_name on
signaled fences.
Matt
> 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();
> > }
>
