From ee61c7303f84e2ef978aaed9b9640476c63ba586 Mon Sep 17 00:00:00 2001 From: Kristian Hogsberg Date: Mon, 2 Dec 2013 17:36:17 -0800 Subject: drm: Don't reference objects in the flink name idr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There's no reason to keep a reference to objects in the name idr. Each handle to an object has a reference to the object and just before we destroy the last handle we take the object out of the name idr. Thus, if an object is in the name idr, there's at least one reference to the object. Or to put it another way, the name idr reference will never keep the object alive. It just looks like it, which is confusing. Signed-off-by: Kristian Høgsberg Reviewed-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 15 --------------- 1 file changed, 15 deletions(-) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 4761adedad2a..3ff39bb0402d 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -175,11 +175,6 @@ drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) mutex_unlock(&filp->prime.lock); } -static void drm_gem_object_ref_bug(struct kref *list_kref) -{ - BUG(); -} - /** * Called after the last handle to the object has been closed * @@ -195,13 +190,6 @@ static void drm_gem_object_handle_free(struct drm_gem_object *obj) if (obj->name) { idr_remove(&dev->object_name_idr, obj->name); obj->name = 0; - /* - * The object name held a reference to this object, drop - * that now. - * - * This cannot be the last reference, since the handle holds one too. - */ - kref_put(&obj->refcount, drm_gem_object_ref_bug); } } @@ -602,9 +590,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, goto err; obj->name = ret; - - /* Allocate a reference for the name table. */ - drm_gem_object_reference(obj); } args->name = (uint64_t) obj->name; -- cgit v1.2.3 From b04a590623661132fbafdda53a6566b227dc39cf Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 11 Dec 2013 14:24:46 +0100 Subject: drm: store the gem vma offset manager in a typed pointer This was hidden in a generic void * dev->mm_private. But only ever used for gem. But thanks to this fake generic pretension no one noticed that Rob's drm drivers are now all broken. So just give the offset manager a type pointer and fix up msm, omapdrm and tilcdc. v2: Fixup compile fail. v3: Fixup rebase fail that David spotted. Cc: David Herrmann Cc: Rob Clark Signed-off-by: Daniel Vetter Signed-off-by: Dave Airlie --- drivers/gpu/drm/drm_gem.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 3ff39bb0402d..bed5c3bfed76 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -91,19 +91,19 @@ int drm_gem_init(struct drm_device *dev) { - struct drm_gem_mm *mm; + struct drm_vma_offset_manager *vma_offset_manager; mutex_init(&dev->object_name_lock); idr_init(&dev->object_name_idr); - mm = kzalloc(sizeof(struct drm_gem_mm), GFP_KERNEL); - if (!mm) { + vma_offset_manager = kzalloc(sizeof(*vma_offset_manager), GFP_KERNEL); + if (!vma_offset_manager) { DRM_ERROR("out of memory\n"); return -ENOMEM; } - dev->mm_private = mm; - drm_vma_offset_manager_init(&mm->vma_manager, + dev->vma_offset_manager = vma_offset_manager; + drm_vma_offset_manager_init(vma_offset_manager, DRM_FILE_PAGE_OFFSET_START, DRM_FILE_PAGE_OFFSET_SIZE); @@ -113,11 +113,10 @@ drm_gem_init(struct drm_device *dev) void drm_gem_destroy(struct drm_device *dev) { - struct drm_gem_mm *mm = dev->mm_private; - drm_vma_offset_manager_destroy(&mm->vma_manager); - kfree(mm); - dev->mm_private = NULL; + drm_vma_offset_manager_destroy(dev->vma_offset_manager); + kfree(dev->vma_offset_manager); + dev->vma_offset_manager = NULL; } /** @@ -362,9 +361,8 @@ void drm_gem_free_mmap_offset(struct drm_gem_object *obj) { struct drm_device *dev = obj->dev; - struct drm_gem_mm *mm = dev->mm_private; - drm_vma_offset_remove(&mm->vma_manager, &obj->vma_node); + drm_vma_offset_remove(dev->vma_offset_manager, &obj->vma_node); } EXPORT_SYMBOL(drm_gem_free_mmap_offset); @@ -386,9 +384,8 @@ int drm_gem_create_mmap_offset_size(struct drm_gem_object *obj, size_t size) { struct drm_device *dev = obj->dev; - struct drm_gem_mm *mm = dev->mm_private; - return drm_vma_offset_add(&mm->vma_manager, &obj->vma_node, + return drm_vma_offset_add(dev->vma_offset_manager, &obj->vma_node, size / PAGE_SIZE); } EXPORT_SYMBOL(drm_gem_create_mmap_offset_size); @@ -818,7 +815,6 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) { struct drm_file *priv = filp->private_data; struct drm_device *dev = priv->minor->dev; - struct drm_gem_mm *mm = dev->mm_private; struct drm_gem_object *obj; struct drm_vma_offset_node *node; int ret = 0; @@ -828,7 +824,8 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma) mutex_lock(&dev->struct_mutex); - node = drm_vma_offset_exact_lookup(&mm->vma_manager, vma->vm_pgoff, + node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, + vma->vm_pgoff, vma_pages(vma)); if (!node) { mutex_unlock(&dev->struct_mutex); -- cgit v1.2.3 From 6ab11a2635ce988ebc2e798947beb72cf7324119 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 20 Jan 2014 08:21:54 +0100 Subject: drm/gem: Always initialize the gem object in object_init At least drm/i915 expects that the obj->dev pointer is set even in failure paths. Specifically when the shmem initialization fails we call i915_gem_object_free which needs to deref obj->base.dev to get at the slab pointer in the device private structure. And the shmem allocation can easily fail when userspace is hitting open file limits. Doing the structure init even when the shmem file allocation fails prevents this Oops. This is a regression from commit 89c8233f82d9c8af5b20e72e4a185a38a7d3c50b Author: David Herrmann Date: Thu Jul 11 11:56:32 2013 +0200 drm/gem: simplify object initialization v2: Add regression note which Chris supplied. Testcase: igt/gem_fd_exhaustion Reported-and-Suggested-by: Linus Torvalds Cc: Linus Torvalds References: http://lists.freedesktop.org/archives/intel-gfx/2014-January/038433.html Cc: stable@vger.kernel.org Reviewed-by: David Herrmann Cc: David Herrmann Signed-off-by: Daniel Vetter --- drivers/gpu/drm/drm_gem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/drm_gem.c') diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index bed5c3bfed76..5bbad873c798 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -128,11 +128,12 @@ int drm_gem_object_init(struct drm_device *dev, { struct file *filp; + drm_gem_private_object_init(dev, obj, size); + filp = shmem_file_setup("drm mm object", size, VM_NORESERVE); if (IS_ERR(filp)) return PTR_ERR(filp); - drm_gem_private_object_init(dev, obj, size); obj->filp = filp; return 0; -- cgit v1.2.3