[Mesa-dev] [PATCH] llvmpipe: Always return some fence in flush (v2)

Thu Jan 10 23:36:16 UTC 2019

Am 10.01.19 um 04:51 schrieb Tomasz Figa:
> Hi Roland,
> 
> On Thu, Jan 10, 2019 at 1:36 AM Roland Scheidegger <sroland at vmware.com> wrote:
>>
>> Sorry but I had to revert this, as we've seen lots of assertion failures
>> (src/gallium/drivers/llvmpipe/lp_fence.c:120: lp_fence_wait: Assertion
>> `f->issued' failed.). For instance with libgl_xlib state tracker and piglit.
>> I'm not entirely sure if it's really safe to just remove the assert or
> 
> Understood. Sorry for not spotting this in my testing.
No problem...

> Would you be
> able to help with more details on how to reproduce these failures?

I'm not sure if this reproduces with dri, so build libgl-xlib (I just
use scons, but it shouldn't matter, as long as you get asserts enabled...).
Set LD_LIBRARY_PATH to the built libGL, and run any of the affected
piglit tests (automated testing here got about 3500 crashes) - picking
just the first piglit/bin/arb_vertex_array_bgra-get -auto
arb_vertex_array_bgra-get: src/gallium/drivers/llvmpipe/lp_fence.c:120:
lp_fence_wait: Assertion `f->issued' failed.

It actually seems to mostly affect tests which don't render anything
(and they might actually crash after reporting the test passed).

FWIW this ist the stack trace:
#0  __GI_raise (sig=sig at entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007ffff7433801 in __GI_abort () at abort.c:79
#2  0x00007ffff742339a in __assert_fail_base (fmt=0x7ffff75aa7d8
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=assertion at entry=0x7ffff56f9fa9 "f->issued",
file=file at entry=0x7ffff56f9f50
"src/gallium/drivers/llvmpipe/lp_fence.c", line=line at entry=120,
    function=function at entry=0x7ffff56fa090 <__PRETTY_FUNCTION__.11755>
"lp_fence_wait") at assert.c:92
#3  0x00007ffff7423412 in __GI___assert_fail (assertion=0x7ffff56f9fa9
"f->issued", file=0x7ffff56f9f50
"src/gallium/drivers/llvmpipe/lp_fence.c", line=120,
function=0x7ffff56fa090 <__PRETTY_FUNCTION__.11755> "lp_fence_wait") at
assert.c:101
#4  0x00007ffff3eae07a in lp_fence_wait (f=0x5555559b1a10) at
src/gallium/drivers/llvmpipe/lp_fence.c:120
#5  0x00007ffff3eaaa9b in llvmpipe_fence_finish (screen=0x555555798170,
ctx=0x0, fence_handle=0x5555559b1a10, timeout=18446744073709551615) at
src/gallium/drivers/llvmpipe/lp_screen.c:637
#6  0x00007ffff3f22eaf in XMesaFlush (c=0x5555557c4db0) at
src/gallium/state_trackers/glx/xlib/xm_api.c:1402
#7  0x00007ffff3f22b80 in XMesaMakeCurrent2 (c=0x0, drawBuffer=0x0,
readBuffer=0x0) at src/gallium/state_trackers/glx/xlib/xm_api.c:1300
#8  0x00007ffff3f260f1 in glXMakeContextCurrent (dpy=0x555555781e60,
draw=0, read=0, ctx=0x0) at
src/gallium/state_trackers/glx/xlib/glx_api.c:1262
#9  0x00007ffff3f2616c in glXMakeCurrent (dpy=0x555555781e60,
drawable=0, ctx=0x0) at src/gallium/state_trackers/glx/xlib/glx_api.c:1283
#10 0x00007ffff6b81a70 in ?? () from
/usr/lib/x86_64-linux-gnu/libwaffle-1.so.0
#11 0x00007ffff6b7e01c in waffle_make_current () from
/usr/lib/x86_64-linux-gnu/libwaffle-1.so.0
#12 0x00007ffff7b30829 in piglit_wfl_framework_teardown
(wfl_fw=0x5555557817a0) at
/home/sroland/devel/piglit/tests/util/piglit-framework-gl/piglit_wfl_framework.c:636
#13 0x00007ffff7b30f05 in piglit_winsys_framework_teardown
(winsys_fw=0x5555557817a0) at
/home/sroland/devel/piglit/tests/util/piglit-framework-gl/piglit_winsys_framework.c:238
#14 0x00007ffff7b31e1b in destroy (gl_fw=0x5555557817a0) at
/home/sroland/devel/piglit/tests/util/piglit-framework-gl/piglit_x11_framework.c:212
#15 0x00007ffff7b14b6e in destroy () at
/home/sroland/devel/piglit/tests/util/piglit-framework-gl.c:210
#16 0x00007ffff7436041 in __run_exit_handlers (status=0,
listp=0x7ffff77de718 <__exit_funcs>,
run_list_atexit=run_list_atexit at entry=true,
run_dtors=run_dtors at entry=true) at exit.c:108
#17 0x00007ffff743613a in __GI_exit (status=<optimized out>) at exit.c:139
#18 0x00007ffff77e66b9 in piglit_report_result (result=PIGLIT_PASS) at
/home/sroland/devel/piglit/tests/util/piglit-util.c:241
#19 0x0000555555554fca in piglit_init (argc=1, argv=0x7fffffffe338) at
/home/sroland/devel/piglit/tests/spec/arb_vertex_array_bgra/get.c:74
#20 0x00007ffff7b30975 in run_test (gl_fw=0x5555557817a0, argc=1,
argv=0x7fffffffe338) at
/home/sroland/devel/piglit/tests/util/piglit-framework-gl/piglit_winsys_framework.c:73
#21 0x00007ffff7b14c22 in piglit_gl_test_run (argc=1,
argv=0x7fffffffe338, config=0x7fffffffe1f0) at
/home/sroland/devel/piglit/tests/util/piglit-framework-gl.c:229
#22 0x0000555555554e20 in main (argc=1, argv=0x7fffffffe338) at
/home/sroland/devel/piglit/tests/spec/arb_vertex_array_bgra/get.c:38

> 
>> if it needs some more work, and I don't have time right now for a
>> thorough investigation, but I'll happily take new patches...
> 
> Perhaps we could make these dummy fences "issued". I'll check how this
> works in the code.
> 
> Best regards,
> Tomasz
> 
>>
>> Roland
>>
>>
>> Am 09.01.19 um 02:09 schrieb Roland Scheidegger:
>>> Am 07.01.19 um 09:54 schrieb Tomasz Figa:
>>>> On Sun, Dec 23, 2018 at 12:55 AM Roland Scheidegger <sroland at vmware.com> wrote:
>>>>>
>>>>> Alright, I guess it should work...
>>>>>
>>>>> Reviewed-by: Roland Scheidegger <sroland at vmware.com>
>>>>>
>>>>
>>>> Thanks!
>>>>
>>>> Would we have anyone who could help to commit it?
>>>
>>> Pushed (albeit I forgot the R-b on it, ah well...)
>>>
>>> Roland
>>>
>>>>
>>>> (I know that I was supposed to apply for commit rights, but I expect
>>>> my contribution rate to be relatively low, due to a shift to different
>>>> areas, so I don't think I'm a good candidate for a committer anymore.)
>>>>
>>>> Best regards,
>>>> Tomasz
>>>>
>>>>>
>>>>> Am 14.12.18 um 09:17 schrieb Tomasz Figa:
>>>>>> If there is no last fence, due to no rendering happening yet, just
>>>>>> create a new signaled fence and return it, to match the expectations of
>>>>>> the EGL sync fence API.
>>>>>>
>>>>>> Fixes random "Could not create sync fence 0x3003" assertion failures from
>>>>>> Skia on Android, coming from the following code:
>>>>>>
>>>>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fandroid.googlesource.com%2Fplatform%2Fframeworks%2Fbase%2F%2B%2Fmaster%2Flibs%2Fhwui%2Fpipeline%2Fskia%2FSkiaOpenGLPipeline.cpp%23427&data=02%7C01%7Csroland%40vmware.com%7C8ff9d362ee064528b60408d676aef1ee%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636826891088770663&sdata=lE1Puv2b4DdAOZ%2FLtHPyu40KxLMI5wZTA%2FugAhr5n4I%3D&reserved=0
>>>>>>
>>>>>> Reproducible especially with thread count >= 4.
>>>>>>
>>>>>> One could make the driver always keep the reference to the last fence,
>>>>>> but:
>>>>>>
>>>>>>  - the driver seems to explicitly destroy the fence whenever a rendering
>>>>>>    pass completes and changing that would require a significant functional
>>>>>>    change to the code. (Specifically, in lp_scene_end_rasterization().)
>>>>>>
>>>>>>  - it still wouldn't solve the problem of an EGL sync fence being created
>>>>>>    and waited on without any rendering happening at all, which is
>>>>>>    also likely to happen with Android code pointed to in the commit.
>>>>>>
>>>>>> Therefore, the simple approach of always creating a fence is taken,
>>>>>> similarly to other drivers, such as radeonsi.
>>>>>>
>>>>>> Tested with piglit llvmpipe suite with no regressions and following
>>>>>> tests fixed:
>>>>>>
>>>>>> egl_khr_fence_sync
>>>>>>  conformance
>>>>>>   eglclientwaitsynckhr_flag_sync_flush
>>>>>>   eglclientwaitsynckhr_nonzero_timeout
>>>>>>   eglclientwaitsynckhr_zero_timeout
>>>>>>   eglcreatesynckhr_default_attributes
>>>>>>   eglgetsyncattribkhr_invalid_attrib
>>>>>>   eglgetsyncattribkhr_sync_status
>>>>>>
>>>>>> v2:
>>>>>>  - remove the useless lp_fence_reference() dance (Nicolai),
>>>>>>  - explain why creating the dummy fence is the right approach.
>>>>>>
>>>>>> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
>>>>>> ---
>>>>>>  src/gallium/drivers/llvmpipe/lp_setup.c | 2 ++
>>>>>>  1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
>>>>>> index b087369473..e72e119c8a 100644
>>>>>> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
>>>>>> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
>>>>>> @@ -361,6 +361,8 @@ lp_setup_flush( struct lp_setup_context *setup,
>>>>>>
>>>>>>     if (fence) {
>>>>>>        lp_fence_reference((struct lp_fence **)fence, setup->last_fence);
>>>>>> +      if (!*fence)
>>>>>> +         *fence = (struct pipe_fence_handle *)lp_fence_create(0);
>>>>>>     }
>>>>>>  }
>>>>>>
>>>>>>
>>>>>
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fmesa-dev&data=02%7C01%7Csroland%40vmware.com%7C8ff9d362ee064528b60408d676aef1ee%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C636826891088770663&sdata=8WM9BnA%2F51WgJ%2F%2FeYvWpQJVaXWhCQ88D61CuHcM1TL8%3D&reserved=0
>>>
>>