mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: drm@redhat.com
Cc: Gerd Hoffmann <kraxel@redhat.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>, Sean Paul <sean@poorly.run>,
	David Airlie <airlied@linux.ie>, Daniel Vetter <daniel@ffwll.ch>,
	dri-devel@lists.freedesktop.org (open list:DRM DRIVERS),
	linux-kernel@vger.kernel.org (open list)
Subject: [PATCH] drm/ttm: fix mmap refcounting
Date: Wed, 13 Nov 2019 14:56:12 +0100	[thread overview]
Message-ID: <20191113135612.19679-1-kraxel@redhat.com> (raw)

When mapping ttm objects via drm_gem_ttm_mmap() helper
drm_gem_mmap_obj() will take an object reference.  That gets
never released due to ttm having its own reference counting.

Fix that by dropping the gem object reference once the ttm mmap
completed (and ttm refcount got bumped).

For that to work properly the drm_gem_object_get() call in
drm_gem_ttm_mmap() must be moved so it happens before calling
obj->funcs->mmap(), otherwise the gem refcount would go down
to zero.

Fixes: 231927d939f0 ("drm/ttm: add drm_gem_ttm_mmap()")
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_gem.c            | 24 ++++++++++++++----------
 drivers/gpu/drm/drm_gem_ttm_helper.c | 13 ++++++++++++-
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 2f2b889096b0..000fa4a1899f 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1105,21 +1105,33 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 	if (obj_size < vma->vm_end - vma->vm_start)
 		return -EINVAL;
 
+	/* Take a ref for this mapping of the object, so that the fault
+	 * handler can dereference the mmap offset's pointer to the object.
+	 * This reference is cleaned up by the corresponding vm_close
+	 * (which should happen whether the vma was created by this call, or
+	 * by a vm_open due to mremap or partial unmap or whatever).
+	 */
+	drm_gem_object_get(obj);
+
 	if (obj->funcs && obj->funcs->mmap) {
 		/* Remove the fake offset */
 		vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
 
 		ret = obj->funcs->mmap(obj, vma);
-		if (ret)
+		if (ret) {
+			drm_gem_object_put_unlocked(obj);
 			return ret;
+		}
 		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
 	} else {
 		if (obj->funcs && obj->funcs->vm_ops)
 			vma->vm_ops = obj->funcs->vm_ops;
 		else if (dev->driver->gem_vm_ops)
 			vma->vm_ops = dev->driver->gem_vm_ops;
-		else
+		else {
+			drm_gem_object_put_unlocked(obj);
 			return -EINVAL;
+		}
 
 		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
 		vma->vm_page_prot = pgprot_writecombine(vm_get_page_prot(vma->vm_flags));
@@ -1128,14 +1140,6 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 
 	vma->vm_private_data = obj;
 
-	/* Take a ref for this mapping of the object, so that the fault
-	 * handler can dereference the mmap offset's pointer to the object.
-	 * This reference is cleaned up by the corresponding vm_close
-	 * (which should happen whether the vma was created by this call, or
-	 * by a vm_open due to mremap or partial unmap or whatever).
-	 */
-	drm_gem_object_get(obj);
-
 	return 0;
 }
 EXPORT_SYMBOL(drm_gem_mmap_obj);
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c
index 7412bfc5c05a..605a8a3da7f9 100644
--- a/drivers/gpu/drm/drm_gem_ttm_helper.c
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -64,8 +64,19 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 		     struct vm_area_struct *vma)
 {
 	struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
+	int ret;
 
-	return ttm_bo_mmap_obj(vma, bo);
+	ret = ttm_bo_mmap_obj(vma, bo);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * ttm has its own object refcounting, so drop gem reference
+	 * to avoid double accounting counting.
+	 */
+	drm_gem_object_put_unlocked(gem);
+
+	return 0;
 }
 EXPORT_SYMBOL(drm_gem_ttm_mmap);
 
-- 
2.18.1


             reply	other threads:[~2019-11-13 13:56 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-13 13:56 Gerd Hoffmann [this message]
2019-11-13 16:33 ` Daniel Vetter
2019-11-14  8:33 ` Thomas Zimmermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191113135612.19679-1-kraxel@redhat.com \
    --to=kraxel@redhat.com \
    --cc=airlied@linux.ie \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=drm@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=sean@poorly.run \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®