On Wed, Apr 10, 2013 at 10:56:42AM +1000, Dave Airlie wrote:
Please don't bikeshed this with requirements to fix problems that are there now anyways. This is the simplest patch to fix an obvious problem, it doesn't fix all the other problems.
I should have merged this months ago, but people keep wanting a superpatch to fix everything.
Imo same review on the semantics of the patch itself still applies:
http://lists.freedesktop.org/archives/dri-devel/2012-December/032374.html
Two main things: - I think the dma_buf reference attached to gem handles should be dropped in drm_gem_object_handle_free instead of drm_gem_handle_delete. - I still have no idea what the drm_prime_lookup_buf_handle check in handle_to_fd is for ...
Note that the locking review was in a 2nd mail:
http://lists.freedesktop.org/archives/dri-devel/2012-December/032376.html
Imo this is an issue with this very patch since this patch also adds the dma_buf reference on exported objects while a handle is open.
So I don't think my original review asked for a superpatch to fix all the issues with currently have, but only for a correct one implementing the handle holds ref on exported obj logic ;-)
I'll try to pimp the self import testcase we have a bit to exercise these corner cases. -Daniel