Re: [PATCH] drm/gem: add mutex lock when using drm_gem_mmap_obj

[Maarten Lankhorst <maarten.lankhorst@canonical.com>]

Op 26-06-13 04:14, Seung-Woo Kim schreef:
> From: YoungJun Cho <yj44.cho@samsung.com>
>
> The drm_gem_mmap_obj() has to be protected with dev->struct_mutex,
> but some caller functions do not. So it adds mutex lock to missing
> callers and adds WARN_ON assertion whether drm_gem_mmap_obj() is
> called with mutex lock or not.
>
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> CC: Rob Clark <robdclark@gmail.com>
> ---
> This patch is based on drm-next branch.
>
>  drivers/gpu/drm/drm_gem.c                 |    4 ++++
>  drivers/gpu/drm/drm_gem_cma_helper.c      |    3 +++
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |    3 +++
>  3 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 4321713..b19bba0 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -661,6 +661,8 @@ EXPORT_SYMBOL(drm_gem_vm_close);
>   * the GEM object is not looked up based on its fake offset. To implement the
>   * DRM mmap operation, drivers should use the drm_gem_mmap() function.
>   *
> + * NOTE: This function has to be protected with dev->struct_mutex
> + *
>   * Return 0 or success or -EINVAL if the object size is smaller than the VMA
>   * size, or if no gem_vm_ops are provided.
>   */
> @@ -669,6 +671,8 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
>  {
>  	struct drm_device *dev = obj->dev;
>  
> +	WARN_ON(!mutex_is_locked(&dev->struct_mutex));
>
Please don't use mutex_is_locked, use lockdep_assert_held, so the cost only exists when PROVE_LOCKING is used..

I know some current code does it wrong, but that is the correct function to use.

~Maarten