Re: [Mesa-dev] [PATCH] i965: unref push_const_bo in intelDestroyContext

[Tapani Pälli]

On 10/27/2017 12:57 PM, Kenneth Graunke wrote:

On Friday, October 27, 2017 2:08:36 AM PDT Emil Velikov wrote:

On 27 October 2017 at 07:52, Tapani Pälli  wrote:

Valgrind shows that leak is caused by gen6_upload_push_constant, add
unref push_const_bo per stage to destructor to fix this (like done for
scratch_bo).

==10952== 144 bytes in 1 blocks are definitely lost in loss record 44 of 66
==10952==at 0x4C30A1E: calloc (vg_replace_malloc.c:711)
==10952==by 0x8C02847: bo_alloc_internal.constprop.10 (brw_bufmgr.c:344)
==10952==by 0x8C425C4: intel_upload_space (intel_upload.c:101)
==10952==by 0x8C22ED0: gen6_upload_push_constants 
(gen6_constant_state.c:154)

Fixes: 24891d7c05 ("i965: Store per-stage push constant BO pointers.")
Signed-off-by: Tapani Pälli 
Cc: mesa-sta...@lists.freedesktop.org
---
  src/mesa/drivers/dri/i965/brw_context.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index c8de074638..61088e2f1f 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -1072,6 +1072,17 @@ intelDestroyContext(__DRIcontext * driContextPriv)
 if (brw->wm.base.scratch_bo)
brw_bo_unreference(brw->wm.base.scratch_bo);

+   if (brw->vs.base.push_const_bo)

I'd drop the if checks - brw_bo_unreference works fine when the bo
pointer is NULL.

With that the patch is
Reviewed-by: Emil Velikov 

-Emil


Likewise, with the ifs gone,
Reviewed-by: Kenneth Graunke 

Thanks for fixing my mistake...sorry for the leaks!


No problem, I sent separate patch also to remove the if's from 
scratch_bo unrefs.


// Tapani
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: unref push_const_bo in intelDestroyContext

[Kenneth Graunke]

On Friday, October 27, 2017 2:08:36 AM PDT Emil Velikov wrote:
> On 27 October 2017 at 07:52, Tapani Pälli  wrote:
> > Valgrind shows that leak is caused by gen6_upload_push_constant, add
> > unref push_const_bo per stage to destructor to fix this (like done for
> > scratch_bo).
> >
> >==10952== 144 bytes in 1 blocks are definitely lost in loss record 44 of 
> > 66
> >==10952==at 0x4C30A1E: calloc (vg_replace_malloc.c:711)
> >==10952==by 0x8C02847: bo_alloc_internal.constprop.10 
> > (brw_bufmgr.c:344)
> >==10952==by 0x8C425C4: intel_upload_space (intel_upload.c:101)
> >==10952==by 0x8C22ED0: gen6_upload_push_constants 
> > (gen6_constant_state.c:154)
> >
> > Fixes: 24891d7c05 ("i965: Store per-stage push constant BO pointers.")
> > Signed-off-by: Tapani Pälli 
> > Cc: mesa-sta...@lists.freedesktop.org
> > ---
> >  src/mesa/drivers/dri/i965/brw_context.c | 11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
> > b/src/mesa/drivers/dri/i965/brw_context.c
> > index c8de074638..61088e2f1f 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > @@ -1072,6 +1072,17 @@ intelDestroyContext(__DRIcontext * driContextPriv)
> > if (brw->wm.base.scratch_bo)
> >brw_bo_unreference(brw->wm.base.scratch_bo);
> >
> > +   if (brw->vs.base.push_const_bo)
> I'd drop the if checks - brw_bo_unreference works fine when the bo
> pointer is NULL.
> 
> With that the patch is
> Reviewed-by: Emil Velikov 
> 
> -Emil

Likewise, with the ifs gone,
Reviewed-by: Kenneth Graunke 

Thanks for fixing my mistake...sorry for the leaks!


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] i965: unref push_const_bo in intelDestroyContext

[Emil Velikov]

On 27 October 2017 at 07:52, Tapani Pälli  wrote:
> Valgrind shows that leak is caused by gen6_upload_push_constant, add
> unref push_const_bo per stage to destructor to fix this (like done for
> scratch_bo).
>
>==10952== 144 bytes in 1 blocks are definitely lost in loss record 44 of 66
>==10952==at 0x4C30A1E: calloc (vg_replace_malloc.c:711)
>==10952==by 0x8C02847: bo_alloc_internal.constprop.10 
> (brw_bufmgr.c:344)
>==10952==by 0x8C425C4: intel_upload_space (intel_upload.c:101)
>==10952==by 0x8C22ED0: gen6_upload_push_constants 
> (gen6_constant_state.c:154)
>
> Fixes: 24891d7c05 ("i965: Store per-stage push constant BO pointers.")
> Signed-off-by: Tapani Pälli 
> Cc: mesa-sta...@lists.freedesktop.org
> ---
>  src/mesa/drivers/dri/i965/brw_context.c | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
> b/src/mesa/drivers/dri/i965/brw_context.c
> index c8de074638..61088e2f1f 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1072,6 +1072,17 @@ intelDestroyContext(__DRIcontext * driContextPriv)
> if (brw->wm.base.scratch_bo)
>brw_bo_unreference(brw->wm.base.scratch_bo);
>
> +   if (brw->vs.base.push_const_bo)
I'd drop the if checks - brw_bo_unreference works fine when the bo
pointer is NULL.

With that the patch is
Reviewed-by: Emil Velikov 

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev