* [PATCH 0/2] Balance mutex_init and mutex_destroy calls
@ 2022-11-07 22:46 Maíra Canal
2022-11-07 22:46 ` [PATCH 1/2] drm/v3d: switch to drmm_mutex_init Maíra Canal
2022-11-07 22:46 ` [PATCH 2/2] drm/v3d: add missing mutex_destroy Maíra Canal
0 siblings, 2 replies; 5+ messages in thread
From: Maíra Canal @ 2022-11-07 22:46 UTC (permalink / raw)
To: Melissa Wen, Emma Anholt, David Airlie, Daniel Vetter
Cc: Juan A . Suarez, Maíra Canal, André Almeida,
linux-kernel, dri-devel
This series introduces some changes to assure the correct resource release on
the V3D driver, especially the mutex. Currently, the V3D has no mutex_destroy()
calls, which means that a mutex is being instantiated, but it is not being
released by the end of its use.
So, use the DRM-managed mutex_init variants when possible to manage the mutex
release and add mutex_destroy() calls when not possible.
Best Regards,
- Maíra Canal
Maíra Canal (2):
drm/v3d: switch to drmm_mutex_init
drm/v3d: add missing mutex_destroy
drivers/gpu/drm/v3d/v3d_gem.c | 17 +++++++++++++----
drivers/gpu/drm/v3d/v3d_perfmon.c | 2 ++
2 files changed, 15 insertions(+), 4 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] drm/v3d: switch to drmm_mutex_init
2022-11-07 22:46 [PATCH 0/2] Balance mutex_init and mutex_destroy calls Maíra Canal
@ 2022-11-07 22:46 ` Maíra Canal
2022-11-08 15:56 ` Daniel Vetter
2022-11-07 22:46 ` [PATCH 2/2] drm/v3d: add missing mutex_destroy Maíra Canal
1 sibling, 1 reply; 5+ messages in thread
From: Maíra Canal @ 2022-11-07 22:46 UTC (permalink / raw)
To: Melissa Wen, Emma Anholt, David Airlie, Daniel Vetter
Cc: Juan A . Suarez, Maíra Canal, André Almeida,
linux-kernel, dri-devel
mutex_init() is supposed to be balanced by a call to mutex_destroy(), but
this is not currently happening on the v3d driver.
Considering the introduction of a DRM-managed mutex_init variant, switch
to drmm_mutex_init.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/v3d_gem.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
index b8980440d137..96af1cb5202a 100644
--- a/drivers/gpu/drm/v3d/v3d_gem.c
+++ b/drivers/gpu/drm/v3d/v3d_gem.c
@@ -10,6 +10,7 @@
#include <linux/sched/signal.h>
#include <linux/uaccess.h>
+#include <drm/drm_managed.h>
#include <drm/drm_syncobj.h>
#include <uapi/drm/v3d_drm.h>
@@ -1075,10 +1076,18 @@ v3d_gem_init(struct drm_device *dev)
spin_lock_init(&v3d->mm_lock);
spin_lock_init(&v3d->job_lock);
- mutex_init(&v3d->bo_lock);
- mutex_init(&v3d->reset_lock);
- mutex_init(&v3d->sched_lock);
- mutex_init(&v3d->cache_clean_lock);
+ ret = drmm_mutex_init(dev, &v3d->bo_lock);
+ if (ret)
+ return ret;
+ ret = drmm_mutex_init(dev, &v3d->reset_lock);
+ if (ret)
+ return ret;
+ ret = drmm_mutex_init(dev, &v3d->sched_lock);
+ if (ret)
+ return ret;
+ ret = drmm_mutex_init(dev, &v3d->cache_clean_lock);
+ if (ret)
+ return ret;
/* Note: We don't allocate address 0. Various bits of HW
* treat 0 as special, such as the occlusion query counters
--
2.38.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/v3d: add missing mutex_destroy
2022-11-07 22:46 [PATCH 0/2] Balance mutex_init and mutex_destroy calls Maíra Canal
2022-11-07 22:46 ` [PATCH 1/2] drm/v3d: switch to drmm_mutex_init Maíra Canal
@ 2022-11-07 22:46 ` Maíra Canal
2022-11-08 16:01 ` Daniel Vetter
1 sibling, 1 reply; 5+ messages in thread
From: Maíra Canal @ 2022-11-07 22:46 UTC (permalink / raw)
To: Melissa Wen, Emma Anholt, David Airlie, Daniel Vetter
Cc: Juan A . Suarez, Maíra Canal, André Almeida,
linux-kernel, dri-devel
v3d_perfmon_open_file() instantiates a mutex for a particular file
instance, but it never destroys it by calling mutex_destroy() in
v3d_perfmon_close_file().
Similarly, v3d_perfmon_create_ioctl() instantiates a mutex for a
particular perfmon, but it never destroys it by calling mutex_destroy()
in v3d_perfmon_destroy_ioctl().
So, add the missing mutex_destroy on both cases.
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
drivers/gpu/drm/v3d/v3d_perfmon.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
index 48aaaa972c49..292c73544255 100644
--- a/drivers/gpu/drm/v3d/v3d_perfmon.c
+++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
@@ -113,6 +113,7 @@ void v3d_perfmon_close_file(struct v3d_file_priv *v3d_priv)
idr_for_each(&v3d_priv->perfmon.idr, v3d_perfmon_idr_del, NULL);
idr_destroy(&v3d_priv->perfmon.idr);
mutex_unlock(&v3d_priv->perfmon.lock);
+ mutex_destroy(&v3d_priv->perfmon.lock);
}
int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
@@ -177,6 +178,7 @@ int v3d_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
if (!perfmon)
return -EINVAL;
+ mutex_destroy(&perfmon->lock);
v3d_perfmon_put(perfmon);
return 0;
--
2.38.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/v3d: switch to drmm_mutex_init
2022-11-07 22:46 ` [PATCH 1/2] drm/v3d: switch to drmm_mutex_init Maíra Canal
@ 2022-11-08 15:56 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2022-11-08 15:56 UTC (permalink / raw)
To: Maíra Canal
Cc: André Almeida, Emma Anholt, linux-kernel, dri-devel,
Melissa Wen, Juan A . Suarez
On Mon, Nov 07, 2022 at 07:46:55PM -0300, Maíra Canal wrote:
> mutex_init() is supposed to be balanced by a call to mutex_destroy(), but
> this is not currently happening on the v3d driver.
>
> Considering the introduction of a DRM-managed mutex_init variant, switch
> to drmm_mutex_init.
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/v3d/v3d_gem.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_gem.c b/drivers/gpu/drm/v3d/v3d_gem.c
> index b8980440d137..96af1cb5202a 100644
> --- a/drivers/gpu/drm/v3d/v3d_gem.c
> +++ b/drivers/gpu/drm/v3d/v3d_gem.c
> @@ -10,6 +10,7 @@
> #include <linux/sched/signal.h>
> #include <linux/uaccess.h>
>
> +#include <drm/drm_managed.h>
> #include <drm/drm_syncobj.h>
> #include <uapi/drm/v3d_drm.h>
>
> @@ -1075,10 +1076,18 @@ v3d_gem_init(struct drm_device *dev)
>
> spin_lock_init(&v3d->mm_lock);
> spin_lock_init(&v3d->job_lock);
> - mutex_init(&v3d->bo_lock);
> - mutex_init(&v3d->reset_lock);
> - mutex_init(&v3d->sched_lock);
> - mutex_init(&v3d->cache_clean_lock);
> + ret = drmm_mutex_init(dev, &v3d->bo_lock);
> + if (ret)
> + return ret;
> + ret = drmm_mutex_init(dev, &v3d->reset_lock);
> + if (ret)
> + return ret;
> + ret = drmm_mutex_init(dev, &v3d->sched_lock);
> + if (ret)
> + return ret;
> + ret = drmm_mutex_init(dev, &v3d->cache_clean_lock);
> + if (ret)
> + return ret;
>
> /* Note: We don't allocate address 0. Various bits of HW
> * treat 0 as special, such as the occlusion query counters
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/v3d: add missing mutex_destroy
2022-11-07 22:46 ` [PATCH 2/2] drm/v3d: add missing mutex_destroy Maíra Canal
@ 2022-11-08 16:01 ` Daniel Vetter
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2022-11-08 16:01 UTC (permalink / raw)
To: Maíra Canal
Cc: André Almeida, Emma Anholt, linux-kernel, dri-devel,
Melissa Wen, Juan A . Suarez
On Mon, Nov 07, 2022 at 07:46:56PM -0300, Maíra Canal wrote:
> v3d_perfmon_open_file() instantiates a mutex for a particular file
> instance, but it never destroys it by calling mutex_destroy() in
> v3d_perfmon_close_file().
>
> Similarly, v3d_perfmon_create_ioctl() instantiates a mutex for a
> particular perfmon, but it never destroys it by calling mutex_destroy()
> in v3d_perfmon_destroy_ioctl().
>
> So, add the missing mutex_destroy on both cases.
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
> drivers/gpu/drm/v3d/v3d_perfmon.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_perfmon.c b/drivers/gpu/drm/v3d/v3d_perfmon.c
> index 48aaaa972c49..292c73544255 100644
> --- a/drivers/gpu/drm/v3d/v3d_perfmon.c
> +++ b/drivers/gpu/drm/v3d/v3d_perfmon.c
> @@ -113,6 +113,7 @@ void v3d_perfmon_close_file(struct v3d_file_priv *v3d_priv)
> idr_for_each(&v3d_priv->perfmon.idr, v3d_perfmon_idr_del, NULL);
> idr_destroy(&v3d_priv->perfmon.idr);
> mutex_unlock(&v3d_priv->perfmon.lock);
> + mutex_destroy(&v3d_priv->perfmon.lock);
> }
>
> int v3d_perfmon_create_ioctl(struct drm_device *dev, void *data,
> @@ -177,6 +178,7 @@ int v3d_perfmon_destroy_ioctl(struct drm_device *dev, void *data,
> if (!perfmon)
> return -EINVAL;
>
> + mutex_destroy(&perfmon->lock);
This one looks wrong, I think this should be put into v3d_perfmon_put.
There is also an error case in v3d_perfmon_create_ioctl() that I think you
missed, right above the kfree(perfmon).
With these two fixes this is Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> v3d_perfmon_put(perfmon);
>
> return 0;
> --
> 2.38.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-11-08 16:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07 22:46 [PATCH 0/2] Balance mutex_init and mutex_destroy calls Maíra Canal
2022-11-07 22:46 ` [PATCH 1/2] drm/v3d: switch to drmm_mutex_init Maíra Canal
2022-11-08 15:56 ` Daniel Vetter
2022-11-07 22:46 ` [PATCH 2/2] drm/v3d: add missing mutex_destroy Maíra Canal
2022-11-08 16:01 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).