On Mon, Jul 20, 2009 at 21:17, STEVE555<[email protected]> wrote: > > > > Ralovich, Kristóf wrote: >> >> Hi, >> >> I have spent a few full days debugging DRI configs and finally managed >> to find the leaking parts. See the attached patches. Tested and works >> with DRI2+intel and drisw. >> >> Please review and apply. >> >> Thanks, >> Kristof >> >> From 5ef3e20910b42ba33916f10af3a78bc7c6bf2fa0 Mon Sep 17 00:00:00 2001 >> From: =?utf-8?q?RALOVICH,=20Krist=C3=B3f?= <[email protected]> >> Date: Sun, 5 Jul 2009 15:41:43 +0200 >> Subject: [PATCH 1/4] glx: Free pdraw as __GLXDRIdrawablePrivate it is. >> >> --- >> src/glx/x11/dri2_glx.c | 3 ++- >> 1 files changed, 2 insertions(+), 1 deletions(-) >> >> diff --git a/src/glx/x11/dri2_glx.c b/src/glx/x11/dri2_glx.c >> index f4865ae..56529fa 100644 >> --- a/src/glx/x11/dri2_glx.c >> +++ b/src/glx/x11/dri2_glx.c >> @@ -154,7 +154,8 @@ static void dri2DestroyDrawable(__GLXDRIdrawable >> *pdraw) >> >> (*core->destroyDrawable)(pdraw->driDrawable); >> DRI2DestroyDrawable(pdraw->psc->dpy, pdraw->drawable); >> - Xfree(pdraw); >> + __GLXDRIdrawablePrivate *p = containerOf(pdraw, >> __GLXDRIdrawablePrivate, base); >> + Xfree(p); >> } >> >> static __GLXDRIdrawable *dri2CreateDrawable(__GLXscreenConfigs *psc, >> -- >> 1.6.3.3 >> >> >> From bc56676dae778f821be245824540f6e5028632b8 Mon Sep 17 00:00:00 2001 >> From: =?utf-8?q?RALOVICH,=20Krist=C3=B3f?= <[email protected]> >> Date: Sun, 5 Jul 2009 16:09:48 +0200 >> Subject: [PATCH 2/4] glx: implement _gl_context_modes_count() >> >> --- >> src/glx/x11/glcontextmodes.c | 25 +++++++++++++++++++++---- >> src/glx/x11/glcontextmodes.h | 2 ++ >> 2 files changed, 23 insertions(+), 4 deletions(-) >> >> diff --git a/src/glx/x11/glcontextmodes.c b/src/glx/x11/glcontextmodes.c >> index 232031c..3039dcf 100644 >> --- a/src/glx/x11/glcontextmodes.c >> +++ b/src/glx/x11/glcontextmodes.c >> @@ -29,6 +29,7 @@ >> * code base. >> * >> * \author Ian Romanick <[email protected]> >> + * \author RALOVICH, Kristof <[email protected]> >> */ >> >> #if defined(IN_MINI_GLX) >> @@ -66,6 +67,7 @@ >> >> #include "glcontextmodes.h" >> >> + >> #if !defined(IN_MINI_GLX) >> #define NUM_VISUAL_TYPES 6 >> >> @@ -357,7 +359,6 @@ _gl_get_context_mode_data(const __GLcontextModes * >> mode, int attribute, >> } >> #endif /* !defined(IN_MINI_GLX) */ >> >> - >> /** >> * Allocate a linked list of \c __GLcontextModes structures. The fields >> of >> * each structure will be initialized to "reasonable" default values. In >> @@ -427,7 +428,6 @@ _gl_context_modes_create(unsigned count, size_t >> minimum_size) >> return base; >> } >> >> - >> /** >> * Destroy a linked list of \c __GLcontextModes structures created by >> * \c _gl_context_modes_create. >> @@ -440,13 +440,11 @@ _gl_context_modes_destroy(__GLcontextModes * modes) >> { >> while (modes != NULL) { >> __GLcontextModes *const next = modes->next; >> - >> _mesa_free(modes); >> modes = next; >> } >> } >> >> - >> /** >> * Find a context mode matching a Visual ID. >> * >> @@ -542,3 +540,22 @@ _gl_context_modes_are_same(const __GLcontextModes * >> a, >> (a->bindToTextureTargets == b->bindToTextureTargets) && >> (a->yInverted == b->yInverted)); >> } >> + >> +/** >> + * Calculate the length of a linked list of \c __GLcontextModes >> + * structures. >> + * >> + * \param modes Linked list of structures to be counted. >> + */ >> +size_t >> +_gl_context_modes_count(const __GLcontextModes* modes) >> +{ >> + __GLcontextModes *m; >> + >> + size_t count = 0; >> + >> + for (m = modes; m != NULL; m = m->next) >> + count++; >> + >> + return count; >> +} >> diff --git a/src/glx/x11/glcontextmodes.h b/src/glx/x11/glcontextmodes.h >> index 6676ae3..4b764a7 100644 >> --- a/src/glx/x11/glcontextmodes.h >> +++ b/src/glx/x11/glcontextmodes.h >> @@ -25,6 +25,7 @@ >> /** >> * \file glcontextmodes.h >> * \author Ian Romanick <[email protected]> >> + * \author RALOVICH, Kristof <[email protected]> >> */ >> >> #ifndef GLCONTEXTMODES_H >> @@ -50,5 +51,6 @@ extern __GLcontextModes >> *_gl_context_modes_find_fbconfig(__GLcontextModes * >> modes, int >> fbid); >> extern GLboolean _gl_context_modes_are_same(const __GLcontextModes * a, >> const __GLcontextModes * b); >> +extern size_t _gl_context_modes_count(const __GLcontextModes* modes); >> >> #endif /* GLCONTEXTMODES_H */ >> -- >> 1.6.3.3 >> >> >> From ad42cabf13abff6ea8342d5d1fe30a975a300349 Mon Sep 17 00:00:00 2001 >> From: =?utf-8?q?RALOVICH,=20Krist=C3=B3f?= <[email protected]> >> Date: Sun, 5 Jul 2009 16:10:55 +0200 >> Subject: [PATCH 3/4] glx: implement functions for proper handling of DRI >> configs >> >> --- >> src/glx/x11/dri_common.c | 140 >> ++++++++++++++++++++++++++++++++++++++++++++++ >> src/glx/x11/dri_common.h | 14 +++++ >> 2 files changed, 154 insertions(+), 0 deletions(-) >> >> diff --git a/src/glx/x11/dri_common.c b/src/glx/x11/dri_common.c >> index 6de4111..be78c77 100644 >> --- a/src/glx/x11/dri_common.c >> +++ b/src/glx/x11/dri_common.c >> @@ -31,6 +31,7 @@ >> * Kevin E. Martin <[email protected]> >> * Brian Paul <[email protected]> >> * Kristian Høgsberg ([email protected]) >> + * RALOVICH, Kristof <[email protected]> >> */ >> >> #ifdef GLX_DIRECT_RENDERING >> @@ -294,6 +295,9 @@ createDriMode(const __DRIcoreExtension *core, >> if (driConfigs[i] == NULL) >> return NULL; >> >> + /* Now modes == driconfigs[i], equality is defined by >> + * driConfigEqual(). */ >> + >> config = Xmalloc(sizeof *config); >> if (config == NULL) >> return NULL; >> @@ -328,6 +332,142 @@ driConvertConfigs(const __DRIcoreExtension *core, >> return head.next; >> } >> >> +/* Free driver_configs[i] where createDriMode() didn't find a match. */ >> +_X_HIDDEN >> +void >> +driDestroyUnmatchedConfigs(const __DRIcoreExtension *core, >> + __GLcontextModes *visuals, >> + __GLcontextModes *fbconfigs, >> + __DRIconfig **driver_configs) >> +{ >> + int n; >> + for(n = 0; driver_configs[n]; n++) { >> + } >> + >> + int *matched = calloc(1, n*sizeof(int)); >> + >> + __GLcontextModes* m; >> + for(m = visuals; m; m = m->next) { >> + for(unsigned int i = 0; driver_configs[i]; i++) { >> + if(driConfigEqual(core, m, driver_configs[i])) { >> + matched[i]++; >> + break; >> + } >> + } >> + } >> + for(m = fbconfigs; m; m = m->next) { >> + for(unsigned int i = 0; driver_configs[i]; i++) { >> + if(driConfigEqual(core, m, driver_configs[i])) { >> + matched[i]++; >> + break; >> + } >> + } >> + } >> + >> + for(unsigned int i = 0; driver_configs[i]; i++) { >> + if(matched[i] < 1) { >> + free(driver_configs[i]); >> + } >> + } >> + >> + free(matched); >> +} >> + >> +/** >> + * Given a linked list of \c __GLcontextModes, put the \c >> + * __GLXDRIconfigPrivate and DRI \c __GLcontextModes (inside \c >> + * __DRIconfig) structures into respective sets filtering out >> + * duplicated instances. >> + * >> + * \param modes The input linked list of context modes. >> + * \param cpSet The output set (array) of \c __GLXDRIconfigPrivate* >> instances. >> + * \param cmSet The output set (array) of \c __GLcontextModes* instances. >> + */ >> +static >> +void >> +driConfigLinkedListToArraySet(const __GLcontextModes* modes, >> + __GLXDRIconfigPrivate** cpSet, >> + __GLcontextModes** cmSet) >> +{ >> + __GLcontextModes* m; >> + for(m = (__GLcontextModes*)modes; m; m = m->next) >> + { >> + __GLXDRIconfigPrivate* cp = containerOf(m, __GLXDRIconfigPrivate, >> modes); >> + >> + unsigned int i = 0; >> + for( ; cpSet[i]; i++) { >> + if(cp == cpSet[i]) >> + goto cp_already_in; >> + } >> + cpSet[i] = cp; >> + cp_already_in: >> + ; >> + >> + /* This would be the elegant way, but requires the definition of >> + * __DRIconfig/__DRIconfigRec from dri/common/utils.h. >> + * >> + * __GLcontextModes* cm = >> + * containerOf((__DRIconfig*)cp->driConfig, __DRIconfig, >> modes); >> + * >> + */ >> + __GLcontextModes* cm = (__GLcontextModes*)cp->driConfig; >> + unsigned int j = 0; >> + for( ; cmSet[j]; j++) { >> + if(cm == cmSet[j]) >> + goto cm_already_in; >> + } >> + cmSet[j] = cm; >> + cm_already_in: >> + ; >> + } >> +} >> + >> +/** >> + * Given two linked lists of Visuals and FBConfigs, put the \c >> + * __GLXDRIconfigPrivate and DRI \c __GLcontextModes (inside \c >> + * __DRIconfig) structures into respective sets filtering out >> + * duplicated instances. The output arrays must be zeroed where unused >> + * and must be large enough to accomodate all the different values of >> + * inputs. >> + * >> + * \param visuals The input linked list of Visuals. >> + * \param fbconfigs The input linked list of FBConfigs. >> + * \param cpSet The output set (array) of \c __GLXDRIconfigPrivate* >> instances. >> + * \param cmSet The output set (array) of \c __GLcontextModes* instances. >> + */ >> +_X_HIDDEN >> +void >> +driCollectConfigs(const __GLcontextModes* visuals, >> + const __GLcontextModes* fbconfigs, >> + __GLXDRIconfigPrivate** cpSet, >> + __GLcontextModes** cmSet) >> +{ >> + driConfigLinkedListToArraySet(visuals, cpSet, cmSet); >> + driConfigLinkedListToArraySet(fbconfigs, cpSet, cmSet); >> +} >> + >> +/** >> + * Release \c NULL terminated arrays of \c __GLXDRIconfigPrivate and >> + * \c __GLcontextModes. >> + */ >> +_X_HIDDEN >> +void >> +driDestroyConfigs(__GLXDRIconfigPrivate** cpSet, >> + __GLcontextModes** cmSet) >> +{ >> + int i = 0; >> + for( ; cpSet[i]; i++) >> + { >> + free(cpSet[i]); >> + } >> + >> + i=0; >> + for( ; cmSet[i]; i++) >> + { >> + free(cmSet[i]); >> + } >> +} >> + >> _X_HIDDEN void >> driBindExtensions(__GLXscreenConfigs *psc, int dri2) >> { >> diff --git a/src/glx/x11/dri_common.h b/src/glx/x11/dri_common.h >> index 61ac9c6..4715b42 100644 >> --- a/src/glx/x11/dri_common.h >> +++ b/src/glx/x11/dri_common.h >> @@ -31,6 +31,7 @@ >> * Kevin E. Martin <[email protected]> >> * Brian Paul <[email protected]> >> * Kristian Høgsberg ([email protected]) >> + * RALOVICH, Kristof <[email protected]> >> */ >> >> #ifndef _DRI_COMMON_H >> @@ -48,6 +49,19 @@ extern __GLcontextModes *driConvertConfigs(const >> __DRIcoreExtension * core, >> __GLcontextModes * modes, >> const __DRIconfig ** configs); >> >> +extern void driDestroyUnmatchedConfigs(const __DRIcoreExtension *core, >> + __GLcontextModes *visuals, >> + __GLcontextModes *fbconfigs, >> + __DRIconfig **driver_configs); >> + >> +extern void driCollectConfigs(const __GLcontextModes* visuals, >> + const __GLcontextModes* fbconfigs, >> + __GLXDRIconfigPrivate** cpSet, >> + __GLcontextModes** cmSet); >> + >> +extern void driDestroyConfigs(__GLXDRIconfigPrivate** cpSet, >> + __GLcontextModes** cmSet); >> + >> extern const __DRIsystemTimeExtension systemTimeExtension; >> >> extern void InfoMessageF(const char *f, ...); >> -- >> 1.6.3.3 >> >> >> From b3fa73ca1e2897ea5f090e14ecabe94d624bde16 Mon Sep 17 00:00:00 2001 >> From: =?utf-8?q?RALOVICH,=20Krist=C3=B3f?= <[email protected]> >> Date: Sun, 5 Jul 2009 16:23:28 +0200 >> Subject: [PATCH 4/4] glx: release DRI context modes properly >> >> This includes destroying those that are unmatched during >> dri2CreateScreen(). >> --- >> src/glx/x11/dri2_glx.c | 4 ++ >> src/glx/x11/glxext.c | 73 >> ++++++++++++++++++++++++++++++++++++++++++------ >> 2 files changed, 68 insertions(+), 9 deletions(-) >> >> diff --git a/src/glx/x11/dri2_glx.c b/src/glx/x11/dri2_glx.c >> index 56529fa..9062419 100644 >> --- a/src/glx/x11/dri2_glx.c >> +++ b/src/glx/x11/dri2_glx.c >> @@ -28,6 +28,7 @@ >> * >> * Authors: >> * Kristian Høgsberg ([email protected]) >> + * RALOVICH, Kristof <[email protected]> >> */ >> >> #ifdef GLX_DIRECT_RENDERING >> @@ -489,6 +490,9 @@ static __GLXDRIscreen >> *dri2CreateScreen(__GLXscreenConfigs *psc, int screen, >> psc->configs = driConvertConfigs(psc->core, psc->configs, >> driver_configs); >> psc->visuals = driConvertConfigs(psc->core, psc->visuals, >> driver_configs); >> >> + driDestroyUnmatchedConfigs(psc->core, psc->visuals, psc->configs, >> driver_configs); >> + free(driver_configs); >> + >> psp->destroyScreen = dri2DestroyScreen; >> psp->createContext = dri2CreateContext; >> psp->createDrawable = dri2CreateDrawable; >> diff --git a/src/glx/x11/glxext.c b/src/glx/x11/glxext.c >> index b296b7c..3b98c88 100644 >> --- a/src/glx/x11/glxext.c >> +++ b/src/glx/x11/glxext.c >> @@ -35,6 +35,7 @@ >> * Direct rendering support added by Precision Insight, Inc. >> * >> * \author Kevin E. Martin <[email protected]> >> + * \author RALOVICH, Kristof <[email protected]> >> */ >> >> #include <assert.h> >> @@ -51,6 +52,9 @@ >> #include <xcb/glx.h> >> #endif >> >> +#ifdef GLX_DIRECT_RENDERING >> +# include "dri_common.h" >> +#endif >> >> #ifdef DEBUG >> void __glXDumpDrawBuffer(__GLXcontext * ctx); >> @@ -122,6 +126,53 @@ XEXT_GENERATE_FIND_DISPLAY(__glXFindDisplay, >> __glXExtensionInfo, >> __GLX_NUMBER_EVENTS, NULL) >> >> >> /************************************************************************/ >> + >> +#ifdef GLX_DIRECT_RENDERING >> +/** >> + * DRI is using the bigger __GLXDRIconfigPrivate structure inherited >> + * from __GLcontextModes structure. We nedd to walk the linked list of >> + * those and make sure shared instances are not freed more than once. >> + */ >> +static >> +void >> +DestroyDRIConfigs(__GLXdisplayPrivate * priv) >> +{ >> + >> + __GLXscreenConfigs *psc; >> + GLint screens; >> + size_t modes = 0; >> + >> + psc = priv->screenConfigs; >> + screens = ScreenCount(priv->dpy); >> + >> + /* Count the length of the linked lists of configs to get an upper >> + * bound on the sizes of the required sets. */ >> + for(unsigned int i = 0; i < screens; i++, psc++) { >> + if(psc->configs) >> + modes += _gl_context_modes_count(psc->configs); >> + if(psc->visuals) >> + modes += _gl_context_modes_count(psc->visuals); >> + } >> + >> + /* Create large enough sets to hold the different configs. Inserting >> + * duplicated instances only once. */ >> + __GLXDRIconfigPrivate** cpSet = calloc(1, modes * 2 * sizeof(*cpSet)); >> + __GLcontextModes** cmSet = calloc(1, modes * 2 * sizeof(*cmSet)); >> + >> + psc = priv->screenConfigs; >> + for(unsigned int i = 0; i < screens; i++, psc++) { >> + driCollectConfigs(psc->configs, psc->visuals, cpSet, cmSet); >> + psc->visuals = 0; >> + psc->configs = 0; >> + } >> + >> + driDestroyConfigs(cpSet, cmSet); >> + >> + free(cmSet); >> + free(cpSet); >> +} >> +#endif >> + >> /* >> ** Free the per screen configs data as well as the array of >> ** __glXScreenConfigs. >> @@ -132,20 +183,24 @@ FreeScreenConfigs(__GLXdisplayPrivate * priv) >> __GLXscreenConfigs *psc; >> GLint i, screens; >> >> +#ifdef GLX_DIRECT_RENDERING >> + DestroyDRIConfigs(priv); >> +#endif >> + >> /* Free screen configuration information */ >> psc = priv->screenConfigs; >> screens = ScreenCount(priv->dpy); >> for (i = 0; i < screens; i++, psc++) { >> - if (psc->configs) { >> - _gl_context_modes_destroy(psc->configs); >> - if (psc->effectiveGLXexts) >> - Xfree(psc->effectiveGLXexts); >> - psc->configs = NULL; /* NOTE: just for paranoia */ >> - } >> - if (psc->visuals) { >> + >> +#ifndef GLX_DIRECT_RENDERING >> + if(psc->visuals) >> _gl_context_modes_destroy(psc->visuals); >> - psc->visuals = NULL; /* NOTE: just for paranoia */ >> - } >> + if(psc->configs) >> + _gl_context_modes_destroy(psc->configs); >> +#endif >> + >> + if (psc->effectiveGLXexts) >> + Xfree(psc->effectiveGLXexts); >> Xfree((char *) psc->serverGLXexts); >> >> #ifdef GLX_DIRECT_RENDERING >> -- >> 1.6.3.3 >> >> >> ------------------------------------------------------------------------------ >> >> _______________________________________________ >> Mesa3d-dev mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/mesa3d-dev >> >> > Hi to all, > I'm currently building Mesa,but the xorg state-tracker is > broken.From having the error messages I got from where the build stopped,I > looked into the problem and discovered these patches that could potentially > fix my problem. > I wish to know if they are going to be committed to the Mesa master git > branch?.Alternatively,is there is a way of using git to get the patches,or > can I copy the separate pieces of code and save them as diffs and apply them > that way?I was wondering if I do the latter,will it affect the Mesa git code > the next time I do a git pull and they get committed after I applied the > patches? > > Regards, > STEVE555 > -- > View this message in context: > http://www.nabble.com/-PATCH--proper-handling-and-destruction-of-DRI-configs-tp24343609p24575871.html > Sent from the mesa3d-dev mailing list archive at Nabble.com. > > > ------------------------------------------------------------------------------ > Enter the BlackBerry Developer Challenge > This is your chance to win up to $100,000 in prizes! For a limited time, > vendors submitting new applications to BlackBerry App World(TM) will have > the opportunity to enter the BlackBerry Developer Challenge. See full prize > details at: http://p.sf.net/sfu/Challenge > _______________________________________________ > Mesa3d-dev mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/mesa3d-dev >
I highly doubt, that these patches fix anything not related to GLX screen config leakages. Kristof ------------------------------------------------------------------------------ Enter the BlackBerry Developer Challenge This is your chance to win up to $100,000 in prizes! For a limited time, vendors submitting new applications to BlackBerry App World(TM) will have the opportunity to enter the BlackBerry Developer Challenge. See full prize details at: http://p.sf.net/sfu/Challenge _______________________________________________ Mesa3d-dev mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
