Jesse, Thanks for the patch, I'm testing it now - it looks promising :)
X memory usage is more stable, but lsof | grep "drm mm" | wc -l still increases - after 10 minutes it went up from 100 to 500... Is this normal? cheers, Adam 2009/3/28 Jesse Barnes <[email protected]>: > On Fri, 27 Mar 2009 13:20:25 -0400 > Kristian Høgsberg <[email protected]> wrote: > >> On Thu, Mar 26, 2009 at 6:33 PM, Jesse Barnes >> <[email protected]> wrote: >> > Ok, I think this is where the leak was: >> > __glXUnrefDrawable->DrawableGone->__glXUnrefDrawable. This sequence >> > meant the glxPriv would stay around (since it was used), but >> > DrawableGone had generally already disposed of the pDrawable before >> > the call to Unref, so we never got into DRI2DestroyBuffers, thus >> > the leak. >> > >> > The loop seems broken to me. It *looks* like DrawableGone should be >> > the real free routine, only called when a drawable really is free, >> > so I've fixed up the code to conform to that. This means removing >> > the GLX priv frees from DRI and DRI2 routines and putting them here >> > and using the GLX unref routine everwhere (since it will only free >> > the resource when the refcount reaches 0). >> > >> > Any thoughts? I'd appreciate some more testers too... I'm not >> > quite sure about the generic DoDestroyDrawable change, but if the >> > refcount is always 1 there anyway it shouldn't make a difference >> > and seems more correct. >> >> The __GLXdrawable is a reference counted object, and the glx code >> references it in a couple of places: when there's an X resource >> pointing to it (a GLXWindow, GLXPixmap or GLXPbuffer) and when it's >> the current drawable or readable for a context. The __GLXdrawable is >> allocated by the backend in use (dri, dri2 or swrast) and must be >> freed by the same backend (don't mix alloc and free across abstraction >> barriers). We unref the __GLXdrawable when we either set a different >> drawable/readable and when the X resource goes away. >> >> The leak is (as you pointed out before) because we NULL the pDraw >> pointer before calling the backend destructor, which then can't unref >> the DRI2Drawable, which we then leak. You had the right fix in the >> originial post, we just need to not touch glxPriv after >> __glXUnrefDrawable if it got destroyed. I suggest this change to >> DrawableGone in irc: >> >> refcount = glxPriv->refcount; >> __glXUnrefDrawable(glxPriv); >> if (refcount > 1) { >> glxPriv->pDraw = NULL; >> glxPriv->drawId = 0; >> } > > Yep, that seems to work too... Magnus or Vasily can you guys confirm? > > Thanks, > -- > Jesse Barnes, Intel Open Source Technology Center > > diff --git a/glx/glxext.c b/glx/glxext.c > index c882372..fec45b7 100644 > --- a/glx/glxext.c > +++ b/glx/glxext.c > @@ -119,6 +119,7 @@ static int ContextGone(__GLXcontext* cx, XID id) > static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid) > { > ScreenPtr pScreen = glxPriv->pDraw->pScreen; > + int refcount; > > switch (glxPriv->type) { > case GLX_DRAWABLE_PIXMAP: > @@ -127,9 +128,12 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid) > break; > } > > - glxPriv->pDraw = NULL; > - glxPriv->drawId = 0; > + refcount = glxPriv->refCount; > __glXUnrefDrawable(glxPriv); > + if (refcount > 1) { > + glxPriv->pDraw = NULL; > + glxPriv->drawId = 0; > + } > > return True; > } > > _______________________________________________ > xorg mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/xorg _______________________________________________ xorg mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/xorg
