Re: [PATCH v2 1/2] drm/drm_crtc: ensure dma_fence_ops remain valid during device unbind

[Philipp Stanner <phasta@mailbox.org>]

On Thu, 2026-07-09 at 15:19 +0100, André Draszik wrote:
> Hi Philipp,
> 
> On Thu, 2026-07-09 at 14:32 +0200, Philipp Stanner wrote:
> > +Cc Danilo (who is currently concerned with drm_device life times)
> > 
> > On Wed, 2026-07-08 at 16:22 +0100, André Draszik wrote:
> > > 
> > 
> > […]
> > 
> > > Link: https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
> > > Signed-off-by: André Draszik <andre.draszik@linaro.org>
> > 
> > I am tempted to think that this also needs a Fixes and needs to be
> > backported into stable kernels, doesn't it? Especially if the BUG_ON
> > disappears in stable kernels.
> 
> Good point, thanks. I forgot to add this in and will try to find a reasonable
> commit to relate to.
> 
> > 
> > > ---
> > >  drivers/gpu/drm/drm_crtc.c | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > > index 63ead8ba6756..d55f1377ec36 100644
> > > --- a/drivers/gpu/drm/drm_crtc.c
> > > +++ b/drivers/gpu/drm/drm_crtc.c
> > > @@ -501,6 +501,12 @@ void drm_crtc_cleanup(struct drm_crtc *crtc)
> > >  {
> > >  	struct drm_device *dev = crtc->dev;
> > >  
> > > +	/* Ensure our dma_fence_ops remain valid for an RCU grace period after
> > > +	 * the fence is signaled. This is necessary because our dma_fence_ops
> > > +	 * dereference crtc->dev.
> > > +	 */
> > > +	synchronize_rcu();
> > 
> > nit:
> > I guess this is the only place where one can reasonably put the
> > synchronize_rcu(). But I would hint at the RCU delay in the function's
> > docu.
> 
> Unfortunately, this still looks like an incomplete fix -
> https://sashiko.dev/#/patchset/20260618-linux-drm_crtc_fix2-v1-1-c03e77b36f34@linaro.org?part=1
> 
> My next version will simply copy the relevant strings into a custom
> 
> struct drm_crtc_fence {
> 	struct dma_fence base;
> 	char driver_name[32];
> 	char timeline_name[32];
> };
> 
> or similar as part of drm_crtc_create_fence() and just use those as part
> of the dma_fence_ops. That approach should avoid all race conditions and
> corner cases with RCU.


Now wait a second. I don't see how your struct solves any issue that is
not already solved.

static const char *drm_crtc_fence_get_driver_name(struct dma_fence *fence)
{
	struct drm_crtc *crtc = fence_to_crtc(fence);

	return crtc->dev->driver->name;
}

static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
{
	struct drm_crtc *crtc = fence_to_crtc(fence);

	return crtc->timeline_name;
}


The issue here seems to be that 
a) the crtc is made invalid in drm_crtc_cleanup() (memset(0))
b) the drm_dev can disappear after drm_crtc_cleanup()

Both issues stem from the fact that the fence callbacks can keep
running into the driver.

It is true that the fence, being refcounted, can stay alive, but none
of the callbacks be invoked anymore, and your grace period wait
fullfill.

It is a strict dma_fence requirement that a fence issuer / producer
signals all its fences before unload.


The embedded spinlock issue is a separate problem. I think that should
not stall your work here and can be addressed in a separate patch.

Note that the embedded spinlock issue is a known one, and it is very
much related to the fence-decoupling work related to the ops pointer
that Christian has been carrying out. So it can be expected to be a
problem in wide parts of DRM.


Regards
P.