[PATCH] drm/etnaviv: Fix missing mutex_destroy()

[PATCH] drm/etnaviv: Fix missing mutex_destroy()

[Sui Jingfeng]

Currently, the calling of mutex_destroy() is ignored on error handling
code path. It is safe for now, since mutex_destroy() actually does
nothing in non-debug builds. But the mutex_destroy() is used to mark
the mutex uninitialized on debug builds, and any subsequent use of the
mutex is forbidden.

It also could lead to problems if mutex_destroy() gets extended, add
missing mutex_destroy() to eliminate potential concerns.

Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>
---
 drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_drv.c    | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_gem.c    | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c    | 5 +++++
 drivers/gpu/drm/etnaviv/etnaviv_mmu.c    | 2 +-
 5 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
index 721d633aece9..1edc02022be4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
@@ -79,6 +79,9 @@ void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc)
 {
 	dma_free_wc(suballoc->dev, SUBALLOC_SIZE, suballoc->vaddr,
 		    suballoc->paddr);
+
+	mutex_destroy(&suballoc->lock);
+
 	kfree(suballoc);
 }
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6500f3999c5f..7844cd961a29 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -564,6 +564,7 @@ static int etnaviv_bind(struct device *dev)
 out_destroy_suballoc:
 	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
 out_free_priv:
+	mutex_destroy(&priv->gem_lock);
 	kfree(priv);
 out_put:
 	drm_dev_put(drm);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
index fe665ca20c02..b68e3b235a7d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
@@ -515,6 +515,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
 	etnaviv_obj->ops->release(etnaviv_obj);
 	drm_gem_object_release(obj);
 
+	mutex_destroy(&etnaviv_obj->lock);
 	kfree(etnaviv_obj);
 }
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index af52922ff494..d6acc4c68102 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1929,8 +1929,13 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
 
 static void etnaviv_gpu_platform_remove(struct platform_device *pdev)
 {
+	struct etnaviv_gpu *gpu = dev_get_drvdata(&pdev->dev);
+
 	component_del(&pdev->dev, &gpu_ops);
 	pm_runtime_disable(&pdev->dev);
+
+	mutex_destroy(&gpu->lock);
+	mutex_destroy(&gpu->sched_lock);
 }
 
 static int etnaviv_gpu_rpm_suspend(struct device *dev)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index e3be16165c86..ed6c42384856 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -361,7 +361,7 @@ static void etnaviv_iommu_context_free(struct kref *kref)
 		container_of(kref, struct etnaviv_iommu_context, refcount);
 
 	etnaviv_cmdbuf_suballoc_unmap(context, &context->cmdbuf_mapping);
-
+	mutex_destroy(&context->lock);
 	context->global->ops->free(context);
 }
 void etnaviv_iommu_context_put(struct etnaviv_iommu_context *context)
-- 
2.34.1

Re: [PATCH] drm/etnaviv: Fix missing mutex_destroy()

[Christian Gmeiner]

Am So., 25. Aug. 2024 um 14:15 Uhr schrieb Sui Jingfeng
<sui.jingfeng@linux.dev>:
>
> Currently, the calling of mutex_destroy() is ignored on error handling
> code path. It is safe for now, since mutex_destroy() actually does
> nothing in non-debug builds. But the mutex_destroy() is used to mark
> the mutex uninitialized on debug builds, and any subsequent use of the
> mutex is forbidden.
>
> It also could lead to problems if mutex_destroy() gets extended, add
> missing mutex_destroy() to eliminate potential concerns.
>
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 +++
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c    | 1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c    | 1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c    | 5 +++++
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.c    | 2 +-
>  5 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> index 721d633aece9..1edc02022be4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> @@ -79,6 +79,9 @@ void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc)
>  {
>         dma_free_wc(suballoc->dev, SUBALLOC_SIZE, suballoc->vaddr,
>                     suballoc->paddr);
> +
> +       mutex_destroy(&suballoc->lock);
> +
>         kfree(suballoc);
>  }
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..7844cd961a29 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -564,6 +564,7 @@ static int etnaviv_bind(struct device *dev)
>  out_destroy_suballoc:
>         etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
>  out_free_priv:
> +       mutex_destroy(&priv->gem_lock);
>         kfree(priv);
>  out_put:
>         drm_dev_put(drm);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index fe665ca20c02..b68e3b235a7d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -515,6 +515,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>         etnaviv_obj->ops->release(etnaviv_obj);
>         drm_gem_object_release(obj);
>
> +       mutex_destroy(&etnaviv_obj->lock);
>         kfree(etnaviv_obj);
>  }
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index af52922ff494..d6acc4c68102 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1929,8 +1929,13 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
>
>  static void etnaviv_gpu_platform_remove(struct platform_device *pdev)
>  {
> +       struct etnaviv_gpu *gpu = dev_get_drvdata(&pdev->dev);
> +
>         component_del(&pdev->dev, &gpu_ops);
>         pm_runtime_disable(&pdev->dev);
> +
> +       mutex_destroy(&gpu->lock);
> +       mutex_destroy(&gpu->sched_lock);
>  }
>
>  static int etnaviv_gpu_rpm_suspend(struct device *dev)
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index e3be16165c86..ed6c42384856 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -361,7 +361,7 @@ static void etnaviv_iommu_context_free(struct kref *kref)
>                 container_of(kref, struct etnaviv_iommu_context, refcount);
>
>         etnaviv_cmdbuf_suballoc_unmap(context, &context->cmdbuf_mapping);
> -
> +       mutex_destroy(&context->lock);
>         context->global->ops->free(context);
>  }
>  void etnaviv_iommu_context_put(struct etnaviv_iommu_context *context)
> --
> 2.34.1
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

Re: [PATCH] drm/etnaviv: Fix missing mutex_destroy()

[Lucas Stach]

Am Sonntag, dem 25.08.2024 um 20:14 +0800 schrieb Sui Jingfeng:
> Currently, the calling of mutex_destroy() is ignored on error handling
> code path. It is safe for now, since mutex_destroy() actually does
> nothing in non-debug builds. But the mutex_destroy() is used to mark
> the mutex uninitialized on debug builds, and any subsequent use of the
> mutex is forbidden.
> 
> It also could lead to problems if mutex_destroy() gets extended, add
> missing mutex_destroy() to eliminate potential concerns.
> 
> Signed-off-by: Sui Jingfeng <sui.jingfeng@linux.dev>

Thanks, applied to etnaviv/next.

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 +++
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c    | 1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gem.c    | 1 +
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c    | 5 +++++
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.c    | 2 +-
>  5 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> index 721d633aece9..1edc02022be4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> @@ -79,6 +79,9 @@ void etnaviv_cmdbuf_suballoc_destroy(struct etnaviv_cmdbuf_suballoc *suballoc)
>  {
>  	dma_free_wc(suballoc->dev, SUBALLOC_SIZE, suballoc->vaddr,
>  		    suballoc->paddr);
> +
> +	mutex_destroy(&suballoc->lock);
> +
>  	kfree(suballoc);
>  }
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6500f3999c5f..7844cd961a29 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -564,6 +564,7 @@ static int etnaviv_bind(struct device *dev)
>  out_destroy_suballoc:
>  	etnaviv_cmdbuf_suballoc_destroy(priv->cmdbuf_suballoc);
>  out_free_priv:
> +	mutex_destroy(&priv->gem_lock);
>  	kfree(priv);
>  out_put:
>  	drm_dev_put(drm);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> index fe665ca20c02..b68e3b235a7d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c
> @@ -515,6 +515,7 @@ void etnaviv_gem_free_object(struct drm_gem_object *obj)
>  	etnaviv_obj->ops->release(etnaviv_obj);
>  	drm_gem_object_release(obj);
>  
> +	mutex_destroy(&etnaviv_obj->lock);
>  	kfree(etnaviv_obj);
>  }
>  
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index af52922ff494..d6acc4c68102 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1929,8 +1929,13 @@ static int etnaviv_gpu_platform_probe(struct platform_device *pdev)
>  
>  static void etnaviv_gpu_platform_remove(struct platform_device *pdev)
>  {
> +	struct etnaviv_gpu *gpu = dev_get_drvdata(&pdev->dev);
> +
>  	component_del(&pdev->dev, &gpu_ops);
>  	pm_runtime_disable(&pdev->dev);
> +
> +	mutex_destroy(&gpu->lock);
> +	mutex_destroy(&gpu->sched_lock);
>  }
>  
>  static int etnaviv_gpu_rpm_suspend(struct device *dev)
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index e3be16165c86..ed6c42384856 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -361,7 +361,7 @@ static void etnaviv_iommu_context_free(struct kref *kref)
>  		container_of(kref, struct etnaviv_iommu_context, refcount);
>  
>  	etnaviv_cmdbuf_suballoc_unmap(context, &context->cmdbuf_mapping);
> -
> +	mutex_destroy(&context->lock);
>  	context->global->ops->free(context);
>  }
>  void etnaviv_iommu_context_put(struct etnaviv_iommu_context *context)