[07/18] drm/mga200g: Hold a proper reference for cursor_set
diff mbox

Message ID 1436477570-4936-8-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter July 9, 2015, 9:32 p.m. UTC
Looking up an obj, immediate dropping the acquired reference and then
continuing to use it isn't how this is supposed to work. Fix this by
holding a reference for the entire function.

While at it stop grabbing dev->struct_mutex, it doesn't protect
anything here.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

Comments

Thierry Reding Aug. 10, 2015, 10:46 a.m. UTC | #1
On Thu, Jul 09, 2015 at 11:32:39PM +0200, Daniel Vetter wrote:
> Looking up an obj, immediate dropping the acquired reference and then
> continuing to use it isn't how this is supposed to work. Fix this by
> holding a reference for the entire function.
> 
> While at it stop grabbing dev->struct_mutex, it doesn't protect
> anything here.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/mgag200/mgag200_cursor.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Thierry Reding <treding@nvidia.com>

Patch
diff mbox

diff --git a/drivers/gpu/drm/mgag200/mgag200_cursor.c b/drivers/gpu/drm/mgag200/mgag200_cursor.c
index 9f9780b7ddf0..4f2068fe5d88 100644
--- a/drivers/gpu/drm/mgag200/mgag200_cursor.c
+++ b/drivers/gpu/drm/mgag200/mgag200_cursor.c
@@ -70,18 +70,22 @@  int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	BUG_ON(pixels_2 != pixels_current && pixels_2 != pixels_prev);
 	BUG_ON(pixels_current == pixels_prev);
 
+	obj = drm_gem_object_lookup(dev, file_priv, handle);
+	if (!obj)
+		return -ENOENT;
+
 	ret = mgag200_bo_reserve(pixels_1, true);
 	if (ret) {
 		WREG8(MGA_CURPOSXL, 0);
 		WREG8(MGA_CURPOSXH, 0);
-		return ret;
+		goto out_unref;
 	}
 	ret = mgag200_bo_reserve(pixels_2, true);
 	if (ret) {
 		WREG8(MGA_CURPOSXL, 0);
 		WREG8(MGA_CURPOSXH, 0);
 		mgag200_bo_unreserve(pixels_1);
-		return ret;
+		goto out_unreserve1;
 	}
 
 	if (!handle) {
@@ -106,16 +110,6 @@  int mga_crtc_cursor_set(struct drm_crtc *crtc,
 		}
 	}
 
-	mutex_lock(&dev->struct_mutex);
-	obj = drm_gem_object_lookup(dev, file_priv, handle);
-	if (!obj) {
-		mutex_unlock(&dev->struct_mutex);
-		ret = -ENOENT;
-		goto out1;
-	}
-	drm_gem_object_unreference(obj);
-	mutex_unlock(&dev->struct_mutex);
-
 	bo = gem_to_mga_bo(obj);
 	ret = mgag200_bo_reserve(bo, true);
 	if (ret) {
@@ -252,7 +246,11 @@  int mga_crtc_cursor_set(struct drm_crtc *crtc,
 	if (ret)
 		mga_hide_cursor(mdev);
 	mgag200_bo_unreserve(pixels_1);
+out_unreserve1:
 	mgag200_bo_unreserve(pixels_2);
+out_unref:
+	drm_gem_object_unreference_unlocked(obj);
+
 	return ret;
 }