2012/11/30 Inki Dae <inki.dae@xxxxxxxxxxx>
Hello, all.
This patch introduces reference count of gem object name and resolves
the below issue.
In case that proces A shares its own gem object with process B,
process B opens a gem object name from process A to get its own
gem handle. But if process A closes the gem handle, the gem object
name to this is also released. So the gem object name that process
B referring to becomes invalid. I'm not sure that this is issue or
not but at least, gem object name IS NOT process-unique but IS
gem object-unique. So I think the gem object name shared by process A
should be preserved as long as the gem object has not been released.
Below is simple example.
at P1:
1. gem create => obj_refcount = 1
2. gem flink => obj_refcount = 2 (allocate obj_name)
5. gem close
a. obj_refcount = 2 and release the obj_name
b. obj_refcount = 1 once the obj_name release
3. and share it with P2
at P2:
4. gem open => obj_refcount = 3
6. the obj_name from P1 is invalid.
7. gem close => obj_refcount = 0(released)
And with this patch,
at P1:
1. gem create => obj_refcount = 1, name_refcount = 0
2. gem flink => obj_refcount = 2, name_refcount = 1 (allocate obj_name)
5. gem close => obj_refcount = 2, name_refcount = 1
3. and share it with P2
at P2:
4. gem open => obj_refcount = 3, name_refcount = 2
6. the gem object name from P1 is valid.
7. gem close
a. obj_refcount = 1, name_refcount = 0(released)
b. obj_refcount = 0(released) once name_ref_count = 0
There may be my missing point so please give me any advice.
Thanks,
Inki Dae
Signed-off-by: Inki Dae <inki.dae@xxxxxxxxxxx>
---
drivers/gpu/drm/drm_gem.c | 41 ++++++++++++++++++++++++++++++++++++++---
include/drm/drmP.h | 12 ++++++++++++
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 24efae4..16e4b75 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -145,6 +145,7 @@ int drm_gem_object_init(struct drm_device *dev,
kref_init(&obj->refcount);
atomic_set(&obj->handle_count, 0);
+ atomic_set(&obj->obj_name_count, 0);
obj->size = size;
return 0;
@@ -166,6 +167,7 @@ int drm_gem_private_object_init(struct drm_device *dev,
kref_init(&obj->refcount);
atomic_set(&obj->handle_count, 0);
+ atomic_set(&obj->obj_name_count, 0);
obj->size = size;
return 0;
@@ -452,6 +454,19 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
return -ENOENT;
again:
+ /*
+ * return current object name increasing reference count of
+ * this object name if exists already and not same process.
+ */
+ if (obj->name) {
+ if (file_priv->pid != current->pid)
This condition should be removed and placed with another. It's my mistake.
+ atomic_inc(&obj->obj_name_count);
+
+ args->name = (uint64_t)obj->name;
+ drm_gem_object_unreference_unlocked(obj);
+ return 0;
+ }
+
if (idr_pre_get(&dev->object_name_idr, GFP_KERNEL) == 0) {
ret = -ENOMEM;
goto err;
@@ -461,6 +476,7 @@ again:
if (!obj->name) {
ret = idr_get_new_above(&dev->object_name_idr, obj, 1,
&obj->name);
+ atomic_set(&obj->obj_name_count, 1);
args->name = (uint64_t) obj->name;
spin_unlock(&dev->object_name_lock);
@@ -502,8 +518,11 @@ drm_gem_open_ioctl(struct drm_device *dev, void *data,
spin_lock(&dev->object_name_lock);
obj = idr_find(&dev->object_name_idr, (int) args->name);
- if (obj)
+ if (obj) {
drm_gem_object_reference(obj);
+ if (file_priv->pid != current->pid)
ditto. For this, will re-send it.
+ atomic_inc(&obj->obj_name_count);
+ }
spin_unlock(&dev->object_name_lock);
if (!obj)
return -ENOENT;
@@ -612,9 +631,25 @@ void drm_gem_object_handle_free(struct drm_gem_object *obj)
/* Remove any name for this object */
spin_lock(&dev->object_name_lock);
if (obj->name) {
- idr_remove(&dev->object_name_idr, obj->name);
- obj->name = 0;
+ /*
+ * The name of this object could being referenced
+ * by another process so remove the name after checking
+ * the obj_name_count of this object.
+ */
+ if (atomic_dec_and_test(&obj->obj_name_count)) {
+ idr_remove(&dev->object_name_idr, obj->name);
+ obj->name = 0;
+ } else {
+ /*
+ * this object name is being referenced by other yet
+ * so do not unreference this.
+ */
+ spin_unlock(&dev->object_name_lock);
+ return;
+ }
+
spin_unlock(&dev->object_name_lock);
+
/*
* The object name held a reference to this object, drop
* that now.
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c9..27657b8 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -628,6 +628,18 @@ struct drm_gem_object {
/** Handle count of this object. Each handle also holds a reference */
atomic_t handle_count; /* number of handles on this object */
+ /*
+ * Name count of this object.
+ *
+ * This count is initialized as 0 with drm_gem_object_init or
+ * drm_gem_private_object_init call. After that, this count is
+ * increased if the name of this object exists already
+ * otherwise is set to 1 at flink. And finally, the name of
+ * this object will be released when this count reaches 0
+ * by gem close.
+ */
+ atomic_t obj_name_count;
+
/** Related drm device */
struct drm_device *dev;
--
1.7.4.1
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
