aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDave Airlie2013-04-21 18:54:36 -0500
committerGreg Kroah-Hartman2013-05-11 15:54:00 -0500
commitfd282737ffe294ec6345338260d0754450cf2a7d (patch)
tree4b96d11399e76882cc935c9c670f97688fe73ec5
parent30edf8c3596c5ec66b13c980196794c41e6ad020 (diff)
downloadkernel-omap-fd282737ffe294ec6345338260d0754450cf2a7d.tar.gz
kernel-omap-fd282737ffe294ec6345338260d0754450cf2a7d.tar.xz
kernel-omap-fd282737ffe294ec6345338260d0754450cf2a7d.zip
drm/prime: keep a reference from the handle to exported dma-buf (v6)
commit 219b47339ced80ca580bb6ce7d1636166984afa7 upstream. Currently we have a problem with this: 1. i915: create gem object 2. i915: export gem object to prime 3. radeon: import gem object 4. close prime fd 5. radeon: unref object 6. i915: unref object i915 has an imported object reference in its file priv, that isn't cleaned up properly until fd close. The reference gets added at step 2, but at step 6 we don't have enough info to clean it up. The solution is to take a reference on the dma-buf when we export it, and drop the reference when the gem handle goes away. So when we export a dma_buf from a gem object, we keep track of it with the handle, we take a reference to the dma_buf. When we close the handle (i.e. userspace is finished with the buffer), we drop the reference to the dma_buf, and it gets collected. This patch isn't meant to fix any other problem or bikesheds, and it doesn't fix any races with other scenarios. v1.1: move export symbol line back up. v2: okay I had to do a bit more, as the first patch showed a leak on one of my tests, that I found using the dma-buf debugfs support, the problem case is exporting a buffer twice with the same handle, we'd add another export handle for it unnecessarily, however we now fail if we try to export the same object with a different gem handle, however I'm not sure if that is a case I want to support, and I've gotten the code to WARN_ON if we hit something like that. v2.1: rebase this patch, write better commit msg. v3: cleanup error handling, track import vs export in linked list, these two patches were separate previously, but seem to work better like this. v4: danvet is correct, this code is no longer useful, since the buffer better exist, so remove it. v5: always take a reference to the dma buf object, import or export. (Imre Deak contributed this originally) v6: square the circle, remove import vs export tracking now that there is no difference Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Signed-off-by: Dave Airlie <airlied@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/gpu/drm/drm_gem.c4
-rw-r--r--drivers/gpu/drm/drm_prime.c76
-rw-r--r--include/drm/drmP.h5
3 files changed, 44 insertions, 41 deletions
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 24efae464e2c..539bae92ace6 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -205,11 +205,11 @@ static void
205drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) 205drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp)
206{ 206{
207 if (obj->import_attach) { 207 if (obj->import_attach) {
208 drm_prime_remove_imported_buf_handle(&filp->prime, 208 drm_prime_remove_buf_handle(&filp->prime,
209 obj->import_attach->dmabuf); 209 obj->import_attach->dmabuf);
210 } 210 }
211 if (obj->export_dma_buf) { 211 if (obj->export_dma_buf) {
212 drm_prime_remove_imported_buf_handle(&filp->prime, 212 drm_prime_remove_buf_handle(&filp->prime,
213 obj->export_dma_buf); 213 obj->export_dma_buf);
214 } 214 }
215} 215}
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7f125738f44e..4f6439d7b39f 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -61,6 +61,7 @@ struct drm_prime_member {
61 struct dma_buf *dma_buf; 61 struct dma_buf *dma_buf;
62 uint32_t handle; 62 uint32_t handle;
63}; 63};
64static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle);
64 65
65int drm_gem_prime_handle_to_fd(struct drm_device *dev, 66int drm_gem_prime_handle_to_fd(struct drm_device *dev,
66 struct drm_file *file_priv, uint32_t handle, uint32_t flags, 67 struct drm_file *file_priv, uint32_t handle, uint32_t flags,
@@ -68,7 +69,8 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
68{ 69{
69 struct drm_gem_object *obj; 70 struct drm_gem_object *obj;
70 void *buf; 71 void *buf;
71 int ret; 72 int ret = 0;
73 struct dma_buf *dmabuf;
72 74
73 obj = drm_gem_object_lookup(dev, file_priv, handle); 75 obj = drm_gem_object_lookup(dev, file_priv, handle);
74 if (!obj) 76 if (!obj)
@@ -77,43 +79,44 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
77 mutex_lock(&file_priv->prime.lock); 79 mutex_lock(&file_priv->prime.lock);
78 /* re-export the original imported object */ 80 /* re-export the original imported object */
79 if (obj->import_attach) { 81 if (obj->import_attach) {
80 get_dma_buf(obj->import_attach->dmabuf); 82 dmabuf = obj->import_attach->dmabuf;
81 *prime_fd = dma_buf_fd(obj->import_attach->dmabuf, flags); 83 goto out_have_obj;
82 drm_gem_object_unreference_unlocked(obj);
83 mutex_unlock(&file_priv->prime.lock);
84 return 0;
85 } 84 }
86 85
87 if (obj->export_dma_buf) { 86 if (obj->export_dma_buf) {
88 get_dma_buf(obj->export_dma_buf); 87 dmabuf = obj->export_dma_buf;
89 *prime_fd = dma_buf_fd(obj->export_dma_buf, flags); 88 goto out_have_obj;
90 drm_gem_object_unreference_unlocked(obj);
91 } else {
92 buf = dev->driver->gem_prime_export(dev, obj, flags);
93 if (IS_ERR(buf)) {
94 /* normally the created dma-buf takes ownership of the ref,
95 * but if that fails then drop the ref
96 */
97 drm_gem_object_unreference_unlocked(obj);
98 mutex_unlock(&file_priv->prime.lock);
99 return PTR_ERR(buf);
100 }
101 obj->export_dma_buf = buf;
102 *prime_fd = dma_buf_fd(buf, flags);
103 } 89 }
90
91 buf = dev->driver->gem_prime_export(dev, obj, flags);
92 if (IS_ERR(buf)) {
93 /* normally the created dma-buf takes ownership of the ref,
94 * but if that fails then drop the ref
95 */
96 ret = PTR_ERR(buf);
97 goto out;
98 }
99 obj->export_dma_buf = buf;
100
104 /* if we've exported this buffer the cheat and add it to the import list 101 /* if we've exported this buffer the cheat and add it to the import list
105 * so we get the correct handle back 102 * so we get the correct handle back
106 */ 103 */
107 ret = drm_prime_add_imported_buf_handle(&file_priv->prime, 104 ret = drm_prime_add_buf_handle(&file_priv->prime,
108 obj->export_dma_buf, handle); 105 obj->export_dma_buf, handle);
109 if (ret) { 106 if (ret)
110 drm_gem_object_unreference_unlocked(obj); 107 goto out;
111 mutex_unlock(&file_priv->prime.lock);
112 return ret;
113 }
114 108
109 *prime_fd = dma_buf_fd(buf, flags);
115 mutex_unlock(&file_priv->prime.lock); 110 mutex_unlock(&file_priv->prime.lock);
116 return 0; 111 return 0;
112
113out_have_obj:
114 get_dma_buf(dmabuf);
115 *prime_fd = dma_buf_fd(dmabuf, flags);
116out:
117 drm_gem_object_unreference_unlocked(obj);
118 mutex_unlock(&file_priv->prime.lock);
119 return ret;
117} 120}
118EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); 121EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
119 122
@@ -130,7 +133,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
130 133
131 mutex_lock(&file_priv->prime.lock); 134 mutex_lock(&file_priv->prime.lock);
132 135
133 ret = drm_prime_lookup_imported_buf_handle(&file_priv->prime, 136 ret = drm_prime_lookup_buf_handle(&file_priv->prime,
134 dma_buf, handle); 137 dma_buf, handle);
135 if (!ret) { 138 if (!ret) {
136 ret = 0; 139 ret = 0;
@@ -149,7 +152,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,
149 if (ret) 152 if (ret)
150 goto out_put; 153 goto out_put;
151 154
152 ret = drm_prime_add_imported_buf_handle(&file_priv->prime, 155 ret = drm_prime_add_buf_handle(&file_priv->prime,
153 dma_buf, *handle); 156 dma_buf, *handle);
154 if (ret) 157 if (ret)
155 goto fail; 158 goto fail;
@@ -307,7 +310,7 @@ void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv)
307} 310}
308EXPORT_SYMBOL(drm_prime_destroy_file_private); 311EXPORT_SYMBOL(drm_prime_destroy_file_private);
309 312
310int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle) 313static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle)
311{ 314{
312 struct drm_prime_member *member; 315 struct drm_prime_member *member;
313 316
@@ -315,14 +318,14 @@ int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv
315 if (!member) 318 if (!member)
316 return -ENOMEM; 319 return -ENOMEM;
317 320
321 get_dma_buf(dma_buf);
318 member->dma_buf = dma_buf; 322 member->dma_buf = dma_buf;
319 member->handle = handle; 323 member->handle = handle;
320 list_add(&member->entry, &prime_fpriv->head); 324 list_add(&member->entry, &prime_fpriv->head);
321 return 0; 325 return 0;
322} 326}
323EXPORT_SYMBOL(drm_prime_add_imported_buf_handle);
324 327
325int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle) 328int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle)
326{ 329{
327 struct drm_prime_member *member; 330 struct drm_prime_member *member;
328 331
@@ -334,19 +337,20 @@ int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fp
334 } 337 }
335 return -ENOENT; 338 return -ENOENT;
336} 339}
337EXPORT_SYMBOL(drm_prime_lookup_imported_buf_handle); 340EXPORT_SYMBOL(drm_prime_lookup_buf_handle);
338 341
339void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf) 342void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf)
340{ 343{
341 struct drm_prime_member *member, *safe; 344 struct drm_prime_member *member, *safe;
342 345
343 mutex_lock(&prime_fpriv->lock); 346 mutex_lock(&prime_fpriv->lock);
344 list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) { 347 list_for_each_entry_safe(member, safe, &prime_fpriv->head, entry) {
345 if (member->dma_buf == dma_buf) { 348 if (member->dma_buf == dma_buf) {
349 dma_buf_put(dma_buf);
346 list_del(&member->entry); 350 list_del(&member->entry);
347 kfree(member); 351 kfree(member);
348 } 352 }
349 } 353 }
350 mutex_unlock(&prime_fpriv->lock); 354 mutex_unlock(&prime_fpriv->lock);
351} 355}
352EXPORT_SYMBOL(drm_prime_remove_imported_buf_handle); 356EXPORT_SYMBOL(drm_prime_remove_buf_handle);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fad21c927a38..881fb15d00c1 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1559,9 +1559,8 @@ extern void drm_prime_gem_destroy(struct drm_gem_object *obj, struct sg_table *s
1559 1559
1560void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); 1560void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv);
1561void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); 1561void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv);
1562int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); 1562int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle);
1563int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle); 1563void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
1564void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf);
1565 1564
1566int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj); 1565int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj);
1567int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf, 1566int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf,