On Fri, Apr 19, 2013 at 08:54:58AM +0300, Imre Deak wrote:
On Fri, 2013-04-19 at 11:11 +1000, Dave Airlie wrote:
Currently we have a problem with this:
- i915: create gem object
- i915: export gem object to prime
- radeon: import gem object
- close prime fd
- radeon: unref object
- 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)
Signed-off-by: Dave Airlie airlied@redhat.com
drm_prime_destroy_file_private() lacks a drm_buf_put(). A separate issue, but the list should actually be empty at that point so perhaps we should warn if it's not.
bikeshed: we don't really need the PRIME_{EXPORT,IMPORT} tracking.
Otherwise looks ok: Reviewed-by: Imre Deak imre.deak@intel.com
Yeah, I couldn't find anything else to bitch about than what Imre spotted, and we can easily fix that up in follow-up patches for 3.11. So
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch