[PATCH] drm/vc4: drain GEM timer and work on teardown

[PATCH] drm/vc4: drain GEM timer and work on teardown

[Hongyan Xu]

The hangcheck timer can rearm itself or queue reset_work, while the V3D
interrupt queues job_done_work. vc4_gem_destroy() releases state
embedded in vc4_dev without first draining those asynchronous callbacks.

Shut down the timer so it cannot be rearmed, then cancel both work items
before GEM state is destroyed.

Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
---
 drivers/gpu/drm/vc4/vc4_gem.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
index e231c906709c..2443b62df89c 100644
--- a/drivers/gpu/drm/vc4/vc4_gem.c
+++ b/drivers/gpu/drm/vc4/vc4_gem.c
@@ -1194,6 +1194,10 @@ static void vc4_gem_destroy(struct drm_device *dev, void *unused)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 
+	timer_shutdown_sync(&vc4->hangcheck.timer);
+	cancel_work_sync(&vc4->hangcheck.reset_work);
+	cancel_work_sync(&vc4->job_done_work);
+
 	/* Waiting for exec to finish would need to be done before
 	 * unregistering V3D.
 	 */
-- 
2.50.1.windows.1

Re: [PATCH] drm/vc4: drain GEM timer and work on teardown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/vc4: memory leak of vc4_exec_info due to unhandled job_done_list
--

commit 789f23627b59e9b2f1e7e763001ed796ee2df14b
Author: Hongyan Xu <getshell@seu.edu.cn>

drm/vc4: drain GEM timer and work on teardown

This commit updates vc4_gem_destroy() to shut down the hangcheck timer and
cancel both the hangcheck reset work and the job done work before destroying
the GEM state. This prevents asynchronous callbacks from executing after
the device structure has been released.

> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> @@ -1202,6 +1202,10 @@ static void vc4_gem_destroy(struct drm_device *dev, void *unused)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  
> +	timer_shutdown_sync(&vc4->hangcheck.timer);
> +	cancel_work_sync(&vc4->hangcheck.reset_work);
> +	cancel_work_sync(&vc4->job_done_work);

[Severity: Medium]
In drivers/gpu/drm/vc4/vc4_gem.c:vc4_gem_destroy(), if there are completed
jobs waiting in vc4->job_done_list when this executes, does cancelling the
work here strand those jobs?

Since the scheduled cleanup work is cancelled and will not run, and
vc4_gem_destroy() does not manually flush or process job_done_list
afterwards, it seems any vc4_exec_info objects left in the list and their
associated GEM objects will be permanently leaked.

> +
>  	/* Waiting for exec to finish would need to be done before
>  	 * unregistering V3D.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928134100.420-1-getshell@seu.edu.cn?part=1

Re: [PATCH] drm/vc4: drain GEM timer and work on teardown

[Maíra Canal]

Hi Hongyan,

This issue was already addressed in [1].

[1] 
https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/94cf5971b1d04c6382437387c4dc3836cc7a1437

Best regards,
- Maíra

On 28/09/26 10:41, Hongyan Xu wrote:
> The hangcheck timer can rearm itself or queue reset_work, while the V3D
> interrupt queues job_done_work. vc4_gem_destroy() releases state
> embedded in vc4_dev without first draining those asynchronous callbacks.
> 
> Shut down the timer so it cannot be rearmed, then cancel both work items
> before GEM state is destroyed.
> 
> Fixes: d5b1a78a772f ("drm/vc4: Add support for drawing 3D frames.")
> Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
> ---
>   drivers/gpu/drm/vc4/vc4_gem.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_gem.c b/drivers/gpu/drm/vc4/vc4_gem.c
> index e231c906709c..2443b62df89c 100644
> --- a/drivers/gpu/drm/vc4/vc4_gem.c
> +++ b/drivers/gpu/drm/vc4/vc4_gem.c
> @@ -1194,6 +1194,10 @@ static void vc4_gem_destroy(struct drm_device *dev, void *unused)
>   {
>   	struct vc4_dev *vc4 = to_vc4_dev(dev);
>   
> +	timer_shutdown_sync(&vc4->hangcheck.timer);
> +	cancel_work_sync(&vc4->hangcheck.reset_work);
> +	cancel_work_sync(&vc4->job_done_work);
> +
>   	/* Waiting for exec to finish would need to be done before
>   	 * unregistering V3D.
>   	 */