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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
