[PATCH 0/8] DRM File Context Cleanups

[PATCH 0/8] DRM File Context Cleanups

[David Herrmann]

Hi

Some rather random cleanups on drm_file and whatever I found on the way. The
idea here really is to get to a point where we can allocate drm_file from a
kernel context to get in-kernel rendering to work. This would allow allocating
dumb-buffers, dma-bufs, etc. from within the kernel and get fbdev generically
working, as well as panic renderers et al.

The only critical thing is drm_file->filp, which points to the underlying
"struct file". We really don't want to fake a "struct file" for in-kernel
renderers (even though it is possible), so this series tries to get rid of all
non-legacy users of it. The only real place left is vm_mmap() in drm_bufs.c.

Tested on i915 only.

Thanks
David

David Herrmann (8):
  drm: rename DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY
  drm: remove redundant drm_file->uid
  drm: reduce GETCLIENT to a minimum
  drm: make vma-manager entries untyped
  drm: use drm_file to tag vm-bos
  drm: use priv->pid to deduce task EUID
  drm: rename drm_file.filp to drm_file.legacy_filp
  drm: provide management functions for drm_file

 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c |   3 +-
 drivers/gpu/drm/ast/ast_ttm.c           |   3 +-
 drivers/gpu/drm/bochs/bochs_mm.c        |   3 +-
 drivers/gpu/drm/cirrus/cirrus_ttm.c     |   3 +-
 drivers/gpu/drm/drm_bufs.c              |   7 +-
 drivers/gpu/drm/drm_drv.c               | 149 ++++++++++++++++++++++++++++++--
 drivers/gpu/drm/drm_fops.c              | 133 ++--------------------------
 drivers/gpu/drm/drm_gem.c               |   8 +-
 drivers/gpu/drm/drm_info.c              |   4 +-
 drivers/gpu/drm/drm_internal.h          |   4 +
 drivers/gpu/drm/drm_ioctl.c             |   5 +-
 drivers/gpu/drm/drm_vma_manager.c       |  40 ++++-----
 drivers/gpu/drm/mgag200/mgag200_ttm.c   |   3 +-
 drivers/gpu/drm/nouveau/nouveau_bo.c    |   3 +-
 drivers/gpu/drm/qxl/qxl_ttm.c           |   3 +-
 drivers/gpu/drm/radeon/radeon_ttm.c     |   3 +-
 include/drm/drmP.h                      |   7 +-
 include/drm/drm_vma_manager.h           |  18 ++--
 18 files changed, 213 insertions(+), 186 deletions(-)

-- 
2.9.2

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

[PATCH 1/8] drm: rename DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY

[David Herrmann]

The minor referred to by "DRM_MINOR_LEGACY" is called 'dev->primary' and
gets 'cardX' as name assigned. Lets reduce this magnificent number of
names for the same concept by one and rename DRM_MINOR_LEGACY to
DRM_MINOR_PRIMARY (to match the actual struct-member name).

Furthermore, this is in no way a legacy node, so lets not call it that.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_drv.c | 14 +++++++-------
 include/drm/drmP.h        |  4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index be27ed3..57ce973 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -112,7 +112,7 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
 					     unsigned int type)
 {
 	switch (type) {
-	case DRM_MINOR_LEGACY:
+	case DRM_MINOR_PRIMARY:
 		return &dev->primary;
 	case DRM_MINOR_RENDER:
 		return &dev->render;
@@ -512,7 +512,7 @@ int drm_dev_init(struct drm_device *dev,
 			goto err_minors;
 	}
 
-	ret = drm_minor_alloc(dev, DRM_MINOR_LEGACY);
+	ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
 	if (ret)
 		goto err_minors;
 
@@ -545,7 +545,7 @@ err_ctxbitmap:
 	drm_legacy_ctxbitmap_cleanup(dev);
 	drm_ht_remove(&dev->map_hash);
 err_minors:
-	drm_minor_free(dev, DRM_MINOR_LEGACY);
+	drm_minor_free(dev, DRM_MINOR_PRIMARY);
 	drm_minor_free(dev, DRM_MINOR_RENDER);
 	drm_minor_free(dev, DRM_MINOR_CONTROL);
 	drm_fs_inode_free(dev->anon_inode);
@@ -608,7 +608,7 @@ static void drm_dev_release(struct kref *ref)
 	drm_ht_remove(&dev->map_hash);
 	drm_fs_inode_free(dev->anon_inode);
 
-	drm_minor_free(dev, DRM_MINOR_LEGACY);
+	drm_minor_free(dev, DRM_MINOR_PRIMARY);
 	drm_minor_free(dev, DRM_MINOR_RENDER);
 	drm_minor_free(dev, DRM_MINOR_CONTROL);
 
@@ -684,7 +684,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	if (ret)
 		goto err_minors;
 
-	ret = drm_minor_register(dev, DRM_MINOR_LEGACY);
+	ret = drm_minor_register(dev, DRM_MINOR_PRIMARY);
 	if (ret)
 		goto err_minors;
 
@@ -701,7 +701,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
 	goto out_unlock;
 
 err_minors:
-	drm_minor_unregister(dev, DRM_MINOR_LEGACY);
+	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
 out_unlock:
@@ -741,7 +741,7 @@ void drm_dev_unregister(struct drm_device *dev)
 	list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
 		drm_legacy_rmmap(dev, r_list->map);
 
-	drm_minor_unregister(dev, DRM_MINOR_LEGACY);
+	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
 	drm_minor_unregister(dev, DRM_MINOR_RENDER);
 	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d377865..d488a72 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -642,7 +642,7 @@ struct drm_driver {
 };
 
 enum drm_minor_type {
-	DRM_MINOR_LEGACY,
+	DRM_MINOR_PRIMARY,
 	DRM_MINOR_CONTROL,
 	DRM_MINOR_RENDER,
 	DRM_MINOR_CNT,
@@ -856,7 +856,7 @@ static inline bool drm_is_control_client(const struct drm_file *file_priv)
 
 static inline bool drm_is_primary_client(const struct drm_file *file_priv)
 {
-	return file_priv->minor->type == DRM_MINOR_LEGACY;
+	return file_priv->minor->type == DRM_MINOR_PRIMARY;
 }
 
 /******************************************************************/
-- 
2.9.2

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

[PATCH 2/8] drm: remove redundant drm_file->uid

[David Herrmann]

Each DRM file-context caches the EUID of the process that opened the file.
It is used exclusively for debugging purposes in /proc/dri/ and friends.

Note, however, that "struct file" already caches the credentials of a
process at open-time. So lets just use drm_file->filp->f_cred->euid if
available. The pointer-chasing will not hurt us, since it is only about
debugging, anyway.

Check priv->filp for NULL before deref, to prepare for in-kernel contexts
(if they ever appear). We should not rely on "struct file" contexts to be
around at all times, anyway.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_fops.c  | 1 -
 drivers/gpu/drm/drm_info.c  | 4 +++-
 drivers/gpu/drm/drm_ioctl.c | 4 +++-
 include/drm/drmP.h          | 1 -
 4 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 323c238..e9d66f5 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -199,7 +199,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 
 	filp->private_data = priv;
 	priv->filp = filp;
-	priv->uid = current_euid();
 	priv->pid = get_pid(task_pid(current));
 	priv->minor = minor;
 
diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index 9ae353f..247ba2b 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -80,6 +80,7 @@ int drm_clients_info(struct seq_file *m, void *data)
 	struct drm_info_node *node = (struct drm_info_node *) m->private;
 	struct drm_device *dev = node->minor->dev;
 	struct drm_file *priv;
+	kuid_t uid;
 
 	seq_printf(m,
 		   "%20s %5s %3s master a %5s %10s\n",
@@ -98,13 +99,14 @@ int drm_clients_info(struct seq_file *m, void *data)
 
 		rcu_read_lock(); /* locks pid_task()->comm */
 		task = pid_task(priv->pid, PIDTYPE_PID);
+		uid = priv->filp ? priv->filp->f_cred->euid : GLOBAL_ROOT_UID;
 		seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
 			   task ? task->comm : "<unknown>",
 			   pid_vnr(priv->pid),
 			   priv->minor->index,
 			   drm_is_current_master(priv) ? 'y' : 'n',
 			   priv->authenticated ? 'y' : 'n',
-			   from_kuid_munged(seq_user_ns(m), priv->uid),
+			   from_kuid_munged(seq_user_ns(m), uid),
 			   priv->magic);
 		rcu_read_unlock();
 	}
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 33af4a5..49cd835 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -191,7 +191,9 @@ static int drm_getclient(struct drm_device *dev, void *data,
 		client->auth = file_priv->authenticated;
 		client->pid = pid_vnr(file_priv->pid);
 		client->uid = from_kuid_munged(current_user_ns(),
-					       file_priv->uid);
+					       file_priv->filp ?
+					       file_priv->filp->f_cred->euid :
+					       GLOBAL_ROOT_UID);
 		client->magic = 0;
 		client->iocs = 0;
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d488a72..0f69f56 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -320,7 +320,6 @@ struct drm_file {
 	unsigned is_master:1;
 
 	struct pid *pid;
-	kuid_t uid;
 	drm_magic_t magic;
 	struct list_head lhead;
 	struct drm_minor *minor;
-- 
2.9.2

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

[PATCH 3/8] drm: reduce GETCLIENT to a minimum

[David Herrmann]

The *only* known user of GETCLIENT is libva, which uses it to check
whether its own context is authenticated. It used to iterate all clients,
look for one that matches its own pid and then check its state.

The entire purpose for us to still have a GETCLIENT implementation is to
serve libva. So lets not pretend we do anything else: Make this function
return information on the caller's context only, fake the PID to the
caller's pid so they always match, and just fill in the "authenticated"
bit, nothing else.

This patch reduces the complexity of GETCLIENT to a bare minimum, avoids
any dependency on priv->uid or priv->pid (allows us to get rid of them),
and makes libva happy by always *exactly* returning the information it
wants.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_ioctl.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 49cd835..bc5c65e 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -189,11 +189,8 @@ static int drm_getclient(struct drm_device *dev, void *data,
 	 */
 	if (client->idx == 0) {
 		client->auth = file_priv->authenticated;
-		client->pid = pid_vnr(file_priv->pid);
-		client->uid = from_kuid_munged(current_user_ns(),
-					       file_priv->filp ?
-					       file_priv->filp->f_cred->euid :
-					       GLOBAL_ROOT_UID);
+		client->pid = task_pid_vnr(current);
+		client->uid = overflowuid;
 		client->magic = 0;
 		client->iocs = 0;
 
-- 
2.9.2

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

[PATCH 4/8] drm: make vma-manager entries untyped

[David Herrmann]

Right now, the vma-manager requires all entries to provide a "struct
file*" as identifier. This is confusing, since there is no inherent
connection to "struct file" in the vma-manager at all, but it is
exclusively used as tag.

Replace its usage with a simple "void *tag" and make sure callers can use
arbitrary tags.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_vma_manager.c | 40 +++++++++++++++++++--------------------
 include/drm/drm_vma_manager.h     | 18 ++++++++----------
 2 files changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
index f306c88..954248e 100644
--- a/drivers/gpu/drm/drm_vma_manager.c
+++ b/drivers/gpu/drm/drm_vma_manager.c
@@ -25,7 +25,6 @@
 #include <drm/drmP.h>
 #include <drm/drm_mm.h>
 #include <drm/drm_vma_manager.h>
-#include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/rbtree.h>
@@ -277,9 +276,9 @@ EXPORT_SYMBOL(drm_vma_offset_remove);
 /**
  * drm_vma_node_allow - Add open-file to list of allowed users
  * @node: Node to modify
- * @filp: Open file to add
+ * @tag: Tag of file to remove
  *
- * Add @filp to the list of allowed open-files for this node. If @filp is
+ * Add @tag to the list of allowed open-files for this node. If @tag is
  * already on this list, the ref-count is incremented.
  *
  * The list of allowed-users is preserved across drm_vma_offset_add() and
@@ -294,7 +293,7 @@ EXPORT_SYMBOL(drm_vma_offset_remove);
  * RETURNS:
  * 0 on success, negative error code on internal failure (out-of-mem)
  */
-int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp)
+int drm_vma_node_allow(struct drm_vma_offset_node *node, void *tag)
 {
 	struct rb_node **iter;
 	struct rb_node *parent = NULL;
@@ -315,10 +314,10 @@ int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp)
 		parent = *iter;
 		entry = rb_entry(*iter, struct drm_vma_offset_file, vm_rb);
 
-		if (filp == entry->vm_filp) {
+		if (tag == entry->vm_tag) {
 			entry->vm_count++;
 			goto unlock;
-		} else if (filp > entry->vm_filp) {
+		} else if (tag > entry->vm_tag) {
 			iter = &(*iter)->rb_right;
 		} else {
 			iter = &(*iter)->rb_left;
@@ -330,7 +329,7 @@ int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp)
 		goto unlock;
 	}
 
-	new->vm_filp = filp;
+	new->vm_tag = tag;
 	new->vm_count = 1;
 	rb_link_node(&new->vm_rb, parent, iter);
 	rb_insert_color(&new->vm_rb, &node->vm_files);
@@ -346,17 +345,17 @@ EXPORT_SYMBOL(drm_vma_node_allow);
 /**
  * drm_vma_node_revoke - Remove open-file from list of allowed users
  * @node: Node to modify
- * @filp: Open file to remove
+ * @tag: Tag of file to remove
  *
- * Decrement the ref-count of @filp in the list of allowed open-files on @node.
- * If the ref-count drops to zero, remove @filp from the list. You must call
- * this once for every drm_vma_node_allow() on @filp.
+ * Decrement the ref-count of @tag in the list of allowed open-files on @node.
+ * If the ref-count drops to zero, remove @tag from the list. You must call
+ * this once for every drm_vma_node_allow() on @tag.
  *
  * This is locked against concurrent access internally.
  *
- * If @filp is not on the list, nothing is done.
+ * If @tag is not on the list, nothing is done.
  */
-void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp)
+void drm_vma_node_revoke(struct drm_vma_offset_node *node, void *tag)
 {
 	struct drm_vma_offset_file *entry;
 	struct rb_node *iter;
@@ -366,13 +365,13 @@ void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp)
 	iter = node->vm_files.rb_node;
 	while (likely(iter)) {
 		entry = rb_entry(iter, struct drm_vma_offset_file, vm_rb);
-		if (filp == entry->vm_filp) {
+		if (tag == entry->vm_tag) {
 			if (!--entry->vm_count) {
 				rb_erase(&entry->vm_rb, &node->vm_files);
 				kfree(entry);
 			}
 			break;
-		} else if (filp > entry->vm_filp) {
+		} else if (tag > entry->vm_tag) {
 			iter = iter->rb_right;
 		} else {
 			iter = iter->rb_left;
@@ -386,9 +385,9 @@ EXPORT_SYMBOL(drm_vma_node_revoke);
 /**
  * drm_vma_node_is_allowed - Check whether an open-file is granted access
  * @node: Node to check
- * @filp: Open-file to check for
+ * @tag: Tag of file to remove
  *
- * Search the list in @node whether @filp is currently on the list of allowed
+ * Search the list in @node whether @tag is currently on the list of allowed
  * open-files (see drm_vma_node_allow()).
  *
  * This is locked against concurrent access internally.
@@ -396,8 +395,7 @@ EXPORT_SYMBOL(drm_vma_node_revoke);
  * RETURNS:
  * true iff @filp is on the list
  */
-bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
-			     struct file *filp)
+bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node, void *tag)
 {
 	struct drm_vma_offset_file *entry;
 	struct rb_node *iter;
@@ -407,9 +405,9 @@ bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
 	iter = node->vm_files.rb_node;
 	while (likely(iter)) {
 		entry = rb_entry(iter, struct drm_vma_offset_file, vm_rb);
-		if (filp == entry->vm_filp)
+		if (tag == entry->vm_tag)
 			break;
-		else if (filp > entry->vm_filp)
+		else if (tag > entry->vm_tag)
 			iter = iter->rb_right;
 		else
 			iter = iter->rb_left;
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 06ea8e07..6edc968 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -24,7 +24,6 @@
  */
 
 #include <drm/drm_mm.h>
-#include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/rbtree.h>
@@ -33,7 +32,7 @@
 
 struct drm_vma_offset_file {
 	struct rb_node vm_rb;
-	struct file *vm_filp;
+	void *vm_tag;
 	unsigned long vm_count;
 };
 
@@ -62,10 +61,9 @@ int drm_vma_offset_add(struct drm_vma_offset_manager *mgr,
 void drm_vma_offset_remove(struct drm_vma_offset_manager *mgr,
 			   struct drm_vma_offset_node *node);
 
-int drm_vma_node_allow(struct drm_vma_offset_node *node, struct file *filp);
-void drm_vma_node_revoke(struct drm_vma_offset_node *node, struct file *filp);
-bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
-			     struct file *filp);
+int drm_vma_node_allow(struct drm_vma_offset_node *node, void *tag);
+void drm_vma_node_revoke(struct drm_vma_offset_node *node, void *tag);
+bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node, void *tag);
 
 /**
  * drm_vma_offset_exact_lookup_locked() - Look up node by exact address
@@ -216,9 +214,9 @@ static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
 /**
  * drm_vma_node_verify_access() - Access verification helper for TTM
  * @node: Offset node
- * @filp: Open-file
+ * @tag: Tag of file to check
  *
- * This checks whether @filp is granted access to @node. It is the same as
+ * This checks whether @tag is granted access to @node. It is the same as
  * drm_vma_node_is_allowed() but suitable as drop-in helper for TTM
  * verify_access() callbacks.
  *
@@ -226,9 +224,9 @@ static inline void drm_vma_node_unmap(struct drm_vma_offset_node *node,
  * 0 if access is granted, -EACCES otherwise.
  */
 static inline int drm_vma_node_verify_access(struct drm_vma_offset_node *node,
-					     struct file *filp)
+					     void *tag)
 {
-	return drm_vma_node_is_allowed(node, filp) ? 0 : -EACCES;
+	return drm_vma_node_is_allowed(node, tag) ? 0 : -EACCES;
 }
 
 #endif /* __DRM_VMA_MANAGER_H__ */
-- 
2.9.2

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

[PATCH 5/8] drm: use drm_file to tag vm-bos

[David Herrmann]

Rather than using "struct file*", use "struct drm_file*" as tag VM tag for
BOs. This will pave the way for "struct drm_file*" without any "struct
file*" back-pointer.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 ++-
 drivers/gpu/drm/ast/ast_ttm.c           | 3 ++-
 drivers/gpu/drm/bochs/bochs_mm.c        | 3 ++-
 drivers/gpu/drm/cirrus/cirrus_ttm.c     | 3 ++-
 drivers/gpu/drm/drm_gem.c               | 8 ++++----
 drivers/gpu/drm/mgag200/mgag200_ttm.c   | 3 ++-
 drivers/gpu/drm/nouveau/nouveau_bo.c    | 3 ++-
 drivers/gpu/drm/qxl/qxl_ttm.c           | 3 ++-
 drivers/gpu/drm/radeon/radeon_ttm.c     | 3 ++-
 9 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index b7742e6..58099c9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -225,7 +225,8 @@ static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 
 	if (amdgpu_ttm_tt_get_usermm(bo->ttm))
 		return -EPERM;
-	return drm_vma_node_verify_access(&rbo->gem_base.vma_node, filp);
+	return drm_vma_node_verify_access(&rbo->gem_base.vma_node,
+					  filp->private_data);
 }
 
 static void amdgpu_move_null(struct ttm_buffer_object *bo,
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index b29a412..608df4c 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -150,7 +150,8 @@ static int ast_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
 	struct ast_bo *astbo = ast_bo(bo);
 
-	return drm_vma_node_verify_access(&astbo->gem.vma_node, filp);
+	return drm_vma_node_verify_access(&astbo->gem.vma_node,
+					  filp->private_data);
 }
 
 static int ast_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index 5c5638a..269cfca 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -128,7 +128,8 @@ static int bochs_bo_verify_access(struct ttm_buffer_object *bo,
 {
 	struct bochs_bo *bochsbo = bochs_bo(bo);
 
-	return drm_vma_node_verify_access(&bochsbo->gem.vma_node, filp);
+	return drm_vma_node_verify_access(&bochsbo->gem.vma_node,
+					  filp->private_data);
 }
 
 static int bochs_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index 1cc9ee6..bb2438d 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -150,7 +150,8 @@ static int cirrus_bo_verify_access(struct ttm_buffer_object *bo, struct file *fi
 {
 	struct cirrus_bo *cirrusbo = cirrus_bo(bo);
 
-	return drm_vma_node_verify_access(&cirrusbo->gem.vma_node, filp);
+	return drm_vma_node_verify_access(&cirrusbo->gem.vma_node,
+					  filp->private_data);
 }
 
 static int cirrus_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 9134ae1..465bacd 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -257,7 +257,7 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 
 	if (drm_core_check_feature(dev, DRIVER_PRIME))
 		drm_gem_remove_prime_handles(obj, file_priv);
-	drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
+	drm_vma_node_revoke(&obj->vma_node, file_priv);
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, file_priv);
@@ -372,7 +372,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 
 	handle = ret;
 
-	ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp);
+	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
 	if (ret)
 		goto err_remove;
 
@@ -386,7 +386,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 	return 0;
 
 err_revoke:
-	drm_vma_node_revoke(&obj->vma_node, file_priv->filp);
+	drm_vma_node_revoke(&obj->vma_node, file_priv);
 err_remove:
 	spin_lock(&file_priv->table_lock);
 	idr_remove(&file_priv->object_idr, handle);
@@ -991,7 +991,7 @@ int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (!obj)
 		return -EINVAL;
 
-	if (!drm_vma_node_is_allowed(node, filp)) {
+	if (!drm_vma_node_is_allowed(node, priv)) {
 		drm_gem_object_unreference_unlocked(obj);
 		return -EACCES;
 	}
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 68268e5..919b35f 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -150,7 +150,8 @@ static int mgag200_bo_verify_access(struct ttm_buffer_object *bo, struct file *f
 {
 	struct mgag200_bo *mgabo = mgag200_bo(bo);
 
-	return drm_vma_node_verify_access(&mgabo->gem.vma_node, filp);
+	return drm_vma_node_verify_access(&mgabo->gem.vma_node,
+					  filp->private_data);
 }
 
 static int mgag200_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 528bdef..4fea5da 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1315,7 +1315,8 @@ nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
 	struct nouveau_bo *nvbo = nouveau_bo(bo);
 
-	return drm_vma_node_verify_access(&nvbo->gem.vma_node, filp);
+	return drm_vma_node_verify_access(&nvbo->gem.vma_node,
+					  filp->private_data);
 }
 
 static int
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index d50c967..3f6dee3 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -210,7 +210,8 @@ static int qxl_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 {
 	struct qxl_bo *qbo = to_qxl_bo(bo);
 
-	return drm_vma_node_verify_access(&qbo->gem_base.vma_node, filp);
+	return drm_vma_node_verify_access(&qbo->gem_base.vma_node,
+					  filp->private_data);
 }
 
 static int qxl_ttm_io_mem_reserve(struct ttm_bo_device *bdev,
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index ffdad81..30a1994 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -237,7 +237,8 @@ static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp)
 
 	if (radeon_ttm_tt_has_userptr(bo->ttm))
 		return -EPERM;
-	return drm_vma_node_verify_access(&rbo->gem_base.vma_node, filp);
+	return drm_vma_node_verify_access(&rbo->gem_base.vma_node,
+					  filp->private_data);
 }
 
 static void radeon_move_null(struct ttm_buffer_object *bo,
-- 
2.9.2

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

[PATCH 6/8] drm: use priv->pid to deduce task EUID

[David Herrmann]

Rather than accessing priv->filp->f_cred, use priv->pid->task->creds. We
want to get rid of "priv->filp", so lets avoid it if possible.

Since we already are in an rcu-read-side, we can use __task_cred() rather
than task_cred_xxx().

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_info.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
index 247ba2b..1df2d33 100644
--- a/drivers/gpu/drm/drm_info.c
+++ b/drivers/gpu/drm/drm_info.c
@@ -99,7 +99,7 @@ int drm_clients_info(struct seq_file *m, void *data)
 
 		rcu_read_lock(); /* locks pid_task()->comm */
 		task = pid_task(priv->pid, PIDTYPE_PID);
-		uid = priv->filp ? priv->filp->f_cred->euid : GLOBAL_ROOT_UID;
+		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;
 		seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
 			   task ? task->comm : "<unknown>",
 			   pid_vnr(priv->pid),
-- 
2.9.2

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

[PATCH 7/8] drm: rename drm_file.filp to drm_file.legacy_filp

[David Herrmann]

We don't want anyone but legacy DRM1 code to use drm_file.filp. Especially
for in-kernel contexts, this might be set to NULL, so lets make sure
no-one accesses it, ever.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_bufs.c | 7 ++++---
 drivers/gpu/drm/drm_fops.c | 2 +-
 include/drm/drmP.h         | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c
index c3a12cd..d2803de 100644
--- a/drivers/gpu/drm/drm_bufs.c
+++ b/drivers/gpu/drm/drm_bufs.c
@@ -1456,7 +1456,7 @@ int drm_legacy_mapbufs(struct drm_device *dev, void *data,
 	if (!drm_core_check_feature(dev, DRIVER_HAVE_DMA))
 		return -EINVAL;
 
-	if (!dma)
+	if (!dma || !file_priv->legacy_filp)
 		return -EINVAL;
 
 	spin_lock(&dev->buf_lock);
@@ -1478,12 +1478,13 @@ int drm_legacy_mapbufs(struct drm_device *dev, void *data,
 				retcode = -EINVAL;
 				goto done;
 			}
-			virtual = vm_mmap(file_priv->filp, 0, map->size,
+			virtual = vm_mmap(file_priv->legacy_filp, 0, map->size,
 					  PROT_READ | PROT_WRITE,
 					  MAP_SHARED,
 					  token);
 		} else {
-			virtual = vm_mmap(file_priv->filp, 0, dma->byte_count,
+			virtual = vm_mmap(file_priv->legacy_filp, 0,
+					  dma->byte_count,
 					  PROT_READ | PROT_WRITE,
 					  MAP_SHARED, 0);
 		}
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index e9d66f5..69ef23c 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -198,7 +198,7 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 		return -ENOMEM;
 
 	filp->private_data = priv;
-	priv->filp = filp;
+	priv->legacy_filp = filp;
 	priv->pid = get_pid(task_pid(current));
 	priv->minor = minor;
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0f69f56..2197ab1 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -330,7 +330,7 @@ struct drm_file {
 	/** Lock for synchronization of access to object_idr. */
 	spinlock_t table_lock;
 
-	struct file *filp;
+	struct file *legacy_filp; /* might be NULL! */
 	void *driver_priv;
 
 	struct drm_master *master; /* master this node is currently associated with
-- 
2.9.2

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

[PATCH 8/8] drm: provide management functions for drm_file

[David Herrmann]

Rather than doing drm_file allocation/destruction right in the fops, lets
provide separate helpers. This decouples drm_file management from the
still-mandatory drm-fops. It prepares for use of drm_file without the
fops, both by possible separate fops implementations and APIs (not that I
am aware of any such plans), and more importantly from in-kernel use where
no real file is available.

Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/gpu/drm/drm_drv.c      | 135 +++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_fops.c     | 132 +++-------------------------------------
 drivers/gpu/drm/drm_internal.h |   4 ++
 3 files changed, 147 insertions(+), 124 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 57ce973..9ab0016 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -95,6 +95,141 @@ void drm_ut_debug_printk(const char *function_name, const char *format, ...)
 }
 EXPORT_SYMBOL(drm_ut_debug_printk);
 
+/**
+ * drm_file_alloc - allocate file context
+ * @minor: minor to allocate on
+ *
+ * This allocates a new DRM file context. It is not linked into any context and
+ * can be used by the caller freely. Note that the context keeps a pointer to
+ * @minor, so it must be freed before @minor is.
+ *
+ * The legacy paths might require the drm_global_mutex to be held.
+ *
+ * RETURNS:
+ * Pointer to newly allocated context, ERR_PTR on failure.
+ */
+struct drm_file *drm_file_alloc(struct drm_minor *minor)
+{
+	struct drm_device *dev = minor->dev;
+	struct drm_file *file;
+	int ret;
+
+	file = kzalloc(sizeof(*file), GFP_KERNEL);
+	if (!file)
+		return ERR_PTR(-ENOMEM);
+
+	file->pid = get_pid(task_pid(current));
+	file->minor = minor;
+	file->authenticated = capable(CAP_SYS_ADMIN); /* legacy compat */
+	INIT_LIST_HEAD(&file->lhead);
+	INIT_LIST_HEAD(&file->fbs);
+	mutex_init(&file->fbs_lock);
+	INIT_LIST_HEAD(&file->blobs);
+	INIT_LIST_HEAD(&file->pending_event_list);
+	INIT_LIST_HEAD(&file->event_list);
+	init_waitqueue_head(&file->event_wait);
+	file->event_space = 4096; /* set aside 4k for event buffer */
+	mutex_init(&file->event_read_lock);
+
+	if (drm_core_check_feature(dev, DRIVER_GEM))
+		drm_gem_open(dev, file);
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_prime_init_file_private(&file->prime);
+
+	if (dev->driver->open) {
+		ret = dev->driver->open(dev, file);
+		if (ret < 0)
+			goto out_prime_destroy;
+	}
+
+	if (drm_is_primary_client(file)) {
+		ret = drm_master_open(file);
+		if (ret)
+			goto out_close;
+	}
+
+	return file;
+
+out_close:
+	if (dev->driver->postclose)
+		dev->driver->postclose(dev, file);
+out_prime_destroy:
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_prime_destroy_file_private(&file->prime);
+	if (drm_core_check_feature(dev, DRIVER_GEM))
+		drm_gem_release(dev, file);
+	put_pid(file->pid);
+	kfree(file);
+	return ERR_PTR(ret);
+}
+
+/**
+ * drm_file_free - free file context
+ * @file: context to free, or NULL
+ *
+ * This destroys and deallocates a DRM file context previously allocated via
+ * drm_file_alloc(). The caller must make sure to unlink it from any contexts
+ * before calling this.
+ *
+ * The legacy paths might require the drm_global_mutex to be held.
+ *
+ * If NULL is passed, this is a no-op.
+ *
+ * RETURNS:
+ * 0 on success, or error code on failure.
+ */
+void drm_file_free(struct drm_file *file)
+{
+	struct drm_pending_event *e;
+	struct drm_device *dev;
+
+	if (!file)
+		return;
+
+	dev = file->minor->dev;
+
+	if (dev->driver->preclose)
+		dev->driver->preclose(dev, file);
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		drm_legacy_lock_release(dev, file->legacy_filp);
+	if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
+		drm_legacy_reclaim_buffers(dev, file);
+
+	spin_lock_irq(&dev->event_lock);
+	while ((e = list_first_entry_or_null(&file->pending_event_list,
+					     struct drm_pending_event,
+					     pending_link))) {
+		list_del(&e->pending_link);
+		e->file_priv = NULL;
+	}
+	while ((e = list_first_entry_or_null(&file->event_list,
+					     struct drm_pending_event, link))) {
+		list_del(&e->link);
+		kfree(e);
+	}
+	spin_unlock_irq(&dev->event_lock);
+
+	drm_legacy_ctxbitmap_flush(dev, file);
+
+	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+		drm_fb_release(file);
+		drm_property_destroy_user_blobs(dev, file);
+	}
+
+	if (drm_core_check_feature(dev, DRIVER_GEM))
+		drm_gem_release(dev, file);
+	if (drm_is_primary_client(file))
+		drm_master_release(file);
+	if (dev->driver->postclose)
+		dev->driver->postclose(dev, file);
+	if (drm_core_check_feature(dev, DRIVER_PRIME))
+		drm_prime_destroy_file_private(&file->prime);
+
+	WARN_ON(!list_empty(&file->event_list));
+	put_pid(file->pid);
+	kfree(file);
+}
+
 /*
  * DRM Minors
  * A DRM device can provide several char-dev interfaces on the DRM-Major. Each
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index 69ef23c..359c636 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -182,7 +182,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 {
 	struct drm_device *dev = minor->dev;
 	struct drm_file *priv;
-	int ret;
 
 	if (filp->f_flags & O_EXCL)
 		return -EBUSY;	/* No exclusive opens */
@@ -193,47 +192,12 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 
 	DRM_DEBUG("pid = %d, minor = %d\n", task_pid_nr(current), minor->index);
 
-	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
+	priv = drm_file_alloc(minor);
+	if (IS_ERR(priv))
+		return PTR_ERR(priv);
 
-	filp->private_data = priv;
 	priv->legacy_filp = filp;
-	priv->pid = get_pid(task_pid(current));
-	priv->minor = minor;
-
-	/* for compatibility root is always authenticated */
-	priv->authenticated = capable(CAP_SYS_ADMIN);
-	priv->lock_count = 0;
-
-	INIT_LIST_HEAD(&priv->lhead);
-	INIT_LIST_HEAD(&priv->fbs);
-	mutex_init(&priv->fbs_lock);
-	INIT_LIST_HEAD(&priv->blobs);
-	INIT_LIST_HEAD(&priv->pending_event_list);
-	INIT_LIST_HEAD(&priv->event_list);
-	init_waitqueue_head(&priv->event_wait);
-	priv->event_space = 4096; /* set aside 4k for event buffer */
-
-	mutex_init(&priv->event_read_lock);
-
-	if (drm_core_check_feature(dev, DRIVER_GEM))
-		drm_gem_open(dev, priv);
-
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_prime_init_file_private(&priv->prime);
-
-	if (dev->driver->open) {
-		ret = dev->driver->open(dev, priv);
-		if (ret < 0)
-			goto out_prime_destroy;
-	}
-
-	if (drm_is_primary_client(priv)) {
-		ret = drm_master_open(priv);
-		if (ret)
-			goto out_close;
-	}
+	filp->private_data = priv;
 
 	mutex_lock(&dev->filelist_mutex);
 	list_add(&priv->lhead, &dev->filelist);
@@ -260,43 +224,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
 #endif
 
 	return 0;
-
-out_close:
-	if (dev->driver->postclose)
-		dev->driver->postclose(dev, priv);
-out_prime_destroy:
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_prime_destroy_file_private(&priv->prime);
-	if (drm_core_check_feature(dev, DRIVER_GEM))
-		drm_gem_release(dev, priv);
-	put_pid(priv->pid);
-	kfree(priv);
-	filp->private_data = NULL;
-	return ret;
-}
-
-static void drm_events_release(struct drm_file *file_priv)
-{
-	struct drm_device *dev = file_priv->minor->dev;
-	struct drm_pending_event *e, *et;
-	unsigned long flags;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	/* Unlink pending events */
-	list_for_each_entry_safe(e, et, &file_priv->pending_event_list,
-				 pending_link) {
-		list_del(&e->pending_link);
-		e->file_priv = NULL;
-	}
-
-	/* Remove unconsumed events */
-	list_for_each_entry_safe(e, et, &file_priv->event_list, link) {
-		list_del(&e->link);
-		kfree(e);
-	}
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
 /*
@@ -370,59 +297,16 @@ int drm_release(struct inode *inode, struct file *filp)
 
 	mutex_lock(&drm_global_mutex);
 
-	DRM_DEBUG("open_count = %d\n", dev->open_count);
-
-	mutex_lock(&dev->filelist_mutex);
-	list_del(&file_priv->lhead);
-	mutex_unlock(&dev->filelist_mutex);
-
-	if (dev->driver->preclose)
-		dev->driver->preclose(dev, file_priv);
-
-	/* ========================================================
-	 * Begin inline drm_release
-	 */
-
 	DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n",
 		  task_pid_nr(current),
 		  (long)old_encode_dev(file_priv->minor->kdev->devt),
 		  dev->open_count);
 
-	if (!drm_core_check_feature(dev, DRIVER_MODESET))
-		drm_legacy_lock_release(dev, filp);
-
-	if (drm_core_check_feature(dev, DRIVER_HAVE_DMA))
-		drm_legacy_reclaim_buffers(dev, file_priv);
-
-	drm_events_release(file_priv);
-
-	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		drm_fb_release(file_priv);
-		drm_property_destroy_user_blobs(dev, file_priv);
-	}
-
-	if (drm_core_check_feature(dev, DRIVER_GEM))
-		drm_gem_release(dev, file_priv);
-
-	drm_legacy_ctxbitmap_flush(dev, file_priv);
-
-	if (drm_is_primary_client(file_priv))
-		drm_master_release(file_priv);
-
-	if (dev->driver->postclose)
-		dev->driver->postclose(dev, file_priv);
-
-	if (drm_core_check_feature(dev, DRIVER_PRIME))
-		drm_prime_destroy_file_private(&file_priv->prime);
-
-	WARN_ON(!list_empty(&file_priv->event_list));
-
-	put_pid(file_priv->pid);
-	kfree(file_priv);
+	mutex_lock(&dev->filelist_mutex);
+	list_del(&file_priv->lhead);
+	mutex_unlock(&dev->filelist_mutex);
 
-	/* ========================================================
-	 * End inline drm_release
-	 */
+	drm_file_free(file_priv);
 
 	if (!--dev->open_count) {
 		drm_lastclose(dev);
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index b86dc9b..9b66cc4 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -28,6 +28,10 @@ extern unsigned int drm_timestamp_monotonic;
 extern struct mutex drm_global_mutex;
 void drm_lastclose(struct drm_device *dev);
 
+/* drm_drv.c */
+struct drm_file *drm_file_alloc(struct drm_minor *minor);
+void drm_file_free(struct drm_file *file);
+
 /* drm_pci.c */
 int drm_irq_by_busid(struct drm_device *dev, void *data,
 		     struct drm_file *file_priv);
-- 
2.9.2

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

Re: [PATCH 4/8] drm: make vma-manager entries untyped

[Chris Wilson]

On Wed, Aug 03, 2016 at 08:04:28PM +0200, David Herrmann wrote:
> Right now, the vma-manager requires all entries to provide a "struct
> file*" as identifier. This is confusing, since there is no inherent
> connection to "struct file" in the vma-manager at all, but it is
> exclusively used as tag.
> 
> Replace its usage with a simple "void *tag" and make sure callers can use
> arbitrary tags.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---

>  struct drm_vma_offset_file {

Now a misnomer.

Lgtm,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/8] drm: remove redundant drm_file->uid

[Chris Wilson]

On Wed, Aug 03, 2016 at 08:04:26PM +0200, David Herrmann wrote:
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 323c238..e9d66f5 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -199,7 +199,6 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)
>  
>  	filp->private_data = priv;
>  	priv->filp = filp;
> -	priv->uid = current_euid();
>  	priv->pid = get_pid(task_pid(current));
>  	priv->minor = minor;
>  
> diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
> index 9ae353f..247ba2b 100644
> --- a/drivers/gpu/drm/drm_info.c
> +++ b/drivers/gpu/drm/drm_info.c
> @@ -98,13 +99,14 @@ int drm_clients_info(struct seq_file *m, void *data)
>  
>  		rcu_read_lock(); /* locks pid_task()->comm */
>  		task = pid_task(priv->pid, PIDTYPE_PID);
> +		uid = priv->filp ? priv->filp->f_cred->euid : GLOBAL_ROOT_UID;

uid = task_euid(task);

>  		seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
>  			   task ? task->comm : "<unknown>",
>  			   pid_vnr(priv->pid),
>  			   priv->minor->index,
>  			   drm_is_current_master(priv) ? 'y' : 'n',
>  			   priv->authenticated ? 'y' : 'n',
> -			   from_kuid_munged(seq_user_ns(m), priv->uid),
> +			   from_kuid_munged(seq_user_ns(m), uid),
			   from_kuid_munged(seq_user_ns(m), task_euid(task)),

Just fits.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 6/8] drm: use priv->pid to deduce task EUID

[Chris Wilson]

On Wed, Aug 03, 2016 at 08:04:30PM +0200, David Herrmann wrote:
> Rather than accessing priv->filp->f_cred, use priv->pid->task->creds. We
> want to get rid of "priv->filp", so lets avoid it if possible.
> 
> Since we already are in an rcu-read-side, we can use __task_cred() rather
> than task_cred_xxx().
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  drivers/gpu/drm/drm_info.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c
> index 247ba2b..1df2d33 100644
> --- a/drivers/gpu/drm/drm_info.c
> +++ b/drivers/gpu/drm/drm_info.c
> @@ -99,7 +99,7 @@ int drm_clients_info(struct seq_file *m, void *data)
>  
>  		rcu_read_lock(); /* locks pid_task()->comm */
>  		task = pid_task(priv->pid, PIDTYPE_PID);
> -		uid = priv->filp ? priv->filp->f_cred->euid : GLOBAL_ROOT_UID;
> +		uid = task ? __task_cred(task)->euid : GLOBAL_ROOT_UID;

Squash this into patch 2, so the poor reader doesn't have to learn about
f_cred.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/8] drm: remove redundant drm_file->uid

[Chris Wilson]

On Wed, Aug 03, 2016 at 08:04:26PM +0200, David Herrmann wrote:
> @@ -98,13 +99,14 @@ int drm_clients_info(struct seq_file *m, void *data)
>  
>  		rcu_read_lock(); /* locks pid_task()->comm */
>  		task = pid_task(priv->pid, PIDTYPE_PID);
> +		uid = priv->filp ? priv->filp->f_cred->euid : GLOBAL_ROOT_UID;
>  		seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
>  			   task ? task->comm : "<unknown>",
>  			   pid_vnr(priv->pid),
>  			   priv->minor->index,
>  			   drm_is_current_master(priv) ? 'y' : 'n',
>  			   priv->authenticated ? 'y' : 'n',
> -			   from_kuid_munged(seq_user_ns(m), priv->uid),
> +			   from_kuid_munged(seq_user_ns(m), uid),
>  			   priv->magic);
>  		rcu_read_unlock();
>  	}
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 33af4a5..49cd835 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -191,7 +191,9 @@ static int drm_getclient(struct drm_device *dev, void *data,
>  		client->auth = file_priv->authenticated;
>  		client->pid = pid_vnr(file_priv->pid);
>  		client->uid = from_kuid_munged(current_user_ns(),
> -					       file_priv->uid);
> +					       file_priv->filp ?
> +					       file_priv->filp->f_cred->euid :
> +					       GLOBAL_ROOT_UID);

Why can't we use task_euid(pid_task(file_priv->pid)) here as well?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/8] drm: use drm_file to tag vm-bos

[Chris Wilson]

On Wed, Aug 03, 2016 at 08:04:29PM +0200, David Herrmann wrote:
> Rather than using "struct file*", use "struct drm_file*" as tag VM tag for
> BOs. This will pave the way for "struct drm_file*" without any "struct
> file*" back-pointer.
> 
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Ok, the danger of untyped is having to check each and trying to spot any
missed conversions.

Couldn't find a mistake or omission,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] drm: rename DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY

[Frank Binns]

On 03/08/16 19:04, David Herrmann wrote:
> The minor referred to by "DRM_MINOR_LEGACY" is called 'dev->primary' and
> gets 'cardX' as name assigned. Lets reduce this magnificent number of
> names for the same concept by one and rename DRM_MINOR_LEGACY to
> DRM_MINOR_PRIMARY (to match the actual struct-member name).
>
> Furthermore, this is in no way a legacy node, so lets not call it that.
>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>

Reviewed-by: Frank Binns <frank.binns@imgtec.com>

> ---
>   drivers/gpu/drm/drm_drv.c | 14 +++++++-------
>   include/drm/drmP.h        |  4 ++--
>   2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index be27ed3..57ce973 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -112,7 +112,7 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
>   					     unsigned int type)
>   {
>   	switch (type) {
> -	case DRM_MINOR_LEGACY:
> +	case DRM_MINOR_PRIMARY:
>   		return &dev->primary;
>   	case DRM_MINOR_RENDER:
>   		return &dev->render;
> @@ -512,7 +512,7 @@ int drm_dev_init(struct drm_device *dev,
>   			goto err_minors;
>   	}
>   
> -	ret = drm_minor_alloc(dev, DRM_MINOR_LEGACY);
> +	ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
>   	if (ret)
>   		goto err_minors;
>   
> @@ -545,7 +545,7 @@ err_ctxbitmap:
>   	drm_legacy_ctxbitmap_cleanup(dev);
>   	drm_ht_remove(&dev->map_hash);
>   err_minors:
> -	drm_minor_free(dev, DRM_MINOR_LEGACY);
> +	drm_minor_free(dev, DRM_MINOR_PRIMARY);
>   	drm_minor_free(dev, DRM_MINOR_RENDER);
>   	drm_minor_free(dev, DRM_MINOR_CONTROL);
>   	drm_fs_inode_free(dev->anon_inode);
> @@ -608,7 +608,7 @@ static void drm_dev_release(struct kref *ref)
>   	drm_ht_remove(&dev->map_hash);
>   	drm_fs_inode_free(dev->anon_inode);
>   
> -	drm_minor_free(dev, DRM_MINOR_LEGACY);
> +	drm_minor_free(dev, DRM_MINOR_PRIMARY);
>   	drm_minor_free(dev, DRM_MINOR_RENDER);
>   	drm_minor_free(dev, DRM_MINOR_CONTROL);
>   
> @@ -684,7 +684,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>   	if (ret)
>   		goto err_minors;
>   
> -	ret = drm_minor_register(dev, DRM_MINOR_LEGACY);
> +	ret = drm_minor_register(dev, DRM_MINOR_PRIMARY);
>   	if (ret)
>   		goto err_minors;
>   
> @@ -701,7 +701,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>   	goto out_unlock;
>   
>   err_minors:
> -	drm_minor_unregister(dev, DRM_MINOR_LEGACY);
> +	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
>   	drm_minor_unregister(dev, DRM_MINOR_RENDER);
>   	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
>   out_unlock:
> @@ -741,7 +741,7 @@ void drm_dev_unregister(struct drm_device *dev)
>   	list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
>   		drm_legacy_rmmap(dev, r_list->map);
>   
> -	drm_minor_unregister(dev, DRM_MINOR_LEGACY);
> +	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
>   	drm_minor_unregister(dev, DRM_MINOR_RENDER);
>   	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
>   }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d377865..d488a72 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -642,7 +642,7 @@ struct drm_driver {
>   };
>   
>   enum drm_minor_type {
> -	DRM_MINOR_LEGACY,
> +	DRM_MINOR_PRIMARY,
>   	DRM_MINOR_CONTROL,
>   	DRM_MINOR_RENDER,
>   	DRM_MINOR_CNT,
> @@ -856,7 +856,7 @@ static inline bool drm_is_control_client(const struct drm_file *file_priv)
>   
>   static inline bool drm_is_primary_client(const struct drm_file *file_priv)
>   {
> -	return file_priv->minor->type == DRM_MINOR_LEGACY;
> +	return file_priv->minor->type == DRM_MINOR_PRIMARY;
>   }
>   
>   /******************************************************************/

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

Re: [PATCH 5/8] drm: use drm_file to tag vm-bos

[Daniel Vetter]

On Wed, Aug 03, 2016 at 08:08:19PM +0100, Chris Wilson wrote:
> On Wed, Aug 03, 2016 at 08:04:29PM +0200, David Herrmann wrote:
> > Rather than using "struct file*", use "struct drm_file*" as tag VM tag for
> > BOs. This will pave the way for "struct drm_file*" without any "struct
> > file*" back-pointer.
> > 
> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> 
> Ok, the danger of untyped is having to check each and trying to spot any
> missed conversions.

Hm yeah ... any reason we can't just switch to struct drm_file instead? At
least I can't come up with any other use case where we might supply
something else, and it's always nice to enlist the compiler for a little
bit of help.
-Daniel

> 
> Couldn't find a mistake or omission,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/8] drm: rename DRM_MINOR_LEGACY to DRM_MINOR_PRIMARY

[Daniel Vetter]

On Thu, Aug 04, 2016 at 08:45:25AM +0100, Frank Binns wrote:
> On 03/08/16 19:04, David Herrmann wrote:
> > The minor referred to by "DRM_MINOR_LEGACY" is called 'dev->primary' and
> > gets 'cardX' as name assigned. Lets reduce this magnificent number of
> > names for the same concept by one and rename DRM_MINOR_LEGACY to
> > DRM_MINOR_PRIMARY (to match the actual struct-member name).
> > 
> > Furthermore, this is in no way a legacy node, so lets not call it that.
> > 
> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> 
> Reviewed-by: Frank Binns <frank.binns@imgtec.com>

Queued up for -misc, will push out once -rc1 hits.
-Daniel

> 
> > ---
> >   drivers/gpu/drm/drm_drv.c | 14 +++++++-------
> >   include/drm/drmP.h        |  4 ++--
> >   2 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index be27ed3..57ce973 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -112,7 +112,7 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
> >   					     unsigned int type)
> >   {
> >   	switch (type) {
> > -	case DRM_MINOR_LEGACY:
> > +	case DRM_MINOR_PRIMARY:
> >   		return &dev->primary;
> >   	case DRM_MINOR_RENDER:
> >   		return &dev->render;
> > @@ -512,7 +512,7 @@ int drm_dev_init(struct drm_device *dev,
> >   			goto err_minors;
> >   	}
> > -	ret = drm_minor_alloc(dev, DRM_MINOR_LEGACY);
> > +	ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> >   	if (ret)
> >   		goto err_minors;
> > @@ -545,7 +545,7 @@ err_ctxbitmap:
> >   	drm_legacy_ctxbitmap_cleanup(dev);
> >   	drm_ht_remove(&dev->map_hash);
> >   err_minors:
> > -	drm_minor_free(dev, DRM_MINOR_LEGACY);
> > +	drm_minor_free(dev, DRM_MINOR_PRIMARY);
> >   	drm_minor_free(dev, DRM_MINOR_RENDER);
> >   	drm_minor_free(dev, DRM_MINOR_CONTROL);
> >   	drm_fs_inode_free(dev->anon_inode);
> > @@ -608,7 +608,7 @@ static void drm_dev_release(struct kref *ref)
> >   	drm_ht_remove(&dev->map_hash);
> >   	drm_fs_inode_free(dev->anon_inode);
> > -	drm_minor_free(dev, DRM_MINOR_LEGACY);
> > +	drm_minor_free(dev, DRM_MINOR_PRIMARY);
> >   	drm_minor_free(dev, DRM_MINOR_RENDER);
> >   	drm_minor_free(dev, DRM_MINOR_CONTROL);
> > @@ -684,7 +684,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >   	if (ret)
> >   		goto err_minors;
> > -	ret = drm_minor_register(dev, DRM_MINOR_LEGACY);
> > +	ret = drm_minor_register(dev, DRM_MINOR_PRIMARY);
> >   	if (ret)
> >   		goto err_minors;
> > @@ -701,7 +701,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >   	goto out_unlock;
> >   err_minors:
> > -	drm_minor_unregister(dev, DRM_MINOR_LEGACY);
> > +	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> >   	drm_minor_unregister(dev, DRM_MINOR_RENDER);
> >   	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
> >   out_unlock:
> > @@ -741,7 +741,7 @@ void drm_dev_unregister(struct drm_device *dev)
> >   	list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head)
> >   		drm_legacy_rmmap(dev, r_list->map);
> > -	drm_minor_unregister(dev, DRM_MINOR_LEGACY);
> > +	drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> >   	drm_minor_unregister(dev, DRM_MINOR_RENDER);
> >   	drm_minor_unregister(dev, DRM_MINOR_CONTROL);
> >   }
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index d377865..d488a72 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -642,7 +642,7 @@ struct drm_driver {
> >   };
> >   enum drm_minor_type {
> > -	DRM_MINOR_LEGACY,
> > +	DRM_MINOR_PRIMARY,
> >   	DRM_MINOR_CONTROL,
> >   	DRM_MINOR_RENDER,
> >   	DRM_MINOR_CNT,
> > @@ -856,7 +856,7 @@ static inline bool drm_is_control_client(const struct drm_file *file_priv)
> >   static inline bool drm_is_primary_client(const struct drm_file *file_priv)
> >   {
> > -	return file_priv->minor->type == DRM_MINOR_LEGACY;
> > +	return file_priv->minor->type == DRM_MINOR_PRIMARY;
> >   }
> >   /******************************************************************/
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/8] drm: remove redundant drm_file->uid

[David Herrmann]

Hey

On Wed, Aug 3, 2016 at 9:01 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Aug 03, 2016 at 08:04:26PM +0200, David Herrmann wrote:
>> @@ -98,13 +99,14 @@ int drm_clients_info(struct seq_file *m, void *data)
>>
>>               rcu_read_lock(); /* locks pid_task()->comm */
>>               task = pid_task(priv->pid, PIDTYPE_PID);
>> +             uid = priv->filp ? priv->filp->f_cred->euid : GLOBAL_ROOT_UID;
>>               seq_printf(m, "%20s %5d %3d   %c    %c %5d %10u\n",
>>                          task ? task->comm : "<unknown>",
>>                          pid_vnr(priv->pid),
>>                          priv->minor->index,
>>                          drm_is_current_master(priv) ? 'y' : 'n',
>>                          priv->authenticated ? 'y' : 'n',
>> -                        from_kuid_munged(seq_user_ns(m), priv->uid),
>> +                        from_kuid_munged(seq_user_ns(m), uid),
>>                          priv->magic);
>>               rcu_read_unlock();
>>       }
>> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 33af4a5..49cd835 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -191,7 +191,9 @@ static int drm_getclient(struct drm_device *dev, void *data,
>>               client->auth = file_priv->authenticated;
>>               client->pid = pid_vnr(file_priv->pid);
>>               client->uid = from_kuid_munged(current_user_ns(),
>> -                                            file_priv->uid);
>> +                                            file_priv->filp ?
>> +                                            file_priv->filp->f_cred->euid :
>> +                                            GLOBAL_ROOT_UID);
>
> Why can't we use task_euid(pid_task(file_priv->pid)) here as well?

task_euid() changes semantics. With this patch I just tried to get rid
of "filp" usage, but keep semantics, so the ABI does not suddently
change.

Note that both calls are actually changed in follow-ups. "uid" is just
cleared to "overflowuid", since no-one ever looks at that field. And
"pid" is set to "pid_vnr(current)". However, I explicitly split those
patches to make sure it is easier to bisect.

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/8] drm: use drm_file to tag vm-bos

[David Herrmann]

Hey

On Thu, Aug 4, 2016 at 10:16 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Aug 03, 2016 at 08:08:19PM +0100, Chris Wilson wrote:
>> On Wed, Aug 03, 2016 at 08:04:29PM +0200, David Herrmann wrote:
>> > Rather than using "struct file*", use "struct drm_file*" as tag VM tag for
>> > BOs. This will pave the way for "struct drm_file*" without any "struct
>> > file*" back-pointer.
>> >
>> > Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>>
>> Ok, the danger of untyped is having to check each and trying to spot any
>> missed conversions.
>
> Hm yeah ... any reason we can't just switch to struct drm_file instead? At
> least I can't come up with any other use case where we might supply
> something else, and it's always nice to enlist the compiler for a little
> bit of help.

Fair enough. Will change this. Will send v2 once I'm back from vacation.

Thanks
David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/8] drm: make vma-manager entries untyped

[Emil Velikov]

On 3 August 2016 at 19:04, David Herrmann <dh.herrmann@gmail.com> wrote:

> @@ -277,9 +276,9 @@ EXPORT_SYMBOL(drm_vma_offset_remove);
>  /**
>   * drm_vma_node_allow - Add open-file to list of allowed users
>   * @node: Node to modify
> - * @filp: Open file to add
> + * @tag: Tag of file to remove
s/to remove/to add/

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