Re: Limit DRI2Drawable reference leak
On Sun, Feb 22, 2015 at 02:09:36AM +0200, Ville Syrjälä wrote: On Sat, Feb 21, 2015 at 10:52:49PM +, Chris Wilson wrote: On Sun, Feb 22, 2015 at 12:13:38AM +0200, Ville Syrjälä wrote: On Sat, Feb 21, 2015 at 09:31:07PM +, Chris Wilson wrote: With the current protocol and implementations, we have to often call DRI2CreateDrawable but can never call DRI2DestroyDrawable. This ends up with us leaking references to DRI2Drawables based on the assumption that the references have identical lifetimes to the Drawable going astray. This was spotted by Daniel Drake as the mali driver would create a new reference to the DRI2Drawable on every GetBuffers, but it can also be observed in mesa when running synthetic benchmarks (creating lots of contexts/glxdrawables for each window) and to a lesser extent with normal composited operation. The first two patches implement the capping of the unnamed internal reference used by DRI2CreateDrawable to just one per Client. IIRC we had many issues around the dri2 reference stuff during the Maemo days. Pauli fixed tons of problems in the dri2 code but some of the patches never made it in. These seem somewhat relevant: http://lists.x.org/archives/xorg-devel/2010-November/014783.html http://lists.x.org/archives/xorg-devel/2010-November/014782.html Indeed. Same problem, similar solution. I was a bit dubious as to whether we could hook up DRI2DestroyDrawable after all this time (for example mesa ignores it except for GLXPixmaps) and feared there was some corner case that was going to explode. Yeah dunno. This code did ship in the N9/N950 and our client side did issue these protocol requests appropriately. But then again, Pauli did a boatload of additional work on the dri2 code after these that didn't make it into upstream either. As I recall one of the issues was clients getting BadDrawables from DRI2DestroyDrawable if the X drawable already went away. And the solution was to decouple the lifetimes of the two. Yes, that is what I think the mesa patch is about. I think the solution would have been that the DRI2Drawable take a refcnt on its parent and then DRI2DestroyDrawable could be made to work. However, if we did that today we would end up with a massive Drawable leak - and pushing the change to mesa would also then expose us to the GLXDrawable vs Drawable close race (and BadDrawable errors again). This also sinks using named references - unless only named references also reference their parent. I think this is something we cannot fix, but we can at least apply damage limitation. I've probably forgotten most of what we did back then, but interested parties may go dig through https://www.gitorious.org/meego-w40/xserver-xorg if they wish. Ta, -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Limit DRI2Drawable reference leak
On Sun, Feb 22, 2015 at 09:02:41AM +, Chris Wilson wrote: Yes, that is what I think the mesa patch is about. I think the solution would have been that the DRI2Drawable take a refcnt on its parent and then DRI2DestroyDrawable could be made to work. However, if we did that today we would end up with a massive Drawable leak - and pushing the change to mesa would also then expose us to the GLXDrawable vs Drawable close race (and BadDrawable errors again). This also sinks using named references - unless only named references also reference their parent. Except Windows don't have reference counts. I think this is something we cannot fix, but we can at least apply damage limitation. Doubly so now. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Limit DRI2Drawable reference leak
On Sat, Feb 21, 2015 at 10:52:49PM +, Chris Wilson wrote: On Sun, Feb 22, 2015 at 12:13:38AM +0200, Ville Syrjälä wrote: On Sat, Feb 21, 2015 at 09:31:07PM +, Chris Wilson wrote: With the current protocol and implementations, we have to often call DRI2CreateDrawable but can never call DRI2DestroyDrawable. This ends up with us leaking references to DRI2Drawables based on the assumption that the references have identical lifetimes to the Drawable going astray. This was spotted by Daniel Drake as the mali driver would create a new reference to the DRI2Drawable on every GetBuffers, but it can also be observed in mesa when running synthetic benchmarks (creating lots of contexts/glxdrawables for each window) and to a lesser extent with normal composited operation. The first two patches implement the capping of the unnamed internal reference used by DRI2CreateDrawable to just one per Client. IIRC we had many issues around the dri2 reference stuff during the Maemo days. Pauli fixed tons of problems in the dri2 code but some of the patches never made it in. These seem somewhat relevant: http://lists.x.org/archives/xorg-devel/2010-November/014783.html http://lists.x.org/archives/xorg-devel/2010-November/014782.html Indeed. Same problem, similar solution. I was a bit dubious as to whether we could hook up DRI2DestroyDrawable after all this time (for example mesa ignores it except for GLXPixmaps) and feared there was some corner case that was going to explode. Yeah dunno. This code did ship in the N9/N950 and our client side did issue these protocol requests appropriately. But then again, Pauli did a boatload of additional work on the dri2 code after these that didn't make it into upstream either. As I recall one of the issues was clients getting BadDrawables from DRI2DestroyDrawable if the X drawable already went away. And the solution was to decouple the lifetimes of the two. I've probably forgotten most of what we did back then, but interested parties may go dig through https://www.gitorious.org/meego-w40/xserver-xorg if they wish. -- Ville Syrjälä syrj...@sci.fi http://www.sci.fi/~syrjala/ ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Limit DRI2Drawable reference leak
On Sat, Feb 21, 2015 at 09:31:07PM +, Chris Wilson wrote: With the current protocol and implementations, we have to often call DRI2CreateDrawable but can never call DRI2DestroyDrawable. This ends up with us leaking references to DRI2Drawables based on the assumption that the references have identical lifetimes to the Drawable going astray. This was spotted by Daniel Drake as the mali driver would create a new reference to the DRI2Drawable on every GetBuffers, but it can also be observed in mesa when running synthetic benchmarks (creating lots of contexts/glxdrawables for each window) and to a lesser extent with normal composited operation. The first two patches implement the capping of the unnamed internal reference used by DRI2CreateDrawable to just one per Client. IIRC we had many issues around the dri2 reference stuff during the Maemo days. Pauli fixed tons of problems in the dri2 code but some of the patches never made it in. These seem somewhat relevant: http://lists.x.org/archives/xorg-devel/2010-November/014783.html http://lists.x.org/archives/xorg-devel/2010-November/014782.html -- Ville Syrjälä syrj...@sci.fi http://www.sci.fi/~syrjala/ ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Re: Limit DRI2Drawable reference leak
On Sun, Feb 22, 2015 at 12:13:38AM +0200, Ville Syrjälä wrote: On Sat, Feb 21, 2015 at 09:31:07PM +, Chris Wilson wrote: With the current protocol and implementations, we have to often call DRI2CreateDrawable but can never call DRI2DestroyDrawable. This ends up with us leaking references to DRI2Drawables based on the assumption that the references have identical lifetimes to the Drawable going astray. This was spotted by Daniel Drake as the mali driver would create a new reference to the DRI2Drawable on every GetBuffers, but it can also be observed in mesa when running synthetic benchmarks (creating lots of contexts/glxdrawables for each window) and to a lesser extent with normal composited operation. The first two patches implement the capping of the unnamed internal reference used by DRI2CreateDrawable to just one per Client. IIRC we had many issues around the dri2 reference stuff during the Maemo days. Pauli fixed tons of problems in the dri2 code but some of the patches never made it in. These seem somewhat relevant: http://lists.x.org/archives/xorg-devel/2010-November/014783.html http://lists.x.org/archives/xorg-devel/2010-November/014782.html Indeed. Same problem, similar solution. I was a bit dubious as to whether we could hook up DRI2DestroyDrawable after all this time (for example mesa ignores it except for GLXPixmaps) and feared there was some corner case that was going to explode. Separating the two resource types cleans up the code slightly and should speed it as well (if many references do persist). Further info, the leak is a result of the conversion in: xserver commit 1da1f33f2dd5b437dd56cd9f5d6782de4ad5a1bc Author: Kristian Høgsberg k...@bitplanet.net Date: Fri Apr 16 05:55:34 2010 -0400 DRI2: Track DRI2 drawables as resources, not privates The main motivation here is to have the resource system clean up the DRI2 drawable automatically so glx doesn't have to. Right now, the glx drawable resource must be destroyed before the X drawable, so that calling DRI2DestroyDrawable doesn't crash. By making the DRI2 drawable a resource, GLX doesn't have to worry about that and the resource destruction order becomes irrelevant. But the scary one is mesa commit 4ebf07a426771b62123e5fcb5a8be0de24037af1 Author: Kristian Høgsberg k...@bitplanet.net Date: Mon Sep 13 08:39:42 2010 -0400 glx: Don't destroy DRI2 drawables for legacy glx drawables For GLX 1.3 drawables, we can destroy the DRI2 drawable when the GLX drawable is destroyed. However, for legacy drawables, there os no good way of knowing when the application is done with it, so we just let the DRI2 drawable linger on the server. The server will destroy the DRI2 drawable when it destroys the X drawable or the client exits anyway. https://bugs.freedesktop.org/show_bug.cgi?id=30109 Which leads me to believe that pushing the named references out to the Clients requires less thinking... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
Limit DRI2Drawable reference leak
With the current protocol and implementations, we have to often call DRI2CreateDrawable but can never call DRI2DestroyDrawable. This ends up with us leaking references to DRI2Drawables based on the assumption that the references have identical lifetimes to the Drawable going astray. This was spotted by Daniel Drake as the mali driver would create a new reference to the DRI2Drawable on every GetBuffers, but it can also be observed in mesa when running synthetic benchmarks (creating lots of contexts/glxdrawables for each window) and to a lesser extent with normal composited operation. The first two patches implement the capping of the unnamed internal reference used by DRI2CreateDrawable to just one per Client. The third patch uses a pair of new DRI2 requests to allow the Client to explicitly manage the lifetime via use of a named reference to the DRI2Drawable. -Chris ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
