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

[Bruno Jimenez]

On Mon, 2014-08-25 at 09:35 -0500, Aaron Watry wrote:
> On Sat, Aug 23, 2014 at 10:01 AM, Bruno Jimenez <brunojimen at gmail.com> 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):
> >
> 
> I'm not actually worried about these items having not been freed... At
> least up to this point, I
> haven't seen any pool items leaking in valgrind, just the list heads themselves.
> 
> If you want, I can add this type of foreach cleanup, but I'm content
> to leave it as is until it becomes
> an issue...  Your choice.
> 
> --Aaron

Hi,

If Valgrind says that there doesn't seem to be any leak, then I'm fine
with the patch as is.

For this patch, with the comment changes suggested by Michel.
Reviewed-by: Bruno Jiménez <brunojimen at gmail.com>

> 
> 
> > 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);
> >>  }
> >>
> >
> >