| Message ID | 1372212882-19082-1-git-send-email-sw0312.kim@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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;