[PATCH] drm/vmwgfx: uninstall the IRQ before freeing the command buffer manager on unload

[PATCH] drm/vmwgfx: uninstall the IRQ before freeing the command buffer manager on unload

[Fan Wu]

vmw_driver_unload() frees the command buffer manager before uninstalling
the device IRQ. vmw_release_device_late() -> vmw_cmdbuf_man_destroy()
frees the manager, while the threaded handler vmw_thread_fn(), released
only later by vmw_irq_uninstall() -> free_irq(), dereferences
dev_priv->cman without a NULL guard and is the sole producer of
schedule_work(&man->work), whose worker vmw_cmdbuf_work_func() recovers
the manager via container_of(). dev_priv->cman is never NULLed on the
unload path, so a handler woken in the window between kfree(man) and
free_irq() runs against freed memory and can re-arm man->work after the
manager's cancel_work_sync() has already returned.

Reorder the unload path to drain pending fences, then uninstall the IRQ,
and only then free the manager: free_irq() guarantees the threaded
handler has exited before kfree(man). vmw_fence_fifo_down() is called
explicitly before the IRQ uninstall so its dma_fence waits are signalled
by the still-live threaded handler (vmw_fences_update() in
vmw_thread_fn()); uninstalling the IRQ first could force pending fence
waits to time out at VMW_FENCE_WAIT_TIMEOUT. vmw_release_device_late()
is left unchanged because it is shared with the hibernation path; its
existing call to vmw_fence_fifo_down() runs against an empty fence list
after the explicit drain above.

Fixes: ef369904aaf7 ("drm/vmwgfx: Move irq bottom half processing to threads")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.5
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 599052d07ae88af2f5775f968fe1e7046a3efe4e..dbe02fe8d8571587733d6f002436bf18dac1887a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1181,10 +1181,11 @@
 	vmw_devcaps_destroy(dev_priv);
 	vmw_vram_manager_fini(dev_priv);
 	ttm_device_fini(&dev_priv->bdev);
-	vmw_release_device_late(dev_priv);
-	vmw_fence_manager_takedown(dev_priv->fman);
+	vmw_fence_fifo_down(dev_priv->fman);
 	if (dev_priv->capabilities & SVGA_CAP_IRQMASK)
 		vmw_irq_uninstall(&dev_priv->drm);
+	vmw_release_device_late(dev_priv);
+	vmw_fence_manager_takedown(dev_priv->fman);

 	ttm_object_device_release(&dev_priv->tdev);

--
2.39.5

Re: [PATCH] drm/vmwgfx: uninstall the IRQ before freeing the command buffer manager on unload

[Maaz Mombasawala]

On 7/14/26 2:45 AM, Fan Wu wrote:
> vmw_driver_unload() frees the command buffer manager before uninstalling
> the device IRQ. vmw_release_device_late() -> vmw_cmdbuf_man_destroy()
> frees the manager, while the threaded handler vmw_thread_fn(), released
> only later by vmw_irq_uninstall() -> free_irq(), dereferences
> dev_priv->cman without a NULL guard and is the sole producer of
> schedule_work(&man->work), whose worker vmw_cmdbuf_work_func() recovers
> the manager via container_of(). dev_priv->cman is never NULLed on the
> unload path, so a handler woken in the window between kfree(man) and
> free_irq() runs against freed memory and can re-arm man->work after the
> manager's cancel_work_sync() has already returned.
> 
> Reorder the unload path to drain pending fences, then uninstall the IRQ,
> and only then free the manager: free_irq() guarantees the threaded
> handler has exited before kfree(man). vmw_fence_fifo_down() is called
> explicitly before the IRQ uninstall so its dma_fence waits are signalled
> by the still-live threaded handler (vmw_fences_update() in
> vmw_thread_fn()); uninstalling the IRQ first could force pending fence
> waits to time out at VMW_FENCE_WAIT_TIMEOUT. vmw_release_device_late()
> is left unchanged because it is shared with the hibernation path; its
> existing call to vmw_fence_fifo_down() runs against an empty fence list
> after the explicit drain above.
> 
> Fixes: ef369904aaf7 ("drm/vmwgfx: Move irq bottom half processing to threads")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 599052d07ae88af2f5775f968fe1e7046a3efe4e..dbe02fe8d8571587733d6f002436bf18dac1887a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1181,10 +1181,11 @@
>  	vmw_devcaps_destroy(dev_priv);
>  	vmw_vram_manager_fini(dev_priv);
>  	ttm_device_fini(&dev_priv->bdev);
> -	vmw_release_device_late(dev_priv);
> -	vmw_fence_manager_takedown(dev_priv->fman);
> +	vmw_fence_fifo_down(dev_priv->fman);
>  	if (dev_priv->capabilities & SVGA_CAP_IRQMASK)
>  		vmw_irq_uninstall(&dev_priv->drm);
> +	vmw_release_device_late(dev_priv);
> +	vmw_fence_manager_takedown(dev_priv->fman);
> 
>  	ttm_object_device_release(&dev_priv->tdev);
> 
> --
> 2.39.5
> 

With this patch we would miss any command buffer processing done on the svga
device in the window between irq_uninstall and vmw_cmdbuf_man_destroy(), so it's
better to leave the takedown order as is.
If you want to address the case where a thread_fn awoken by an earlier irq is
scheduled before irq_uinstall but after vmw_cmdbuf_man_destroy(), then just add checks
to vmw_thread_fn(), though I'm not sure if that case is even possible since the
command buffer contexts should already be taken down at that point.


-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com>

Re: [PATCH] drm/vmwgfx: uninstall the IRQ before freeing the command buffer manager on unload

[Fan Wu]

Hi Maaz,

Thanks for the review. I'll drop this reordering — keeping the IRQ alive
for command-buffer processing during teardown is the right call.

One question before I investigate further: is there an existing teardown
invariant that guarantees an already-woken, but not yet run, CMDBUF or
FENCE threaded handler (vmw_thread_fn) cannot remain pending once cman or
fman teardown begins? If there is, I may well be chasing a non-issue, and
I'd rather not propose an alternative that fights an invariant I'm missing.

I'll re-check whether such a handler can outlive the current destroy
sequence before suggesting anything further.

Best,
Fan Wu


On Jul 16, 2026, at 07:39, Maaz Mombasawala <maaz.mombasawala@broadcom.com> wrote:
> 
> With this patch we would miss any command buffer processing done on the svga
> device in the window between irq_uninstall and vmw_cmdbuf_man_destroy(), so it's
> better to leave the takedown order as is.
> If you want to address the case where a thread_fn awoken by an earlier irq is
> scheduled before irq_uinstall but after vmw_cmdbuf_man_destroy(), then just add checks
> to vmw_thread_fn(), though I'm not sure if that case is even possible since the
> command buffer contexts should already be taken down at that point.
> 

Re: [PATCH] drm/vmwgfx: uninstall the IRQ before freeing the command buffer manager on unload

[Maaz Mombasawala]

On 7/16/26 7:24 PM, Fan Wu wrote:
> Hi Maaz,
> 
> Thanks for the review. I'll drop this reordering — keeping the IRQ alive
> for command-buffer processing during teardown is the right call.
> 
> One question before I investigate further: is there an existing teardown
> invariant that guarantees an already-woken, but not yet run, CMDBUF or
> FENCE threaded handler (vmw_thread_fn) cannot remain pending once cman or
> fman teardown begins? If there is, I may well be chasing a non-issue, and
> I'd rather not propose an alternative that fights an invariant I'm missing.
> 
> I'll re-check whether such a handler can outlive the current destroy
> sequence before suggesting anything further.
> 
> Best,
> Fan Wu
> 

You can drop this, the current teardown sequence processes things in
the correct order workqueue -> thread_fn -> irq.

-- 
Maaz Mombasawala <maaz.mombasawala@broadcom.com>