[Mesa-dev] [PATCH 1/3] panfrost: Fix various leaks unmapping resources

[Emil Velikov]

Hi Alyssa,

On Fri, 15 Feb 2019 at 08:50, Alyssa Rosenzweig <alyssa at rosenzweig.io> wrote:
>
> Signed-off-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
> ---
>  src/gallium/drivers/panfrost/pan_resource.c | 22 ++++++++++++---------
>  src/gallium/drivers/panfrost/pan_screen.h   |  4 +++-
>  2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
> index 7fa00117a28..fb9b8e63c83 100644
> --- a/src/gallium/drivers/panfrost/pan_resource.c
> +++ b/src/gallium/drivers/panfrost/pan_resource.c
> @@ -287,17 +287,20 @@ panfrost_destroy_bo(struct panfrost_screen *screen, struct panfrost_bo *pbo)
>  {
>         struct panfrost_bo *bo = (struct panfrost_bo *)pbo;
>
> -        if (bo->entry[0] != NULL) {
> -                /* Most allocations have an entry to free */
> -                bo->entry[0]->freed = true;
> -                pb_slab_free(&screen->slabs, &bo->entry[0]->base);
> +        for (int l = 0; l < MAX_MIP_LEVELS; ++l) {
> +                if (bo->entry[l] != NULL) {
Nit: staying consistent with "foo != NULL" vs "foo" checks helps a lot.

> +                        /* Most allocations have an entry to free */
> +                        bo->entry[l]->freed = true;
> +                        pb_slab_free(&screen->slabs, &bo->entry[l]->base);
> +                }
>          }
>
>          if (bo->tiled) {
>                  /* Tiled has a malloc'd CPU, so just plain ol' free needed */
>
> -                for (int l = 0; bo->cpu[l]; l++) {
> -                        free(bo->cpu[l]);
> +                for (int l = 0; l < MAX_MIP_LEVELS; ++l) {
> +                        if (bo->cpu[l])
free(NULL); is perfectly valid.

> +                                free(bo->cpu[l]);
>                  }
>          }
>
> @@ -509,9 +512,10 @@ panfrost_slab_can_reclaim(void *priv, struct pb_slab_entry *entry)
>  static void
>  panfrost_slab_free(void *priv, struct pb_slab *slab)
>  {
> -        /* STUB */
> -        //struct panfrost_memory *mem = (struct panfrost_memory *) slab;
> -        printf("stub: Tried to free slab\n");
> +        struct panfrost_memory *mem = (struct panfrost_memory *) slab;
> +        struct panfrost_screen *screen = (struct panfrost_screen *) priv;
> +
> +        screen->driver->free_slab(screen, mem);
The function pointer seems to be NULL. Did you forget to git add the
file which sets it?

HTH
-Emil