[Mesa-dev] Mesa (master): draw: Only run prepare when state, prim and opt changes
Stéphane Marchesin
stephane.marchesin at gmail.com
Thu Jan 26 20:32:40 PST 2012
Ok so I'm thinking of adding a refcount to the variant to know if they
are bound, and not garbage collect them in that case. Let me know what
you think.
Stéphane
2012/1/26 Stéphane Marchesin <stephane.marchesin at gmail.com>:
> So actually it's a case of a use-after free. The variant is freed with
> draw_llvm_destroy_variant and then reused through
> llvm_pipeline_generic after free_gallium_state (and llvm) reused the
> memory for something else. What prevents a variant bound to an fpme
> from being freed by the garbage collection?
>
> Stéphane
>
>
> 2012/1/26 Stéphane Marchesin <stephane.marchesin at gmail.com>:
>> I just took a look at it in gdb. Basically the jit_func pointer is
>> corrupted by the free_gallivm_state function (in lp_bld_init.c). There
>> is a comment to that effect already. It seems like the bug was always
>> there but hidden because we regenerated state more than we had to.
>> I'll keep digging...
>>
>> Stéphane
>>
>>
>> 2012/1/26 Stéphane Marchesin <stephane.marchesin at gmail.com>:
>>> Hmm, I'll take a look later today.
>>>
>>> Stéphane
>>>
>>> 2012/1/26 Jose Fonseca <jfonseca at vmware.com>:
>>>> Stephane,
>>>>
>>>> This commit caused a segmentation fault on glean texSwizzle test + llvmpipe:
>>>>
>>>> $ gdb --args glean --run results --overwrite --quick --tests texSwizzle
>>>> (gdb) r
>>>> Starting program: glean --run results --overwrite --quick --tests texSwizzle
>>>> [Thread debugging using libthread_db enabled]
>>>>
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> 0xfffffffffffffffc in ?? ()
>>>> (gdb) bt
>>>> #0 0xfffffffffffffffc in ?? ()
>>>> #1 0x00007ffff6a26438 in llvm_pipeline_generic (middle=0x76e4a0, fetch_info=0x7fffffffd730, prim_info=0x7fffffffd700)
>>>> at src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:240
>>>> #2 0x00007ffff6a266fe in llvm_middle_end_linear_run (middle=0x76e4a0, start=0, count=4, prim_flags=0)
>>>> at src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:358
>>>> #3 0x00007ffff697bf23 in vsplit_segment_simple_linear (vsplit=0x76b670, flags=0, istart=0, icount=4) at src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h:237
>>>> #4 0x00007ffff697c228 in vsplit_run_linear (frontend=0x76b670, start=0, count=4) at src/gallium/auxiliary/draw/draw_split_tmp.h:61
>>>> #5 0x00007ffff697224e in draw_pt_arrays (draw=0x762510, prim=6, start=0, count=4) at src/gallium/auxiliary/draw/draw_pt.c:142
>>>> #6 0x00007ffff6972eb1 in draw_vbo (draw=0x762510, info=0x7fffffffd910) at src/gallium/auxiliary/draw/draw_pt.c:534
>>>> #7 0x00007ffff6689f67 in llvmpipe_draw_vbo (pipe=0x72fa10, info=0x7fffffffd910) at src/gallium/drivers/llvmpipe/lp_draw_arrays.c:85
>>>> #8 0x00007ffff68037f4 in st_draw_vbo (ctx=0x7c4b30, arrays=0x831c88, prims=0x7fffffffd9e0, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=0,
>>>> max_index=3, tfb_vertcount=0x0) at src/mesa/state_tracker/st_draw.c:1113
>>>> #9 0x00007ffff689d811 in vbo_draw_arrays (ctx=0x7c4b30, mode=6, start=0, count=4, numInstances=1) at src/mesa/vbo/vbo_exec_array.c:635
>>>> #10 0x00007ffff689d950 in vbo_exec_DrawArrays (mode=6, start=0, count=4) at src/mesa/vbo/vbo_exec_array.c:667
>>>> #11 0x0000000000458205 in GLEAN::TexSwizzleTest::TestSwizzles (this=0x6f10e0) at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/ttexswizzle.cpp:293
>>>> #12 0x0000000000458558 in GLEAN::TexSwizzleTest::runOne (this=0x6f10e0, r=..., w=<optimized out>)
>>>> at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/ttexswizzle.cpp:387
>>>> #13 0x0000000000458ec7 in GLEAN::BaseTest<GLEAN::TexSwizzleResult>::run (this=0x6f10e0, environment=<optimized out>)
>>>> at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/tbase.h:317
>>>> #14 0x00000000004610b8 in main (argc=7, argv=0x7fffffffdec8) at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/main.cpp:140
>>>> (gdb)
>>>>
>>>> Can you look into it?
>>>>
>>>> Jose
>>>>
>>>>
>>>> ----- Original Message -----
>>>>> Module: Mesa
>>>>> Branch: master
>>>>> Commit: b6d3a435a0e0e53a9e8cc4c4249dc7c2f897a83d
>>>>> URL:
>>>>> http://cgit.freedesktop.org/mesa/mesa/commit/?id=b6d3a435a0e0e53a9e8cc4c4249dc7c2f897a83d
>>>>>
>>>>> Author: Jakob Bornecrantz <wallbraker at gmail.com>
>>>>> Date: Mon Jan 24 02:11:59 2011 +0100
>>>>>
>>>>> draw: Only run prepare when state, prim and opt changes
>>>>>
>>>>> In bad applications like ipers which does a lot of draw calls with
>>>>> no state changes this helps to greatly reduce time spent in prepare.
>>>>> In ipers around 7% of CPU was spent in various prepare functions,
>>>>> after this commit no prepare function show on the profile.
>>>>>
>>>>> This commit also has the added benefit of now grouping all pipelined
>>>>> drawing into a single draw call if the driver uses vbuf_render.
>>>>>
>>>>> Reviewed-by: Stéphane Marchesin <marcheu at chromium.org>
>>>>> Tested-by: Stéphane Marchesin <marcheu at chromium.org>
>>>>>
>>>>> ---
>>>>>
>>>>> src/gallium/auxiliary/draw/draw_context.c | 6 +++
>>>>> src/gallium/auxiliary/draw/draw_private.h | 8 ++++
>>>>> src/gallium/auxiliary/draw/draw_pt.c | 49
>>>>> ++++++++++++++++++++++++---
>>>>> src/gallium/auxiliary/draw/draw_pt.h | 2 +-
>>>>> src/gallium/auxiliary/draw/draw_pt_vsplit.c | 11 ++++--
>>>>> 5 files changed, 66 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/src/gallium/auxiliary/draw/draw_context.c
>>>>> b/src/gallium/auxiliary/draw/draw_context.c
>>>>> index 4ce4445..3c0b1aa 100644
>>>>> --- a/src/gallium/auxiliary/draw/draw_context.c
>>>>> +++ b/src/gallium/auxiliary/draw/draw_context.c
>>>>> @@ -355,6 +355,10 @@ draw_set_vertex_elements(struct draw_context
>>>>> *draw,
>>>>> {
>>>>> assert(count <= PIPE_MAX_ATTRIBS);
>>>>>
>>>>> + /* We could improve this by only flushing the frontend and the
>>>>> fetch part
>>>>> + * of the middle. This would avoid recalculating the emit keys.*/
>>>>> + draw_do_flush( draw, DRAW_FLUSH_STATE_CHANGE );
>>>>> +
>>>>> memcpy(draw->pt.vertex_element, elements, count *
>>>>> sizeof(elements[0]));
>>>>> draw->pt.nr_vertex_elements = count;
>>>>> }
>>>>> @@ -654,6 +658,8 @@ void draw_do_flush( struct draw_context *draw,
>>>>> unsigned flags )
>>>>>
>>>>> draw_pipeline_flush( draw, flags );
>>>>>
>>>>> + draw_pt_flush( draw, flags );
>>>>> +
>>>>> draw->flushing = FALSE;
>>>>> }
>>>>> }
>>>>> diff --git a/src/gallium/auxiliary/draw/draw_private.h
>>>>> b/src/gallium/auxiliary/draw/draw_private.h
>>>>> index 1a0286d..c3eca97 100644
>>>>> --- a/src/gallium/auxiliary/draw/draw_private.h
>>>>> +++ b/src/gallium/auxiliary/draw/draw_private.h
>>>>> @@ -63,6 +63,7 @@ struct draw_stage;
>>>>> struct vbuf_render;
>>>>> struct tgsi_exec_machine;
>>>>> struct tgsi_sampler;
>>>>> +struct draw_pt_front_end;
>>>>>
>>>>>
>>>>> /**
>>>>> @@ -137,6 +138,12 @@ struct draw_context
>>>>> /* Support prototype passthrough path:
>>>>> */
>>>>> struct {
>>>>> + /* Current active frontend */
>>>>> + struct draw_pt_front_end *frontend;
>>>>> + unsigned prim;
>>>>> + unsigned opt;
>>>>> + unsigned eltSize; /* saved eltSize for flushing */
>>>>> +
>>>>> struct {
>>>>> struct draw_pt_middle_end *fetch_emit;
>>>>> struct draw_pt_middle_end *fetch_shade_emit;
>>>>> @@ -391,6 +398,7 @@ void draw_remove_extra_vertex_attribs(struct
>>>>> draw_context *draw);
>>>>> boolean draw_pt_init( struct draw_context *draw );
>>>>> void draw_pt_destroy( struct draw_context *draw );
>>>>> void draw_pt_reset_vertex_ids( struct draw_context *draw );
>>>>> +void draw_pt_flush( struct draw_context *draw, unsigned flags );
>>>>>
>>>>>
>>>>> /*******************************************************************************
>>>>> diff --git a/src/gallium/auxiliary/draw/draw_pt.c
>>>>> b/src/gallium/auxiliary/draw/draw_pt.c
>>>>> index 9a017fd..025d539 100644
>>>>> --- a/src/gallium/auxiliary/draw/draw_pt.c
>>>>> +++ b/src/gallium/auxiliary/draw/draw_pt.c
>>>>> @@ -52,7 +52,7 @@ DEBUG_GET_ONCE_BOOL_OPTION(draw_no_fse,
>>>>> "DRAW_NO_FSE", FALSE)
>>>>> * - backend -- the vbuf_render provided by the driver.
>>>>> */
>>>>> static boolean
>>>>> -draw_pt_arrays(struct draw_context *draw,
>>>>> +draw_pt_arrays(struct draw_context *draw,
>>>>> unsigned prim,
>>>>> unsigned start,
>>>>> unsigned count)
>>>>> @@ -106,17 +106,56 @@ draw_pt_arrays(struct draw_context *draw,
>>>>> middle = draw->pt.middle.general;
>>>>> }
>>>>>
>>>>> - frontend = draw->pt.front.vsplit;
>>>>> + frontend = draw->pt.frontend;
>>>>> +
>>>>> + if (frontend ) {
>>>>> + if (draw->pt.prim != prim || draw->pt.opt != opt) {
>>>>> + /* In certain conditions switching primitives requires us
>>>>> to flush
>>>>> + * and validate the different stages. One example is when
>>>>> smooth
>>>>> + * lines are active but first drawn with triangles and then
>>>>> with
>>>>> + * lines.
>>>>> + */
>>>>> + draw_do_flush( draw, DRAW_FLUSH_STATE_CHANGE );
>>>>> + frontend = NULL;
>>>>> + } else if (draw->pt.eltSize != draw->pt.user.eltSize) {
>>>>> + /* Flush draw state if eltSize changed.
>>>>> + * This could be improved so only the frontend is flushed
>>>>> since it
>>>>> + * converts all indices to ushorts and the fetch part of
>>>>> the middle
>>>>> + * always perpares both linear and indexed.
>>>>> + */
>>>>> + frontend->flush( frontend, DRAW_FLUSH_STATE_CHANGE );
>>>>> + frontend = NULL;
>>>>> + }
>>>>> + }
>>>>>
>>>>> - frontend->prepare( frontend, prim, middle, opt );
>>>>> + if (!frontend) {
>>>>> + frontend = draw->pt.front.vsplit;
>>>>>
>>>>> - frontend->run(frontend, start, count);
>>>>> + frontend->prepare( frontend, prim, middle, opt );
>>>>>
>>>>> - frontend->finish( frontend );
>>>>> + draw->pt.frontend = frontend;
>>>>> + draw->pt.eltSize = draw->pt.user.eltSize;
>>>>> + draw->pt.prim = prim;
>>>>> + draw->pt.opt = opt;
>>>>> + }
>>>>> +
>>>>> + frontend->run( frontend, start, count );
>>>>>
>>>>> return TRUE;
>>>>> }
>>>>>
>>>>> +void draw_pt_flush( struct draw_context *draw, unsigned flags )
>>>>> +{
>>>>> + if (draw->pt.frontend) {
>>>>> + draw->pt.frontend->flush( draw->pt.frontend, flags );
>>>>> +
>>>>> + /* don't prepare if we only are flushing the backend */
>>>>> + if (!(flags & DRAW_FLUSH_BACKEND))
>>>>> + draw->pt.frontend = NULL;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +
>>>>>
>>>>> boolean draw_pt_init( struct draw_context *draw )
>>>>> {
>>>>> diff --git a/src/gallium/auxiliary/draw/draw_pt.h
>>>>> b/src/gallium/auxiliary/draw/draw_pt.h
>>>>> index 9a45845..2c2efdc 100644
>>>>> --- a/src/gallium/auxiliary/draw/draw_pt.h
>>>>> +++ b/src/gallium/auxiliary/draw/draw_pt.h
>>>>> @@ -73,7 +73,7 @@ struct draw_pt_front_end {
>>>>> unsigned start,
>>>>> unsigned count );
>>>>>
>>>>> - void (*finish)( struct draw_pt_front_end * );
>>>>> + void (*flush)( struct draw_pt_front_end *, unsigned flags );
>>>>> void (*destroy)( struct draw_pt_front_end * );
>>>>> };
>>>>>
>>>>> diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>>>> b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>>>> index c19dcd9..0fed057 100644
>>>>> --- a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>>>> +++ b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>>>> @@ -178,11 +178,14 @@ static void vsplit_prepare(struct
>>>>> draw_pt_front_end *frontend,
>>>>> }
>>>>>
>>>>>
>>>>> -static void vsplit_finish(struct draw_pt_front_end *frontend)
>>>>> +static void vsplit_flush(struct draw_pt_front_end *frontend,
>>>>> unsigned flags)
>>>>> {
>>>>> struct vsplit_frontend *vsplit = (struct vsplit_frontend *)
>>>>> frontend;
>>>>> - vsplit->middle->finish(vsplit->middle);
>>>>> - vsplit->middle = NULL;
>>>>> +
>>>>> + if (!(flags & DRAW_FLUSH_BACKEND)) {
>>>>> + vsplit->middle->finish(vsplit->middle);
>>>>> + vsplit->middle = NULL;
>>>>> + }
>>>>> }
>>>>>
>>>>>
>>>>> @@ -202,7 +205,7 @@ struct draw_pt_front_end *draw_pt_vsplit(struct
>>>>> draw_context *draw)
>>>>>
>>>>> vsplit->base.prepare = vsplit_prepare;
>>>>> vsplit->base.run = NULL;
>>>>> - vsplit->base.finish = vsplit_finish;
>>>>> + vsplit->base.flush = vsplit_flush;
>>>>> vsplit->base.destroy = vsplit_destroy;
>>>>> vsplit->draw = draw;
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> mesa-commit mailing list
>>>>> mesa-commit at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/mesa-commit
>>>>>
More information about the mesa-dev
mailing list