* [PATCH 1/6] drm: remove redundant drm_file->uid
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
@ 2016-09-01 12:48 ` David Herrmann
2016-09-01 12:48 ` [PATCH 2/6] drm: use drm_file to tag vm-bos David Herrmann
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: David Herrmann @ 2016-09-01 12:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
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 we can already fetch the EUID from
priv->pid->task->creds. The pointer-chasing will not hurt us, since it is
only about debugging, anyway.
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_fops.c | 1 -
drivers/gpu/drm/drm_info.c | 4 +++-
include/drm/drmP.h | 1 -
3 files changed, 3 insertions(+), 3 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..1df2d33 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 = 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),
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/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.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/6] drm: use drm_file to tag vm-bos
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
2016-09-01 12:48 ` [PATCH 1/6] drm: remove redundant drm_file->uid David Herrmann
@ 2016-09-01 12:48 ` David Herrmann
2016-09-01 12:48 ` [PATCH 3/6] drm: rename drm_file.filp to drm_file.legacy_filp David Herrmann
` (4 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: David Herrmann @ 2016-09-01 12:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
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/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/drm_vma_manager.h | 20 +++++++++--------
11 files changed, 51 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 9b61c8b..d975346 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/drm_vma_manager.c b/drivers/gpu/drm/drm_vma_manager.c
index f306c88..a788a80 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, struct drm_file *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,18 @@ 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,
+ struct drm_file *tag)
{
struct drm_vma_offset_file *entry;
struct rb_node *iter;
@@ -366,13 +366,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 +386,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.
@@ -397,7 +397,7 @@ EXPORT_SYMBOL(drm_vma_node_revoke);
* true iff @filp is on the list
*/
bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
- struct file *filp)
+ struct drm_file *tag)
{
struct drm_vma_offset_file *entry;
struct rb_node *iter;
@@ -407,9 +407,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/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 6190035..3a3e0b8 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 0c00e19..b4341fa 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,
diff --git a/include/drm/drm_vma_manager.h b/include/drm/drm_vma_manager.h
index 06ea8e07..304cdaa 100644
--- a/include/drm/drm_vma_manager.h
+++ b/include/drm/drm_vma_manager.h
@@ -24,16 +24,17 @@
*/
#include <drm/drm_mm.h>
-#include <linux/fs.h>
#include <linux/mm.h>
#include <linux/module.h>
#include <linux/rbtree.h>
#include <linux/spinlock.h>
#include <linux/types.h>
+struct drm_file;
+
struct drm_vma_offset_file {
struct rb_node vm_rb;
- struct file *vm_filp;
+ struct drm_file *vm_tag;
unsigned long vm_count;
};
@@ -62,10 +63,11 @@ 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);
+int drm_vma_node_allow(struct drm_vma_offset_node *node, struct drm_file *tag);
+void drm_vma_node_revoke(struct drm_vma_offset_node *node,
+ struct drm_file *tag);
bool drm_vma_node_is_allowed(struct drm_vma_offset_node *node,
- struct file *filp);
+ struct drm_file *tag);
/**
* drm_vma_offset_exact_lookup_locked() - Look up node by exact address
@@ -216,9 +218,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 +228,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)
+ struct drm_file *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.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/6] drm: rename drm_file.filp to drm_file.legacy_filp
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
2016-09-01 12:48 ` [PATCH 1/6] drm: remove redundant drm_file->uid David Herrmann
2016-09-01 12:48 ` [PATCH 2/6] drm: use drm_file to tag vm-bos David Herrmann
@ 2016-09-01 12:48 ` David Herrmann
2016-09-01 12:48 ` [PATCH 4/6] drm: provide management functions for drm_file David Herrmann
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: David Herrmann @ 2016-09-01 12:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
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.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/6] drm: provide management functions for drm_file
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
` (2 preceding siblings ...)
2016-09-01 12:48 ` [PATCH 3/6] drm: rename drm_file.filp to drm_file.legacy_filp David Herrmann
@ 2016-09-01 12:48 ` David Herrmann
2016-09-19 11:55 ` Daniel Vetter
2016-09-01 12:48 ` [PATCH 5/6] drm: drop obsolete drm_core.h David Herrmann
` (2 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: David Herrmann @ 2016-09-01 12:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
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.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 4/6] drm: provide management functions for drm_file
2016-09-01 12:48 ` [PATCH 4/6] drm: provide management functions for drm_file David Herrmann
@ 2016-09-19 11:55 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-09-19 11:55 UTC (permalink / raw)
To: David Herrmann; +Cc: Daniel Vetter, dri-devel
On Thu, Sep 01, 2016 at 02:48:35PM +0200, David Herrmann wrote:
> 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.
> + */
Hm, in drm core we only type kerneldoc for exported stuff, not internal
interfaces. The idea being that the target audience for those docs is
(mostly) driver authors.
And since you're touching this, drm_file.[hc] would look pretty I think,
with kerneldoc for the structures ...
-Daniel
> +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.3
>
--
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 5/6] drm: drop obsolete drm_core.h
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
` (3 preceding siblings ...)
2016-09-01 12:48 ` [PATCH 4/6] drm: provide management functions for drm_file David Herrmann
@ 2016-09-01 12:48 ` David Herrmann
2016-09-01 12:48 ` [PATCH 6/6] drm: cleanup drm_core_{init,exit}() David Herrmann
2016-09-19 11:56 ` [PATCH 0/6] DRM Core Cleanups Daniel Vetter
6 siblings, 0 replies; 10+ messages in thread
From: David Herrmann @ 2016-09-01 12:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
The drm_core.h header contains a set of constants meant to be used
throughout DRM. However, as it turns out, they're each used just once and
don't bring any benefit. They're also grossly mis-named and lack
name-spacing. This patch inlines them, or moves them into drm_internal.h
as appropriate:
- CORE_AUTHOR and CORE_DESC are inlined into corresponding MODULE_*()
macros. It's just confusing having to follow 2 pointers when trying to
find the definition of these fields. Grep'ping for MODULE_AUTHOR()
should reveal the full information, if there's no strong reason not to.
- CORE_NAME, CORE_DATE, CORE_MAJOR, CORE_MINOR, and CORE_PATCHLEVEL are
inlined into the sysfs 'version' attribute. They're stripped
everywhere else (which is just some printk() statements). CORE_NAME
just doesn't make *any* sense, as we hard-code it in many places,
anyway. The other constants are outdated and just serve
binary-compatibility purposes. Hence, inline them in 'version' sysfs
attribute (we might even try dropping it..).
- DRM_IF_MAJOR and DRM_IF_MINOR are moved into drm_internal.h as they're
only used by the global ioctl handlers. Furthermore, versioning
interfaces breaks backports and as such is deprecated, anyway. We just
keep them for historic reasons. I doubt anyone will ever modify them
again.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/drm_drv.c | 8 +++-----
drivers/gpu/drm/drm_internal.h | 3 +++
drivers/gpu/drm/drm_ioc32.c | 1 -
drivers/gpu/drm/drm_ioctl.c | 1 -
drivers/gpu/drm/drm_sysfs.c | 8 +-------
include/drm/drm_core.h | 34 ----------------------------------
6 files changed, 7 insertions(+), 48 deletions(-)
delete mode 100644 include/drm/drm_core.h
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9ab0016..d771453 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -33,7 +33,6 @@
#include <linux/mount.h>
#include <linux/slab.h>
#include <drm/drmP.h>
-#include <drm/drm_core.h>
#include "drm_crtc_internal.h"
#include "drm_legacy.h"
#include "drm_internal.h"
@@ -46,8 +45,8 @@
unsigned int drm_debug = 0;
EXPORT_SYMBOL(drm_debug);
-MODULE_AUTHOR(CORE_AUTHOR);
-MODULE_DESCRIPTION(CORE_DESC);
+MODULE_AUTHOR("Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl");
+MODULE_DESCRIPTION("DRM shared core routines");
MODULE_LICENSE("GPL and additional rights");
MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
"\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n"
@@ -966,8 +965,7 @@ static int __init drm_core_init(void)
goto err_p3;
}
- DRM_INFO("Initialized %s %d.%d.%d %s\n",
- CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
+ DRM_INFO("Initialized\n");
return 0;
err_p3:
drm_sysfs_destroy();
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 9b66cc4..3d6a587 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -21,6 +21,9 @@
* OTHER DEALINGS IN THE SOFTWARE.
*/
+#define DRM_IF_MAJOR 1
+#define DRM_IF_MINOR 4
+
/* drm_irq.c */
extern unsigned int drm_timestamp_monotonic;
diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index 57676f8..2795d6a 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -32,7 +32,6 @@
#include <linux/export.h>
#include <drm/drmP.h>
-#include <drm/drm_core.h>
#define DRM_IOCTL_VERSION32 DRM_IOWR(0x00, drm_version32_t)
#define DRM_IOCTL_GET_UNIQUE32 DRM_IOWR(0x01, drm_unique32_t)
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index bc5c65e..845390f 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -29,7 +29,6 @@
*/
#include <drm/drmP.h>
-#include <drm/drm_core.h>
#include <drm/drm_auth.h>
#include "drm_legacy.h"
#include "drm_internal.h"
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 32dd821..9a37196 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -19,7 +19,6 @@
#include <linux/export.h>
#include <drm/drm_sysfs.h>
-#include <drm/drm_core.h>
#include <drm/drmP.h>
#include "drm_internal.h"
@@ -37,12 +36,7 @@ static char *drm_devnode(struct device *dev, umode_t *mode)
return kasprintf(GFP_KERNEL, "dri/%s", dev_name(dev));
}
-static CLASS_ATTR_STRING(version, S_IRUGO,
- CORE_NAME " "
- __stringify(CORE_MAJOR) "."
- __stringify(CORE_MINOR) "."
- __stringify(CORE_PATCHLEVEL) " "
- CORE_DATE);
+static CLASS_ATTR_STRING(version, S_IRUGO, "drm 1.1.0 20060810");
/**
* drm_sysfs_init - initialize sysfs helpers
diff --git a/include/drm/drm_core.h b/include/drm/drm_core.h
deleted file mode 100644
index 4e75238..0000000
--- a/include/drm/drm_core.h
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Copyright 2004 Jon Smirl <jonsmirl@gmail.com>
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sub license,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice (including the
- * next paragraph) shall be included in all copies or substantial portions
- * of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL
- * VIA, S3 GRAPHICS, AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
- * DEALINGS IN THE SOFTWARE.
- */
-#define CORE_AUTHOR "Gareth Hughes, Leif Delgass, José Fonseca, Jon Smirl"
-
-#define CORE_NAME "drm"
-#define CORE_DESC "DRM shared core routines"
-#define CORE_DATE "20060810"
-
-#define DRM_IF_MAJOR 1
-#define DRM_IF_MINOR 4
-
-#define CORE_MAJOR 1
-#define CORE_MINOR 1
-#define CORE_PATCHLEVEL 0
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/6] drm: cleanup drm_core_{init,exit}()
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
` (4 preceding siblings ...)
2016-09-01 12:48 ` [PATCH 5/6] drm: drop obsolete drm_core.h David Herrmann
@ 2016-09-01 12:48 ` David Herrmann
2016-09-19 11:56 ` [PATCH 0/6] DRM Core Cleanups Daniel Vetter
6 siblings, 0 replies; 10+ messages in thread
From: David Herrmann @ 2016-09-01 12:48 UTC (permalink / raw)
To: dri-devel; +Cc: Daniel Vetter
Various cleanups to the DRM core initialization and exit handlers:
- Register chrdev last: Once register_chrdev() returns, open() will
succeed on the given chrdevs. This is usually not an issue, as no
chardevs are registered, yet. However, nodes can be created by
user-space via mknod(2), even though such major/minor combinations are
unknown to the kernel. Avoid calling into drm_stub_open() in those
cases.
Again, drm_stub_open() would just bail out as the inode is unknown,
but it's really non-obvious if you hack on drm_stub_open().
- Unify error-paths into just one label. All the error-path helpers can
be called even though the constructors were not called yet, or failed.
Hence, just call all cleanups unconditionally.
- Call into drm_global_release(). This is a no-op, but provides
debugging helpers in case there're GLOBALS left on module unload. This
function was unused until now.
- Use DRM_ERROR() instead of printk(), and also print the error-code on
failure (even if it is static!).
- Don't throw away error-codes of register_chrdev()!
- Don't hardcode -1 as errno. This is just plain wrong.
- Order exit-handlers in the exact reverse order of initialization
(except if the order actually matters for syncing-reasons, which is
not the case here, though).
v2:
- Call drm_core_exit() directly from the init-error-handler. Requires to
drop __exit annotation, though.
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
drivers/gpu/drm/drm_drv.c | 48 ++++++++++++++++++++++-------------------------
1 file changed, 22 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index d771453..0773547 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -941,52 +941,48 @@ static const struct file_operations drm_stub_fops = {
.llseek = noop_llseek,
};
+static void drm_core_exit(void)
+{
+ unregister_chrdev(DRM_MAJOR, "drm");
+ debugfs_remove(drm_debugfs_root);
+ drm_sysfs_destroy();
+ idr_destroy(&drm_minors_idr);
+ drm_connector_ida_destroy();
+ drm_global_release();
+}
+
static int __init drm_core_init(void)
{
- int ret = -ENOMEM;
+ int ret;
drm_global_init();
drm_connector_ida_init();
idr_init(&drm_minors_idr);
- if (register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops))
- goto err_p1;
-
ret = drm_sysfs_init();
if (ret < 0) {
- printk(KERN_ERR "DRM: Error creating drm class.\n");
- goto err_p2;
+ DRM_ERROR("Cannot create DRM class: %d\n", ret);
+ goto error;
}
drm_debugfs_root = debugfs_create_dir("dri", NULL);
if (!drm_debugfs_root) {
- DRM_ERROR("Cannot create /sys/kernel/debug/dri\n");
- ret = -1;
- goto err_p3;
+ ret = -ENOMEM;
+ DRM_ERROR("Cannot create debugfs-root: %d\n", ret);
+ goto error;
}
+ ret = register_chrdev(DRM_MAJOR, "drm", &drm_stub_fops);
+ if (ret < 0)
+ goto error;
+
DRM_INFO("Initialized\n");
return 0;
-err_p3:
- drm_sysfs_destroy();
-err_p2:
- unregister_chrdev(DRM_MAJOR, "drm");
- idr_destroy(&drm_minors_idr);
-err_p1:
+error:
+ drm_core_exit();
return ret;
}
-static void __exit drm_core_exit(void)
-{
- debugfs_remove(drm_debugfs_root);
- drm_sysfs_destroy();
-
- unregister_chrdev(DRM_MAJOR, "drm");
-
- drm_connector_ida_destroy();
- idr_destroy(&drm_minors_idr);
-}
-
module_init(drm_core_init);
module_exit(drm_core_exit);
--
2.9.3
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 0/6] DRM Core Cleanups
2016-09-01 12:48 [PATCH 0/6] DRM Core Cleanups David Herrmann
` (5 preceding siblings ...)
2016-09-01 12:48 ` [PATCH 6/6] drm: cleanup drm_core_{init,exit}() David Herrmann
@ 2016-09-19 11:56 ` Daniel Vetter
2016-09-19 11:58 ` Daniel Vetter
6 siblings, 1 reply; 10+ messages in thread
From: Daniel Vetter @ 2016-09-19 11:56 UTC (permalink / raw)
To: David Herrmann; +Cc: Daniel Vetter, dri-devel
On Thu, Sep 01, 2016 at 02:48:31PM +0200, David Herrmann wrote:
> Hey
>
> The remaining cleanup patches pending on dri-devel in one batch. Random cleanups
> all over the place. Should all be straightforward.
Except for patch 4 all applied to drm-misc, thanks.
-Daniel
>
> Thanks
> David
>
> David Herrmann (6):
> drm: remove redundant drm_file->uid
> drm: use drm_file to tag vm-bos
> drm: rename drm_file.filp to drm_file.legacy_filp
> drm: provide management functions for drm_file
> drm: drop obsolete drm_core.h
> drm: cleanup drm_core_{init,exit}()
>
> 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 | 191 ++++++++++++++++++++++++++------
> 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 | 7 ++
> drivers/gpu/drm/drm_ioc32.c | 1 -
> drivers/gpu/drm/drm_ioctl.c | 1 -
> drivers/gpu/drm/drm_sysfs.c | 8 +-
> 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 | 3 +-
> include/drm/drm_core.h | 34 ------
> include/drm/drm_vma_manager.h | 20 ++--
> 21 files changed, 235 insertions(+), 246 deletions(-)
> delete mode 100644 include/drm/drm_core.h
>
> --
> 2.9.3
>
--
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 0/6] DRM Core Cleanups
2016-09-19 11:56 ` [PATCH 0/6] DRM Core Cleanups Daniel Vetter
@ 2016-09-19 11:58 ` Daniel Vetter
0 siblings, 0 replies; 10+ messages in thread
From: Daniel Vetter @ 2016-09-19 11:58 UTC (permalink / raw)
To: David Herrmann; +Cc: Daniel Vetter, dri-devel
On Mon, Sep 19, 2016 at 01:56:56PM +0200, Daniel Vetter wrote:
> On Thu, Sep 01, 2016 at 02:48:31PM +0200, David Herrmann wrote:
> > Hey
> >
> > The remaining cleanup patches pending on dri-devel in one batch. Random cleanups
> > all over the place. Should all be straightforward.
>
> Except for patch 4 all applied to drm-misc, thanks.
I lied ;-) Patch 3 doesn't compile, you missed i810.
-Daniel
> -Daniel
>
> >
> > Thanks
> > David
> >
> > David Herrmann (6):
> > drm: remove redundant drm_file->uid
> > drm: use drm_file to tag vm-bos
> > drm: rename drm_file.filp to drm_file.legacy_filp
> > drm: provide management functions for drm_file
> > drm: drop obsolete drm_core.h
> > drm: cleanup drm_core_{init,exit}()
> >
> > 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 | 191 ++++++++++++++++++++++++++------
> > 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 | 7 ++
> > drivers/gpu/drm/drm_ioc32.c | 1 -
> > drivers/gpu/drm/drm_ioctl.c | 1 -
> > drivers/gpu/drm/drm_sysfs.c | 8 +-
> > 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 | 3 +-
> > include/drm/drm_core.h | 34 ------
> > include/drm/drm_vma_manager.h | 20 ++--
> > 21 files changed, 235 insertions(+), 246 deletions(-)
> > delete mode 100644 include/drm/drm_core.h
> >
> > --
> > 2.9.3
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
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
^ permalink raw reply [flat|nested] 10+ messages in thread