diff options
author | Dave Airlie | 2013-04-21 18:54:36 -0500 |
---|---|---|
committer | Greg Kroah-Hartman | 2013-05-11 15:54:00 -0500 |
commit | fd282737ffe294ec6345338260d0754450cf2a7d (patch) | |
tree | 4b96d11399e76882cc935c9c670f97688fe73ec5 | |
parent | 30edf8c3596c5ec66b13c980196794c41e6ad020 (diff) | |
download | kernel-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.c | 4 | ||||
-rw-r--r-- | drivers/gpu/drm/drm_prime.c | 76 | ||||
-rw-r--r-- | include/drm/drmP.h | 5 |
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 | |||
205 | drm_gem_remove_prime_handles(struct drm_gem_object *obj, struct drm_file *filp) | 205 | drm_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 | }; |
64 | static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); | ||
64 | 65 | ||
65 | int drm_gem_prime_handle_to_fd(struct drm_device *dev, | 66 | int 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 | |||
113 | out_have_obj: | ||
114 | get_dma_buf(dmabuf); | ||
115 | *prime_fd = dma_buf_fd(dmabuf, flags); | ||
116 | out: | ||
117 | drm_gem_object_unreference_unlocked(obj); | ||
118 | mutex_unlock(&file_priv->prime.lock); | ||
119 | return ret; | ||
117 | } | 120 | } |
118 | EXPORT_SYMBOL(drm_gem_prime_handle_to_fd); | 121 | EXPORT_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 | } |
308 | EXPORT_SYMBOL(drm_prime_destroy_file_private); | 311 | EXPORT_SYMBOL(drm_prime_destroy_file_private); |
309 | 312 | ||
310 | int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle) | 313 | static 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 | } |
323 | EXPORT_SYMBOL(drm_prime_add_imported_buf_handle); | ||
324 | 327 | ||
325 | int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle) | 328 | int 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 | } |
337 | EXPORT_SYMBOL(drm_prime_lookup_imported_buf_handle); | 340 | EXPORT_SYMBOL(drm_prime_lookup_buf_handle); |
338 | 341 | ||
339 | void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf) | 342 | void 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 | } |
352 | EXPORT_SYMBOL(drm_prime_remove_imported_buf_handle); | 356 | EXPORT_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 | ||
1560 | void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); | 1560 | void drm_prime_init_file_private(struct drm_prime_file_private *prime_fpriv); |
1561 | void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); | 1561 | void drm_prime_destroy_file_private(struct drm_prime_file_private *prime_fpriv); |
1562 | int drm_prime_add_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t handle); | 1562 | int drm_prime_lookup_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle); |
1563 | int drm_prime_lookup_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf, uint32_t *handle); | 1563 | void drm_prime_remove_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf); |
1564 | void drm_prime_remove_imported_buf_handle(struct drm_prime_file_private *prime_fpriv, struct dma_buf *dma_buf); | ||
1565 | 1564 | ||
1566 | int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj); | 1565 | int drm_prime_add_dma_buf(struct drm_device *dev, struct drm_gem_object *obj); |
1567 | int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf, | 1566 | int drm_prime_lookup_obj(struct drm_device *dev, struct dma_buf *buf, |