All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/gem: add mutex lock when using drm_gem_mmap_obj
@ 2013-06-26  2:14 Seung-Woo Kim
  2013-06-26  7:12 ` Maarten Lankhorst
  0 siblings, 1 reply; 8+ messages in thread
From: Seung-Woo Kim @ 2013-06-26  2:14 UTC (permalink / raw)
  To: dri-devel, airlied
  Cc: kyungmin.park, laurent.pinchart+renesas, sw0312.kim, yj44.cho

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));
+
 	/* Check for valid size. */
 	if (obj_size < vma->vm_end - vma->vm_start)
 		return -EINVAL;
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 9efabce..ce06397 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -487,9 +487,12 @@ static int drm_gem_cma_dmabuf_mmap(struct dma_buf *dmabuf,
 {
 	struct drm_gem_cma_object *cma_obj = dmabuf->priv;
 	struct drm_gem_object *gem_obj = &cma_obj->base;
+	struct drm_device *dev = gem_obj->dev;
 	int ret;
 
+	mutex_lock(&dev->struct_mutex);
 	ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
+	mutex_unlock(&dev->struct_mutex);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 3256693..4fcca8d 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -140,12 +140,15 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
 		struct vm_area_struct *vma)
 {
 	struct drm_gem_object *obj = buffer->priv;
+	struct drm_device *dev = obj->dev;
 	int ret = 0;
 
 	if (WARN_ON(!obj->filp))
 		return -EINVAL;
 
+	mutex_lock(&dev->struct_mutex);
 	ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma);
+	mutex_unlock(&dev->struct_mutex);
 	if (ret < 0)
 		return ret;
 
-- 
1.7.4.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26  2:14 [PATCH] drm/gem: add mutex lock when using drm_gem_mmap_obj Seung-Woo Kim
@ 2013-06-26  7:12 ` Maarten Lankhorst
  2013-06-26  8:28   ` YoungJun Cho
  2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
  0 siblings, 2 replies; 8+ messages in thread
From: Maarten Lankhorst @ 2013-06-26  7:12 UTC (permalink / raw)
  To: Seung-Woo Kim
  Cc: kyungmin.park, laurent.pinchart+renesas, yj44.cho, dri-devel

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26  7:12 ` Maarten Lankhorst
@ 2013-06-26  8:28   ` YoungJun Cho
  2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
  1 sibling, 0 replies; 8+ messages in thread
From: YoungJun Cho @ 2013-06-26  8:28 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: kyungmin.park, laurent.pinchart+renesas, Seung-Woo Kim, dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2289 bytes --]

On Jun 26, 2013 4:13 PM, "Maarten Lankhorst" <
maarten.lankhorst@canonical.com> wrote:
>
> 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
>

Thank you for nice comments!
I will update it again.

Best regards YJ
_______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

[-- Attachment #1.2: Type: text/html, Size: 3409 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26  7:12 ` Maarten Lankhorst
  2013-06-26  8:28   ` YoungJun Cho
@ 2013-06-26 23:39   ` Seung-Woo Kim
  2013-06-27  4:53     ` Maarten Lankhorst
                       ` (3 more replies)
  1 sibling, 4 replies; 8+ messages in thread
From: Seung-Woo Kim @ 2013-06-26 23:39 UTC (permalink / raw)
  To: dri-devel, airlied
  Cc: laurent.pinchart+renesas, sw0312.kim, yj44.cho, kyungmin.park

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 assertion to check 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.

Changes since v1:
- Use lockdep_assert_held() instead of mutex_is_locked() as Maarten commented
- Fix commit message about assertion

 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(+)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4321713..34c0be7 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;
 
+	lockdep_assert_held(&dev->struct_mutex);
+
 	/* Check for valid size. */
 	if (obj_size < vma->vm_end - vma->vm_start)
 		return -EINVAL;
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
index 9efabce..ce06397 100644
--- a/drivers/gpu/drm/drm_gem_cma_helper.c
+++ b/drivers/gpu/drm/drm_gem_cma_helper.c
@@ -487,9 +487,12 @@ static int drm_gem_cma_dmabuf_mmap(struct dma_buf *dmabuf,
 {
 	struct drm_gem_cma_object *cma_obj = dmabuf->priv;
 	struct drm_gem_object *gem_obj = &cma_obj->base;
+	struct drm_device *dev = gem_obj->dev;
 	int ret;
 
+	mutex_lock(&dev->struct_mutex);
 	ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
+	mutex_unlock(&dev->struct_mutex);
 	if (ret < 0)
 		return ret;
 
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index 3256693..4fcca8d 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -140,12 +140,15 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
 		struct vm_area_struct *vma)
 {
 	struct drm_gem_object *obj = buffer->priv;
+	struct drm_device *dev = obj->dev;
 	int ret = 0;
 
 	if (WARN_ON(!obj->filp))
 		return -EINVAL;
 
+	mutex_lock(&dev->struct_mutex);
 	ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma);
+	mutex_unlock(&dev->struct_mutex);
 	if (ret < 0)
 		return ret;
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
@ 2013-06-27  4:53     ` Maarten Lankhorst
  2013-06-27 13:00     ` Laurent Pinchart
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Maarten Lankhorst @ 2013-06-27  4:53 UTC (permalink / raw)
  To: Seung-Woo Kim
  Cc: laurent.pinchart+renesas, kyungmin.park, yj44.cho, dri-devel

Op 27-06-13 01:39, 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 assertion to check 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>
> ---
>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
  2013-06-27  4:53     ` Maarten Lankhorst
@ 2013-06-27 13:00     ` Laurent Pinchart
  2013-06-27 14:16     ` Rob Clark
  2013-06-27 14:20     ` Maarten Lankhorst
  3 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2013-06-27 13:00 UTC (permalink / raw)
  To: Seung-Woo Kim, robdclark; +Cc: kyungmin.park, yj44.cho, dri-devel

Hi,


On Thursday 27 June 2013 08:39:58 Seung-Woo Kim wrote:
> 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 assertion to check 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>

Thanks for the patch.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> This patch is based on drm-next branch.
> 
> Changes since v1:
> - Use lockdep_assert_held() instead of mutex_is_locked() as Maarten
> commented
> - Fix commit message about assertion
> 
>  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(+)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 4321713..34c0be7 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;
> 
> +	lockdep_assert_held(&dev->struct_mutex);
> +
>  	/* Check for valid size. */
>  	if (obj_size < vma->vm_end - vma->vm_start)
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c
> b/drivers/gpu/drm/drm_gem_cma_helper.c index 9efabce..ce06397 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -487,9 +487,12 @@ static int drm_gem_cma_dmabuf_mmap(struct dma_buf
> *dmabuf, {
>  	struct drm_gem_cma_object *cma_obj = dmabuf->priv;
>  	struct drm_gem_object *gem_obj = &cma_obj->base;
> +	struct drm_device *dev = gem_obj->dev;
>  	int ret;
> 
> +	mutex_lock(&dev->struct_mutex);
>  	ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
> +	mutex_unlock(&dev->struct_mutex);
>  	if (ret < 0)
>  		return ret;
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c index 3256693..4fcca8d 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> @@ -140,12 +140,15 @@ static int omap_gem_dmabuf_mmap(struct dma_buf
> *buffer, struct vm_area_struct *vma)
>  {
>  	struct drm_gem_object *obj = buffer->priv;
> +	struct drm_device *dev = obj->dev;
>  	int ret = 0;
> 
>  	if (WARN_ON(!obj->filp))
>  		return -EINVAL;
> 
> +	mutex_lock(&dev->struct_mutex);
>  	ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma);
> +	mutex_unlock(&dev->struct_mutex);
>  	if (ret < 0)
>  		return ret;
-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
  2013-06-27  4:53     ` Maarten Lankhorst
  2013-06-27 13:00     ` Laurent Pinchart
@ 2013-06-27 14:16     ` Rob Clark
  2013-06-27 14:20     ` Maarten Lankhorst
  3 siblings, 0 replies; 8+ messages in thread
From: Rob Clark @ 2013-06-27 14:16 UTC (permalink / raw)
  To: Seung-Woo Kim
  Cc: laurent.pinchart+renesas, yj44.cho, kyungmin.park, dri-devel

On Wed, Jun 26, 2013 at 7:39 PM, Seung-Woo Kim <sw0312.kim@samsung.com> wrote:
> 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 assertion to check whether drm_gem_mmap_obj() is
> called with mutex lock or not.

Yeah, looks like since drm_gem_mmap_obj() is using
drm_vm_open_locked() (vs drm_vm_open() / vm_ops->open()), we need
this.  I missed that when reviewing the original patch to
drm_gem_mmap_obj()'ify things.

Reviewed-by: Rob Clark <robdclark@gmail.com>


> 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.
>
> Changes since v1:
> - Use lockdep_assert_held() instead of mutex_is_locked() as Maarten commented
> - Fix commit message about assertion
>
>  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(+)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 4321713..34c0be7 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;
>
> +       lockdep_assert_held(&dev->struct_mutex);
> +
>         /* Check for valid size. */
>         if (obj_size < vma->vm_end - vma->vm_start)
>                 return -EINVAL;
> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c
> index 9efabce..ce06397 100644
> --- a/drivers/gpu/drm/drm_gem_cma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c
> @@ -487,9 +487,12 @@ static int drm_gem_cma_dmabuf_mmap(struct dma_buf *dmabuf,
>  {
>         struct drm_gem_cma_object *cma_obj = dmabuf->priv;
>         struct drm_gem_object *gem_obj = &cma_obj->base;
> +       struct drm_device *dev = gem_obj->dev;
>         int ret;
>
> +       mutex_lock(&dev->struct_mutex);
>         ret = drm_gem_mmap_obj(gem_obj, gem_obj->size, vma);
> +       mutex_unlock(&dev->struct_mutex);
>         if (ret < 0)
>                 return ret;
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> index 3256693..4fcca8d 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
> @@ -140,12 +140,15 @@ static int omap_gem_dmabuf_mmap(struct dma_buf *buffer,
>                 struct vm_area_struct *vma)
>  {
>         struct drm_gem_object *obj = buffer->priv;
> +       struct drm_device *dev = obj->dev;
>         int ret = 0;
>
>         if (WARN_ON(!obj->filp))
>                 return -EINVAL;
>
> +       mutex_lock(&dev->struct_mutex);
>         ret = drm_gem_mmap_obj(obj, omap_gem_mmap_size(obj), vma);
> +       mutex_unlock(&dev->struct_mutex);
>         if (ret < 0)
>                 return ret;
>
> --
> 1.7.9.5
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] drm/gem: add mutex lock when using drm_gem_mmap_obj
  2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
                       ` (2 preceding siblings ...)
  2013-06-27 14:16     ` Rob Clark
@ 2013-06-27 14:20     ` Maarten Lankhorst
  3 siblings, 0 replies; 8+ messages in thread
From: Maarten Lankhorst @ 2013-06-27 14:20 UTC (permalink / raw)
  To: Seung-Woo Kim
  Cc: laurent.pinchart+renesas, kyungmin.park, yj44.cho, dri-devel

Op 27-06-13 01:39, 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 assertion to check 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>
>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-06-27 14:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-26  2:14 [PATCH] drm/gem: add mutex lock when using drm_gem_mmap_obj Seung-Woo Kim
2013-06-26  7:12 ` Maarten Lankhorst
2013-06-26  8:28   ` YoungJun Cho
2013-06-26 23:39   ` [PATCH v2] " Seung-Woo Kim
2013-06-27  4:53     ` Maarten Lankhorst
2013-06-27 13:00     ` Laurent Pinchart
2013-06-27 14:16     ` Rob Clark
2013-06-27 14:20     ` Maarten Lankhorst

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.