Re: [PATCH] drm/panthor: Fix the "done_fence is initialized" detection logic

[Liviu Dudau <liviu.dudau@arm.com>]

On Mon, Mar 09, 2026 at 02:15:49PM +0100, Boris Brezillon wrote:
> On Mon, 9 Mar 2026 11:05:06 +0000
> Liviu Dudau <liviu.dudau@arm.com> wrote:
> 
> > > After commit 541c8f2468b9 ("dma-buf: detach fence ops on signal v3"),
> > > dma_fence::ops == NULL can't be used to check if the fence is initialized
> > > or not. We could turn this into an "is_signaled() || ops == NULL" test,
> > > but that's fragile, since it's still subject to dma_fence internal
> > > changes. So let's have the "is_initialized" state encoded directly in
> > > the pointer through the lowest bit which is guaranteed to be unused
> > > because of the dma_fence alignment constraint.  
> > 
> > I'm confused! There is only one place where we end up being interested if the
> > fence has been initialized or not, and that is in job_release(). I don't
> > see why checking for "ops != NULL" before calling dma_fence_put() should not
> > be enough,
> 
> Because after 541c8f2468b9 ("dma-buf: detach fence ops on signal v3"),
> dma_fence->ops is set back to NULL at signal time[1].

Yes, I gathered that. What I meant to say was that I don't understand why we need
all this infrastructure just for one check. Meanwhile Christian pointed out that
a simpler solution already exists.

> 
> > or even better, why don't we call dma_fence_put() regardless,
> > as the core code should take care of an uninitialized dma_fence AFAICT.
> 
> When the job is created, we pre-allocate the done_fence, but we leave it
> uninitialized until ::run_job() is called. If we call
> dma_fence_release() (through dma_fence_put()) on a dma_fence that was
> not dma_fence_init()-ialized, we have a NULL deref on the cb_list, and
> probably other issues too.

I don't see the benefit of not initializing the done_fence until we ::run_job()
but I might have missed something obvious. If we want to keep that, maybe we
should not be droping the reference in job_release() but when we
signal the fence. But that would leak the memory of the uninitialized done_fence.

Best regards,
Liviu

> 
> [1]https://gitlab.freedesktop.org/drm/misc/kernel/-/blob/drm-misc-next/drivers/dma-buf/dma-fence.c?ref_type=heads#L373

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯