[Mesa-dev] [PATCH] r600/compute: Don't leak compute pool item_list/unallocated_list

[Bruno Jimenez]

On Mon, 2014-08-25 at 18:04 +0200, Jan Vesely wrote:
> On Sat, 2014-08-23 at 17:01 +0200, Bruno Jimenez wrote:
> > On Thu, 2014-08-21 at 14:37 -0500, Aaron Watry wrote:
> > > v2: Change to C-style comments and fix indentation
> > > 
> > > Signed-off-by: Aaron Watry <awatry at gmail.com>
> > > ---
> > >  src/gallium/drivers/r600/compute_memory_pool.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/src/gallium/drivers/r600/compute_memory_pool.c b/src/gallium/drivers/r600/compute_memory_pool.c
> > > index 9324b84..82de9cd 100644
> > > --- a/src/gallium/drivers/r600/compute_memory_pool.c
> > > +++ b/src/gallium/drivers/r600/compute_memory_pool.c
> > > @@ -95,6 +95,11 @@ void compute_memory_pool_delete(struct compute_memory_pool* pool)
> > >  		pool->screen->b.b.resource_destroy((struct pipe_screen *)
> > >  			pool->screen, (struct pipe_resource *)pool->bo);
> > >  	}
> > > +	/* In theory, all of the items were freed in compute_memory_free.
> > > +	   Just delete the list heads */
> > 
> > Hi,
> > 
> > If you are worried about the items not have been freed, you can try
> > doing something like this (mostly copied from compute_memory_free):
> 
> Is the situation legal? My current understanding is that leftover items
> would indicate a bug somewhere in the state tracker.
> would assert(LIST_IS_EMPTY(pool->item_list))) ever hit?
> 
> jan

Hi,

I'm afraid that my knowledge of the state tracker is not enough to
assure this. But I mostly agree that the state tracker should be
responsible for deleting all the items before deleting the pool itself.

Anyway, Aaron says that there doesn't seem to be any item leaking,
according to valgrind. So I'm perfectly fine by just freeing the list
heads.

Bruno

> 
> > 
> > struct compute_memory_item *item, *next;
> > struct pipe_screen *screen = (struct pipe_screen *)pool->screen;
> > struct pipe_resource *res;
> > 
> > if (!LIST_IS_EMPTY(pool->item_list)) {
> >     LIST_FOR_EACH_ENTRY_SAFE(item, next, pool->item_list, link) {
> >         if (item->real_buffer) {
> >             res = (struct pipe_resource *)item->real_buffer;
> >             pool->screen->b.b.resource_destroy(screen, res);
> >         }
> > 
> >         free(item);
> >     }
> > }
> > /* And the same for the unallocated_list */
> > 
> > Note: I haven't tested it, but I think that it should work.
> > 
> > Hope it helps!
> > Bruno
> > 
> > > +	free(pool->item_list);
> > > +	free(pool->unallocated_list);
> > > +	/* And then the pool itself */
> > >  	free(pool);
> > >  }
> > >  
> > 
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>