[Mesa-dev] i965: free scratch buffers when destroying the context

Submitted by Iago Toral Quiroga on March 6, 2015, 8:26 a.m.

Details

Message ID 1425630395-2206-1-git-send-email-itoral@igalia.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga March 6, 2015, 8:26 a.m.
If scratch space is needed for a shader stage we try to reuse the last scratch
buffer bound to that stage. If we can't, we free the old scratch buffer and
allocate a new one. This means we always keep the last scratch buffer for a
particular shader stage around for the entire life span of the context.

These buffers are being reported by Valgrind as definitely lost after
destroying the OpenGL context. For example, for the geometry shader stage:

==18350== 248 bytes in 1 blocks are definitely lost in loss record 85 of 150
==18350==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==18350==    by 0xA1B35D6: drm_intel_gem_bo_alloc_internal (intel_bufmgr_gem.c:724)
==18350==    by 0xA1B383F: drm_intel_gem_bo_alloc (intel_bufmgr_gem.c:794)
==18350==    by 0xA1AEFA3: drm_intel_bo_alloc (intel_bufmgr.c:52)
==18350==    by 0x9D08E31: brw_get_scratch_bo (brw_program.c:226)
==18350==    by 0x9D2A0F2: do_gs_prog (brw_vec4_gs.c:280)
==18350==    by 0x9D2A635: brw_gs_precompile (brw_vec4_gs.c:401)
==18350==    by 0x9D14F68: brw_shader_precompile(gl_context*, gl_shader_program*) (brw_shader.cpp:76)
==18350==    by 0x9D157B8: brw_link_shader (brw_shader.cpp:269)
==18350==    by 0x9B0941E: _mesa_glsl_link_shader (ir_to_mesa.cpp:3038)
==18350==    by 0x99AE4ED: link_program (shaderapi.c:917)
==18350==    by 0x99AF365: _mesa_LinkProgram (shaderapi.c:1385)

So make sure that by the time we destroy the context we check if we have live
scratch buffers for the various stages and release them if that is the case.
---
I sent this some months ago but I think it was never reviewed. The problems
still exists and it is easy to spot by using geometry shaders in SNB, which
always require scratch space due to the use of a register array to buffer
vertex data.

 src/mesa/drivers/dri/i965/brw_context.c | 6 ++++++
 1 file changed, 6 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index 786e6f5..972e458 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -908,6 +908,12 @@  intelDestroyContext(__DRIcontext * driContextPriv)
    brw_draw_destroy(brw);
 
    drm_intel_bo_unreference(brw->curbe.curbe_bo);
+   if (brw->vs.base.scratch_bo)
+      drm_intel_bo_unreference(brw->vs.base.scratch_bo);
+   if (brw->gs.base.scratch_bo)
+      drm_intel_bo_unreference(brw->gs.base.scratch_bo);
+   if (brw->wm.base.scratch_bo)
+      drm_intel_bo_unreference(brw->wm.base.scratch_bo);
 
    drm_intel_gem_context_destroy(brw->hw_ctx);
 

Comments

Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>

On 2015-03-06 00:26:35, Iago Toral Quiroga wrote:
> If scratch space is needed for a shader stage we try to reuse the last scratch
> buffer bound to that stage. If we can't, we free the old scratch buffer and
> allocate a new one. This means we always keep the last scratch buffer for a
> particular shader stage around for the entire life span of the context.
> 
> These buffers are being reported by Valgrind as definitely lost after
> destroying the OpenGL context. For example, for the geometry shader stage:
> 
> ==18350== 248 bytes in 1 blocks are definitely lost in loss record 85 of 150
> ==18350==    at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==18350==    by 0xA1B35D6: drm_intel_gem_bo_alloc_internal (intel_bufmgr_gem.c:724)
> ==18350==    by 0xA1B383F: drm_intel_gem_bo_alloc (intel_bufmgr_gem.c:794)
> ==18350==    by 0xA1AEFA3: drm_intel_bo_alloc (intel_bufmgr.c:52)
> ==18350==    by 0x9D08E31: brw_get_scratch_bo (brw_program.c:226)
> ==18350==    by 0x9D2A0F2: do_gs_prog (brw_vec4_gs.c:280)
> ==18350==    by 0x9D2A635: brw_gs_precompile (brw_vec4_gs.c:401)
> ==18350==    by 0x9D14F68: brw_shader_precompile(gl_context*, gl_shader_program*) (brw_shader.cpp:76)
> ==18350==    by 0x9D157B8: brw_link_shader (brw_shader.cpp:269)
> ==18350==    by 0x9B0941E: _mesa_glsl_link_shader (ir_to_mesa.cpp:3038)
> ==18350==    by 0x99AE4ED: link_program (shaderapi.c:917)
> ==18350==    by 0x99AF365: _mesa_LinkProgram (shaderapi.c:1385)
> 
> So make sure that by the time we destroy the context we check if we have live
> scratch buffers for the various stages and release them if that is the case.
> ---
> I sent this some months ago but I think it was never reviewed. The problems
> still exists and it is easy to spot by using geometry shaders in SNB, which
> always require scratch space due to the use of a register array to buffer
> vertex data.
> 
>  src/mesa/drivers/dri/i965/brw_context.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 786e6f5..972e458 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -908,6 +908,12 @@ intelDestroyContext(__DRIcontext * driContextPriv)
>     brw_draw_destroy(brw);
>  
>     drm_intel_bo_unreference(brw->curbe.curbe_bo);
> +   if (brw->vs.base.scratch_bo)
> +      drm_intel_bo_unreference(brw->vs.base.scratch_bo);
> +   if (brw->gs.base.scratch_bo)
> +      drm_intel_bo_unreference(brw->gs.base.scratch_bo);
> +   if (brw->wm.base.scratch_bo)
> +      drm_intel_bo_unreference(brw->wm.base.scratch_bo);
>  
>     drm_intel_gem_context_destroy(brw->hw_ctx);
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev