| Message ID | 20240825121452.363342-1-sui.jingfeng@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/etnaviv: Fix missing mutex_destroy() | expand |
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 >
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)
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)
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(-)