Re: Breakage in "track dev_mapping in more robust and flexible way"

[Ilija Hadzic <ihadzic@research.bell-labs.com>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 599 bytes --]


Can you give the attached patch a whirl and let me know if it fixes the 
problem?

As I indicated in my previous note, vmwgfx should be the only affected 
driver because it looks at dev_mapping in the open hook (others do it when 
they create an object, so they are not affected).

I'll probably revise it (and I'll have some general questions about 
drm_open syscall) before officially send the patch, but I wanted to get 
something quickly to you to check if it fixes your problem. I hope that 
your vmwgfx test environment is such that you can reproduce the original
problem.

thanks,

-- Ilija

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2618 bytes --]

From 18a489e7415f495c7ba48cc61733d6c7d8f3fd68 Mon Sep 17 00:00:00 2001
From: Ilija Hadzic <ihadzic@research.bell-labs.com>
Date: Thu, 25 Oct 2012 15:28:05 -0400
Subject: [PATCH] drm: set dev_mapping before calling drm_open_helper

Some drivers (specifically vmwgfx) look at dev_mapping
in their open hook, so we have to set dev->dev_mapping
earlier in the process.

Signed-off-by: Ilija Hadzic <ihadzic@research.bell-labs.com>
---
 drivers/gpu/drm/drm_fops.c |   43 +++++++++++++++++++++++++++++--------------
 1 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 7ef1b67..50b7b47 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -121,6 +121,8 @@ int drm_open(struct inode *inode, struct file *filp)
 	int minor_id = iminor(inode);
 	struct drm_minor *minor;
 	int retcode = 0;
+	int need_setup = 0;
+	struct address_space *old_mapping;
 
 	minor = idr_find(&drm_minors_idr, minor_id);
 	if (!minor)
@@ -132,23 +134,36 @@ int drm_open(struct inode *inode, struct file *filp)
 	if (drm_device_is_unplugged(dev))
 		return -ENODEV;
 
+	if (!dev->open_count++)
+		need_setup = 1;
+	mutex_lock(&dev->struct_mutex);
+	old_mapping = dev->dev_mapping;
+	if (old_mapping == NULL)
+		dev->dev_mapping = &inode->i_data;
+	mutex_unlock(&dev->struct_mutex);
+
 	retcode = drm_open_helper(inode, filp, dev);
-	if (!retcode) {
-		atomic_inc(&dev->counts[_DRM_STAT_OPENS]);
-		if (!dev->open_count++)
-			retcode = drm_setup(dev);
-	}
-	if (!retcode) {
-		mutex_lock(&dev->struct_mutex);
-		if (dev->dev_mapping == NULL)
-			dev->dev_mapping = &inode->i_data;
-		/* ihold ensures nobody can remove inode with our i_data */
-		ihold(container_of(dev->dev_mapping, struct inode, i_data));
-		inode->i_mapping = dev->dev_mapping;
-		filp->f_mapping = dev->dev_mapping;
-		mutex_unlock(&dev->struct_mutex);
+	if (retcode)
+		goto err_undo;
+	atomic_inc(&dev->counts[_DRM_STAT_OPENS]);
+	if (need_setup) {
+		retcode = drm_setup(dev);
+		if (retcode)
+			goto err_undo;
 	}
+	/* ihold ensures nobody can remove inode with our i_data */
+	mutex_lock(&dev->struct_mutex);
+	ihold(container_of(dev->dev_mapping, struct inode, i_data));
+	inode->i_mapping = dev->dev_mapping;
+	filp->f_mapping = dev->dev_mapping;
+	mutex_unlock(&dev->struct_mutex);
+	return 0;
 
+err_undo:
+	dev->open_count--;
+	mutex_lock(&dev->struct_mutex);
+	dev->dev_mapping = old_mapping;
+	mutex_unlock(&dev->struct_mutex);
 	return retcode;
 }
 EXPORT_SYMBOL(drm_open);
-- 
1.7.4.1


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

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