[PATCH 1/2] drm/prime: keep a reference from the handle to exported dma-buf (v5)

[Daniel Vetter]

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:
> > 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)
> > 
> > Signed-off-by: Dave Airlie <airlied at 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 at 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 at ffwll.ch>
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch