Re: [Intel-gfx] [PATCH] Destroy screen pixmap on screen close.

2010-07-02 Thread Chris Wilson
On Thu,  1 Jul 2010 09:56:40 -0400, Keith Packard kei...@keithp.com wrote:
 This avoids a memory leak on server reset.
 
 Signed-off-by: Keith Packard kei...@keithp.com
 ---
  uxa/uxa.c |6 --
  1 files changed, 4 insertions(+), 2 deletions(-)
 
 diff --git a/uxa/uxa.c b/uxa/uxa.c
 index a9a705c..dcfaaa9 100644
 --- a/uxa/uxa.c
 +++ b/uxa/uxa.c
 @@ -1,7 +1,7 @@
  /*
 - * Copyright © 2001 Keith Packard
 + * Copyright © 2001 Keith Packard
   *
 - * Partly based on code that is Copyright © The XFree86 Project Inc.
 + * Partly based on code that is Copyright © The XFree86 Project Inc.
   *
   * Permission to use, copy, modify, distribute, and sell this software and 
 its
   * documentation for any purpose is hereby granted without fee, provided that
 @@ -381,6 +381,8 @@ static Bool uxa_close_screen(int i, ScreenPtr pScreen)
  
   uxa_glyphs_fini(pScreen);
  
 + (void) (*pScreen-DestroyPixmap) (pScreen-devPrivate);
 + pScreen-devPrivate = NULL;

This looks like the responsibility of miCloseScreen(). Are we failing to
chain up properly?

Currently we have

  uxa_close_screen - PictureCloseScreen - fbCloseScreen

and fbCloseScreen supersedes miCloseScreen. So should it not be
fbCloseScreen that calls miCloseScreen, since fb has taken over the
management of the mi interface?
-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] Destroy screen pixmap on screen close.

2010-07-02 Thread Keith Packard
On Fri, 02 Jul 2010 09:24:07 +0100, Chris Wilson ch...@chris-wilson.co.uk 
wrote:

 This looks like the responsibility of miCloseScreen(). Are we failing to
 chain up properly?

I don't think miCloseScreen (or fbCloseScreen) can do this -- before
we're called, rendering may not have been finished, after we're called,
the acceleration code is shut down and pixmaps cannot be freed.

-- 
keith.pack...@intel.com


pgpD1SJAXrNIk.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] Destroy screen pixmap on screen close.

2010-07-02 Thread Chris Wilson
On Fri, 02 Jul 2010 11:54:44 -0400, Keith Packard kei...@keithp.com wrote:
 On Fri, 02 Jul 2010 09:24:07 +0100, Chris Wilson ch...@chris-wilson.co.uk 
 wrote:
 
  This looks like the responsibility of miCloseScreen(). Are we failing to
  chain up properly?
 
 I don't think miCloseScreen (or fbCloseScreen) can do this -- before
 we're called, rendering may not have been finished, after we're called,
 the acceleration code is shut down and pixmaps cannot be freed.

That sounds true, and deserves a comment in the code. It doesn't explain
why fbCloseScreen() calls free(screen-devPrivate) instead of
miCloseScreen() and causing the leak in the first place... :)
-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx