(In reply to comment #0)
> Version #6 of EGL_EXT_image_dma_buf_import changed the semantics of fd
> ownership. The EGL implementation no longer claims ownership of the fd.
> 
> #6 (David Garbett, December 05, 2013)
>    - Application now retains ownership of dma_buf file descriptors.
> 
> Mesa and Piglit needs to update to the new behavior.

Is there a way to {compile,run}-time detech the behaviour? Or is it expected that applications migrate once and for all when a new Mesa release occurs? Thanks.

Or, should this be considered as a genuine bug in Mesa implementation only, and applications should readily have been supporting #6 behaviour? On the other hand, #5 was out-of-draft already, thus indicating production-status somehow.

According to SVN log, the most approaching constant that could be checked would be EGL_EGLEXT_VERSION >= 20140114. i.e. r20692 for the EXT_image_dma_buf_import spec, and r24760 for the earliest version bump in eglext.h.

Anyway, our Mesa headers are not in sync, so that's probably not much useful. :)

As an additional comment,
I would like to add that glamor uses EGL_EXT_image_dma_buf_import with the new behaviour (actually I didn't know the spec was updated in December, but I knew the decision was taken to go this way).

Hi,

I have patches sent for fixing it:
http://lists.freedesktop.org/archives/mesa-dev/2014-August/065137.html
http://lists.freedesktop.org/archives/piglit/2014-August/012029.html

At least the Mesa patch will get a re-post after I get some more comments.

Patches are now in Piglit and Mesa:
http://cgit.freedesktop.org/piglit/commit/?id=f51d966f0610721a0947619950f43ddd4d29d42d
http://cgit.freedesktop.org/mesa/mesa/commit/?id=08264e5dad4df448e7718e782ad9077902089a07

There is still the question of what should be done with Mesa stable branches 10.0, 10.1, and 10.2. The patch applies on those.

I would apply the patch as piglit is already updated. Ian, Chad, any opinion?

I see little risk in cherry-picking the fix to stable branches. The fix is isolated and only *removes* code.

I do see risk in not cherry-picking the fix. If an app uses this extension with unfixed Mesa 10.2, then that app will leak file descriptors.

(In reply to comment #7)
> I see little risk in cherry-picking the fix to stable branches. The fix is
> isolated and only *removes* code.
> 
> I do see risk in not cherry-picking the fix. If an app uses this extension
> with unfixed Mesa 10.2, then that app will leak file descriptors.

Sounds good to me. I just wanted you or Ian to take a look at the patch before we shipped it in a release. :)

(In reply to comment #7)
> I do see risk in not cherry-picking the fix. If an app uses this extension
> with unfixed Mesa 10.2, then that app will leak file descriptors.

Hmm, isn't it the vice versa though?

If an app is written to work on unfixed Mesa (the app is broken), it works on both unfixed and fixed Mesa, but leaks fds on fixed Mesa, because nothing will close the fds given to Mesa.

If an app is written to work on fixed Mesa (the app is correct), it will not work on unfixed Mesa, because unfixed Mesa will close the fds behind the app's back. (Assuming the app actually needs to store and use the fds again.)

So, concretely, do I have to send a copy of the patch to mesa-stable@ or something, or is it already taken care of?
Any tag lines to add to the patch?

(In reply to comment #9)
> (In reply to comment #7)
> > I do see risk in not cherry-picking the fix. If an app uses this extension
> > with unfixed Mesa 10.2, then that app will leak file descriptors.
> 
> Hmm, isn't it the vice versa though?

Pekka, you're right. I spoke backwards.
 
> If an app is written to work on unfixed Mesa (the app is broken), it works
> on both unfixed and fixed Mesa, but leaks fds on fixed Mesa, because nothing
> will close the fds given to Mesa.
> 
> If an app is written to work on fixed Mesa (the app is correct), it will not
> work on unfixed Mesa, because unfixed Mesa will close the fds behind the
> app's back. (Assuming the app actually needs to store and use the fds again.)

So regardless of our decision, to backport or not to backport, there will be bugs. I think the right decision in this case is to prefer supporting apps written to the correct behavior. So let's backport.

There is another reason to backport: There likely have never existed apps that relied on the old behavior. During the Khronos discussion where we decided to "fix" the extension, the consensus was that there were no known in-production apps that used the extension. Therefore we believed it was safe to "fix" the extension to follow more traditional Linux fd ownership rules. No existent apps => no real compatibility break. Anyway, we predicted that the yet-to-appear client would might assume traditional fd ownership rules even if we decided to not fix the extension.

> So, concretely, do I have to send a copy of the patch to mesa-stable@ or
> something, or is it already taken care of?
> Any tag lines to add to the patch?

Just send a copy to mesa-stable@. If you're unsure about the protocol, ask Emil (xexaxo on #dri-devel).

Everyone following along... does backporting sound sensible to you too? Am I alone in that opinion?

Sent to Mesa-stable:
http://lists.freedesktop.org/archives/mesa-stable/2014-September/001683.html

Hello gents,

The commit has been picked up for the latest 10.2.7 release. The 10.0 and 10.1 branches are effectively EOL, so I do not anticipate any further releases for them.