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,
I have the output of my Mesa build where it stops here:
gmake[4]: Leaving directory `/opt/mesa/src/gallium/state_trackers/egl'
gmake[4]: Entering directory `/opt/mesa/src/gallium/state_trackers/xorg'
rm -f depend
touch depend
/usr/bin/makedepend -fdepend -I/usr/lib/gcc/i586-redhat-linux/4.4.0/include
-I. -I../../../../src/gallium/include -I../../../../src/gallium/auxiliary
-I../../../../src/gallium/drivers -I/src/gallium/include
-I/src/gallium/auxiliary -I/src/gallium/drivers -DHAVE_CONFIG_H
-I/usr/include/pixman-1 -I/usr/include/xorg -I/usr/include/drm
-I../../../../src/gallium/include -I../../../../src/gallium/auxiliary
-I../../../../include -I../../../../src/mesa
-I../../../../src/mesa/drivers/dri/common -I../../../../src/mesa/main
./xorg_composite.c ./xorg_crtc.c ./xorg_dri2.c ./xorg_driver.c ./xorg_exa.c
./xorg_output.c 2> /dev/null
gmake[4]: Leaving directory `/opt/mesa/src/gallium/state_trackers/xorg'
gmake[4]: Entering directory `/opt/mesa/src/gallium/state_trackers/xorg'
gcc -c -I. -I../../../../src/gallium/include
-I../../../../src/gallium/auxiliary -I../../../../src/gallium/drivers
-I/src/gallium/include -I/src/gallium/auxiliary -I/src/gallium/drivers
-DHAVE_CONFIG_H -I/usr/include/pixman-1 -I/usr/include/xorg
-I/usr/include/drm -I../../../../src/gallium/include
-I../../../../src/gallium/auxiliary -I../../../../include
-I../../../../src/mesa -I../../../../src/mesa/drivers/dri/common
-I../../../../src/mesa/main -g -O2 -Wall -Wmissing-prototypes -std=c99
-ffast-math -fno-strict-aliasing -m32 -fPIC -DUSE_X86_ASM -DUSE_MMX_ASM
-DUSE_3DNOW_ASM -DUSE_SSE_ASM -D_GNU_SOURCE -DPTHREADS -DHAVE_POSIX_MEMALIGN
-DMESA_SELINUX -DUSE_XCB -DUSE_EXTERNAL_DXTN_LIB=1 -DIN_DRI_DRIVER
-DGLX_DIRECT_RENDERING -DGLX_INDIRECT_RENDERING -DHAVE_ALIAS
xorg_composite.c -o xorg_composite.o
In file included from xorg_tracker.h:37,
from xorg_exa.h:5,
from xorg_composite.h:5,
from xorg_composite.c:2:
/usr/include/drm/drm.h:123: error: expected specifier-qualifier-list before
‘size_t’
/usr/include/drm/drm.h:137: error: expected specifier-qualifier-list before
‘size_t’
gmake[4]: *** [xorg_composite.o] Error 1
gmake[4]: Leaving directory `/opt/mesa/src/gallium/state_trackers/xorg'
gmake[3]: *** [subdirs] Error 1
gmake[3]: Leaving directory `/opt/mesa/src/gallium/state_trackers'
gmake[2]: *** [default] Error 1
gmake[2]: Leaving directory `/opt/mesa/src/gallium'
gmake[1]: *** [subdirs] Error 1
gmake[1]: Leaving directory `/opt/mesa/src'
gmake: *** [default] Error 1
If I leave out the xorg state-tracker,I know Mesa builds fine.
Regards,
STEVE555
--
View this message in context:
http://www.nabble.com/-PATCH--proper-handling-and-destruction-of-DRI-configs-tp24343609p24583359.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
