Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Roman Gilg]

On Sat, May 5, 2018 at 8:50 PM, Mario Kleiner
 wrote:
> Thanks. Can you see if you get any freezes in kwin_x11 by "violent
> alt-tabbing" with patch 1? I've seen two such freezes within 8 hours
> of normal use yesterday, each occuring when i alt-tabbed (normally)
> and the switcher side panel moved out from the left. Desktop was
> mostly blocked then, i assume it was kwin_x11 that got stuck. When
> VT-switching to the console and attaching gdb to kwin_x11 the
> backtrace showed it blocked in the loader_dri3_swapbuffer_barrier()
> from patch 1. I don't know if that was the cause of the freeze, or if
> something unrelated was going wrong and kwin just happens to block
> there when one VT switches away from the X-Server while kwin does its
> thing. Freezes happened both with compositing enabled and disabled.

Yes, I can confirm it happened to me as well now with patch 1.
Happened when holding Alt + Tab, but I had to try a few times. I
logged in directly with sddm and got the following backtrace without
VT switch by ssh session. So should be a clean one:

#0  0x7f4517e1074d in poll () at ../sysdeps/unix/syscall-template.S:84
#1  0x7f4516d32172 in poll (__timeout=-1, __nfds=1,
__fds=0x7ffed4e8ead0) at /usr/include/x86_64-linux-gnu/bits/poll2.h:46
#2  _xcb_conn_wait (c=c@entry=0xcc4be0, cond=cond@entry=0x19ef968,
vector=vector@entry=0x0, count=count@entry=0x0) at
/home/roman/dev/gfx/xcb/src/libxcb/src/xcb_conn.c:479
#3  0x7f4516d33e69 in xcb_wait_for_special_event (c=0xcc4be0,
se=0x19ef940) at /home/roman/dev/gfx/xcb/src/libxcb/src/xcb_in.c:795
#4  0x7f450d764d36 in dri3_wait_for_event_locked (draw=0x1a28288)
at ../../src/mesa/src/loader/loader_dri3_helper.c:457
#5  0x7f450d765568 in loader_dri3_wait_for_sbc
(draw=draw@entry=0x1a28288, target_sbc=47, target_sbc@entry=0,
ust=ust@entry=0x7ffed4e8eca0, msc=msc@entry=0x7ffed4e8eca8,
sbc=sbc@entry=0x7ffed4e8ecb0) at
../../src/mesa/src/loader/loader_dri3_helper.c:534
#6  0x7f450d76560b in loader_dri3_swapbuffer_barrier
(draw=0x1a28288) at
../../src/mesa/src/loader/loader_dri3_helper.c:1930
#7  loader_dri3_drawable_fini (draw=0x1a28288) at
../../src/mesa/src/loader/loader_dri3_helper.c:238
#8  0x7f450d75aaad in dri3_destroy_drawable (base=0x1a28250) at
../../src/mesa/src/glx/dri3_glx.c:349
#9  0x7f450d7537d2 in driReleaseDrawables (gc=gc@entry=0x1542920)
at ../../src/mesa/src/glx/dri_common.c:481
#10 0x7f450d75af0c in dri3_bind_context (context=0x1542920,
old=, draw=41946959, read=41946959) at
../../src/mesa/src/glx/dri3_glx.c:198
#11 0x7f450d746537 in MakeContextCurrent (dpy=0xcc3850,
draw=41946959, read=41946959, gc_user=0x1542920) at
../../src/mesa/src/glx/glxcurrent.c:213
#12 0x7f44fd3056fd in ?? () from
/usr/lib/x86_64-linux-gnu/qt5/plugins/xcbglintegrations/libqxcb-glx-integration.so
#13 0x7f45157d5229 in QOpenGLContext::makeCurrent(QSurface*) ()
from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#14 0x7f451024ea5e in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Quick.so.5
#15 0x7f451024faa5 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5Quick.so.5
#16 0x7f45157a5625 in QWindow::event(QEvent*) () from
/usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#17 0x7f45102c78c5 in QQuickWindow::event(QEvent*) () from
/usr/lib/x86_64-linux-gnu/libQt5Quick.so.5
#18 0x7f44d974565b in PlasmaQuick::Dialog::event(QEvent*) () from
/usr/lib/x86_64-linux-gnu/libKF5PlasmaQuick.so.5
#19 0x7f4515f40acc in QApplicationPrivate::notify_helper(QObject*,
QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#20 0x7f4515f48417 in QApplication::notify(QObject*, QEvent*) ()
from /usr/lib/x86_64-linux-gnu/libQt5Widgets.so.5
#21 0x7f45152003c8 in QCoreApplication::notifyInternal2(QObject*,
QEvent*) () from /usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#22 0x7f451579a28d in
QGuiApplicationPrivate::processExposeEvent(QWindowSystemInterfacePrivate::ExposeEvent*)
() from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#23 0x7f451579aebd in
QGuiApplicationPrivate::processWindowSystemEvent(QWindowSystemInterfacePrivate::WindowSystemEvent*)
() from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#24 0x7f45157748fb in
QWindowSystemInterface::sendWindowSystemEvents(QFlags)
() from /usr/lib/x86_64-linux-gnu/libQt5Gui.so.5
#25 0x7f44fec534b6 in ?? () from /usr/lib/x86_64-linux-gnu/libQt5XcbQpa.so.5
#26 0x7f45151fe64a in
QEventLoop::exec(QFlags) () from
/usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#27 0x7f4515207854 in QCoreApplication::exec() () from
/usr/lib/x86_64-linux-gnu/libQt5Core.so.5
#28 0x7f45180e81f9 in kdemain () from
/usr/lib/x86_64-linux-gnu/libkdeinit5_kwin_x11.so
#29 0x7f4517d35830 in __libc_start_main (main=0x4006b0, argc=1,
argv=0x7ffed4e8f868, init=, fini=,
rtld_fini=, stack_end=0x7ffed4e8f858) at
../csu/libc-start.c:291
#30 0x004006e9 in _start ()

> So far, only using patch 2/2, i haven't seen any new freezes, but
> looking at the debug output i get a lot of these 

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Mario Kleiner]

On Sat, May 5, 2018 at 4:44 PM, Roman Gilg  wrote:
> Without this patch plasmashell on Xserver/Mesa master freezes on me
> when opening the launcher menu (Kickoff). With the patch haven't
> experienced freezes yet.
>
> Haven't tested the Steam client yet. Might be a different problem though.
>
> Tested-by: Roman Gilg 
>

Thanks. Can you see if you get any freezes in kwin_x11 by "violent
alt-tabbing" with patch 1? I've seen two such freezes within 8 hours
of normal use yesterday, each occuring when i alt-tabbed (normally)
and the switcher side panel moved out from the left. Desktop was
mostly blocked then, i assume it was kwin_x11 that got stuck. When
VT-switching to the console and attaching gdb to kwin_x11 the
backtrace showed it blocked in the loader_dri3_swapbuffer_barrier()
from patch 1. I don't know if that was the cause of the freeze, or if
something unrelated was going wrong and kwin just happens to block
there when one VT switches away from the X-Server while kwin does its
thing. Freezes happened both with compositing enabled and disabled.

So far, only using patch 2/2, i haven't seen any new freezes, but
looking at the debug output i get a lot of these when alt-tabbing:

INIT: wxh = 264 x 1062, drawable 54662577 eid 54662586
QXcbConnection: XCB error: 3 (BadWindow), sequence: 13890, resource
id: 54662577, major code: 20 (GetProperty), minor code: 0
QXcbConnection: XCB error: 3 (BadWindow), sequence: 13902, resource
id: 54662577, major code: 20 (GetProperty), minor code: 0
FINI: wxh = 264 x 1062, drawable 54662577 eid 54662586 recv_sbc 3,
send_sbc 4 PENDING 1
INIT: wxh = 264 x 1062, drawable 54662633 eid 54662644
QXcbConnection: XCB error: 3 (BadWindow), sequence: 14502, resource
id: 54662633, major code: 20 (GetProperty), minor code: 0
QXcbConnection: XCB error: 3 (BadWindow), sequence: 14514, resource
id: 54662633, major code: 20 (GetProperty), minor code: 0
FINI: wxh = 264 x 1062, drawable 54662633 eid 54662644 recv_sbc 2,
send_sbc 3 PENDING 1

Curiously my display is only 1050 pixels high, that window looks higher.

Anyway, would be good to know if patch 1/1 replaces a high frequency
of lockups of plasmashell by a low frequency of lockups of kwin_x11 if
you notice something like that.

Ideally one of the more experienced Mesa developers would find a
better solution than one of these patches, at least long-term.

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

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Roman Gilg]

Without this patch plasmashell on Xserver/Mesa master freezes on me
when opening the launcher menu (Kickoff). With the patch haven't
experienced freezes yet.

Haven't tested the Steam client yet. Might be a different problem though.

Tested-by: Roman Gilg 

On Fri, May 4, 2018 at 3:45 PM, Mario Kleiner
 wrote:
> Before destroying the loader_dri3_drawable, make sure all pending
> swaps for it have completed. This guides against the following scenario,
> which happens, e.g., with KDE Plasma-5's plasmashell (which uses
> QT-5's QtGui/QtQuick for rendering), when it repaints multiple
> UI elements, each represented by its own Window/GLXDrawable, using
> one common GLXContext for all GLXDrawable's:
>
> 1. glXMakeCurrent(dpy, drawable1, context);
> 2. glXXX render to drawable1
> 3. glXSwapBuffers(dpy, drawable1); #1
> 4. glXMakeCurrent(dpy, drawable2, context);
> 5. glXXX render to drawable2
> 6. glXSwapBuffers(dpy, drawable2);
> // While the swap #1 is still pending for drawable1:
> 7. glXMakeCurrent(dpy, drawable1, context);
> 8. glXXX render to drawable1
> 9. glXSwapBuffers(dpy, drawable1);
>
> Binding a different drawable2 to the same context via glXMakeCurrent
> will cause its previous drawable1 to be released (cfe. dri3_bind_context
> -> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
> This unselects for Present notify event delivery on the associated
> X-Window and loses all dri3 related state. If drawable1 is selected for
> the context again [7], a new incarnation of loader_dri3_drawable is
> created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
> loader_dri3_drawable_init(), which again selects for Present notify
> event delivery for the underlying X-Window, but the new incarnation lost
> all state wrt. to previous rendering and swaps. The server now delivers
> PresentPixmapIdle and PresentPixmapComplete events from the completed
> previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
> doesn't expect those. One problem is that the new incarnation has a
> draw->send_sbc == 0, but now receives PresentPixmapComplete events with
> sbc's > 0, therefore updating draw->recv_sbc to > 0 in
> dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
> misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
> and ends up with a very large draw->recv_sbc. During the next swapbuffers
> call [9], the totally wrong recv_sbc is used for calculating the target_msc
> for the PresentPixmap request, leading to a target_msc billions of vblanks
> in the future, leading to a swap that never completes and thereby frozen UI
> and hang of the client.
>
> Make sure that a loader_dri3_drawable can only be destroyed after all
> its pending swaps have completed, to prevent misdelivery of PresentNotify
> events to the right X-Window, but the wrong incarnation of the associated
> loader_dri3_drawable.
>
> Signed-off-by: Mario Kleiner 
> Cc: xorg-de...@lists.x.org
> Cc: dan...@fooishbar.org
> Cc: eero.t.tammi...@intel.com
> Cc: m...@fireburn.co.uk
> ---
>  src/loader/loader_dri3_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 6bb11c4..7bd79af 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable 
> *draw)
>  {
> int i;
>
> +   if (draw->special_event)
> +  loader_dri3_swapbuffer_barrier(draw);
> +
> draw->ext->core->destroyDrawable(draw->dri_drawable);
>
> for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Mike Lothian]

Hi

Yes I've not seen any freezes with Plasmashell, so a big improvement

Had issues with patch 2

Cheers

Mike

On 4 May 2018 at 17:36, Mario Kleiner  wrote:
> On Fri, May 4, 2018 at 6:31 PM, Mike Lothian  wrote:
>> Hi
>>
>> I'm still seeing the freeze in the Steam client with this patch
>>
>> Just about to test the other one
>>
>
> Thanks for testing. So the plasmashell hang is gone, right?
> Maybe the steam issue is a different bug. With *only* patch 2/2
> applied, seing ORPHAN printouts would hint at a similar problem,
> absence of them would probably mean something else.
>
> -mario
>
>> Mike
>>
>> On 4 May 2018 at 17:17, Mike Lothian  wrote:
>>> Hi Mario
>>>
>>> Again thanks for looking into this issue :D
>>>
>>> Tested-by: Mike Lothian 
>>>
>>> I'll give the other patch a whirl now
>>>
>>> Cheers
>>>
>>> Mike
>>>
>>> On 4 May 2018 at 14:45, Mario Kleiner  wrote:
 Before destroying the loader_dri3_drawable, make sure all pending
 swaps for it have completed. This guides against the following scenario,
 which happens, e.g., with KDE Plasma-5's plasmashell (which uses
 QT-5's QtGui/QtQuick for rendering), when it repaints multiple
 UI elements, each represented by its own Window/GLXDrawable, using
 one common GLXContext for all GLXDrawable's:

 1. glXMakeCurrent(dpy, drawable1, context);
 2. glXXX render to drawable1
 3. glXSwapBuffers(dpy, drawable1); #1
 4. glXMakeCurrent(dpy, drawable2, context);
 5. glXXX render to drawable2
 6. glXSwapBuffers(dpy, drawable2);
 // While the swap #1 is still pending for drawable1:
 7. glXMakeCurrent(dpy, drawable1, context);
 8. glXXX render to drawable1
 9. glXSwapBuffers(dpy, drawable1);

 Binding a different drawable2 to the same context via glXMakeCurrent
 will cause its previous drawable1 to be released (cfe. dri3_bind_context
 -> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
 This unselects for Present notify event delivery on the associated
 X-Window and loses all dri3 related state. If drawable1 is selected for
 the context again [7], a new incarnation of loader_dri3_drawable is
 created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
 loader_dri3_drawable_init(), which again selects for Present notify
 event delivery for the underlying X-Window, but the new incarnation lost
 all state wrt. to previous rendering and swaps. The server now delivers
 PresentPixmapIdle and PresentPixmapComplete events from the completed
 previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
 doesn't expect those. One problem is that the new incarnation has a
 draw->send_sbc == 0, but now receives PresentPixmapComplete events with
 sbc's > 0, therefore updating draw->recv_sbc to > 0 in
 dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
 misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
 and ends up with a very large draw->recv_sbc. During the next swapbuffers
 call [9], the totally wrong recv_sbc is used for calculating the target_msc
 for the PresentPixmap request, leading to a target_msc billions of vblanks
 in the future, leading to a swap that never completes and thereby frozen UI
 and hang of the client.

 Make sure that a loader_dri3_drawable can only be destroyed after all
 its pending swaps have completed, to prevent misdelivery of PresentNotify
 events to the right X-Window, but the wrong incarnation of the associated
 loader_dri3_drawable.

 Signed-off-by: Mario Kleiner 
 Cc: xorg-de...@lists.x.org
 Cc: dan...@fooishbar.org
 Cc: eero.t.tammi...@intel.com
 Cc: m...@fireburn.co.uk
 ---
  src/loader/loader_dri3_helper.c | 3 +++
  1 file changed, 3 insertions(+)

 diff --git a/src/loader/loader_dri3_helper.c 
 b/src/loader/loader_dri3_helper.c
 index 6bb11c4..7bd79af 100644
 --- a/src/loader/loader_dri3_helper.c
 +++ b/src/loader/loader_dri3_helper.c
 @@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable 
 *draw)
  {
 int i;

 +   if (draw->special_event)
 +  loader_dri3_swapbuffer_barrier(draw);
 +
 draw->ext->core->destroyDrawable(draw->dri_drawable);

 for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
 --
 2.7.4

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

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Mario Kleiner]

On Fri, May 4, 2018 at 6:31 PM, Mike Lothian  wrote:
> Hi
>
> I'm still seeing the freeze in the Steam client with this patch
>
> Just about to test the other one
>

Thanks for testing. So the plasmashell hang is gone, right?
Maybe the steam issue is a different bug. With *only* patch 2/2
applied, seing ORPHAN printouts would hint at a similar problem,
absence of them would probably mean something else.

-mario

> Mike
>
> On 4 May 2018 at 17:17, Mike Lothian  wrote:
>> Hi Mario
>>
>> Again thanks for looking into this issue :D
>>
>> Tested-by: Mike Lothian 
>>
>> I'll give the other patch a whirl now
>>
>> Cheers
>>
>> Mike
>>
>> On 4 May 2018 at 14:45, Mario Kleiner  wrote:
>>> Before destroying the loader_dri3_drawable, make sure all pending
>>> swaps for it have completed. This guides against the following scenario,
>>> which happens, e.g., with KDE Plasma-5's plasmashell (which uses
>>> QT-5's QtGui/QtQuick for rendering), when it repaints multiple
>>> UI elements, each represented by its own Window/GLXDrawable, using
>>> one common GLXContext for all GLXDrawable's:
>>>
>>> 1. glXMakeCurrent(dpy, drawable1, context);
>>> 2. glXXX render to drawable1
>>> 3. glXSwapBuffers(dpy, drawable1); #1
>>> 4. glXMakeCurrent(dpy, drawable2, context);
>>> 5. glXXX render to drawable2
>>> 6. glXSwapBuffers(dpy, drawable2);
>>> // While the swap #1 is still pending for drawable1:
>>> 7. glXMakeCurrent(dpy, drawable1, context);
>>> 8. glXXX render to drawable1
>>> 9. glXSwapBuffers(dpy, drawable1);
>>>
>>> Binding a different drawable2 to the same context via glXMakeCurrent
>>> will cause its previous drawable1 to be released (cfe. dri3_bind_context
>>> -> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
>>> This unselects for Present notify event delivery on the associated
>>> X-Window and loses all dri3 related state. If drawable1 is selected for
>>> the context again [7], a new incarnation of loader_dri3_drawable is
>>> created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
>>> loader_dri3_drawable_init(), which again selects for Present notify
>>> event delivery for the underlying X-Window, but the new incarnation lost
>>> all state wrt. to previous rendering and swaps. The server now delivers
>>> PresentPixmapIdle and PresentPixmapComplete events from the completed
>>> previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
>>> doesn't expect those. One problem is that the new incarnation has a
>>> draw->send_sbc == 0, but now receives PresentPixmapComplete events with
>>> sbc's > 0, therefore updating draw->recv_sbc to > 0 in
>>> dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
>>> misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
>>> and ends up with a very large draw->recv_sbc. During the next swapbuffers
>>> call [9], the totally wrong recv_sbc is used for calculating the target_msc
>>> for the PresentPixmap request, leading to a target_msc billions of vblanks
>>> in the future, leading to a swap that never completes and thereby frozen UI
>>> and hang of the client.
>>>
>>> Make sure that a loader_dri3_drawable can only be destroyed after all
>>> its pending swaps have completed, to prevent misdelivery of PresentNotify
>>> events to the right X-Window, but the wrong incarnation of the associated
>>> loader_dri3_drawable.
>>>
>>> Signed-off-by: Mario Kleiner 
>>> Cc: xorg-de...@lists.x.org
>>> Cc: dan...@fooishbar.org
>>> Cc: eero.t.tammi...@intel.com
>>> Cc: m...@fireburn.co.uk
>>> ---
>>>  src/loader/loader_dri3_helper.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/src/loader/loader_dri3_helper.c 
>>> b/src/loader/loader_dri3_helper.c
>>> index 6bb11c4..7bd79af 100644
>>> --- a/src/loader/loader_dri3_helper.c
>>> +++ b/src/loader/loader_dri3_helper.c
>>> @@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable 
>>> *draw)
>>>  {
>>> int i;
>>>
>>> +   if (draw->special_event)
>>> +  loader_dri3_swapbuffer_barrier(draw);
>>> +
>>> draw->ext->core->destroyDrawable(draw->dri_drawable);
>>>
>>> for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
>>> --
>>> 2.7.4
>>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Mike Lothian]

Hi

I'm still seeing the freeze in the Steam client with this patch

Just about to test the other one

Mike

On 4 May 2018 at 17:17, Mike Lothian  wrote:
> Hi Mario
>
> Again thanks for looking into this issue :D
>
> Tested-by: Mike Lothian 
>
> I'll give the other patch a whirl now
>
> Cheers
>
> Mike
>
> On 4 May 2018 at 14:45, Mario Kleiner  wrote:
>> Before destroying the loader_dri3_drawable, make sure all pending
>> swaps for it have completed. This guides against the following scenario,
>> which happens, e.g., with KDE Plasma-5's plasmashell (which uses
>> QT-5's QtGui/QtQuick for rendering), when it repaints multiple
>> UI elements, each represented by its own Window/GLXDrawable, using
>> one common GLXContext for all GLXDrawable's:
>>
>> 1. glXMakeCurrent(dpy, drawable1, context);
>> 2. glXXX render to drawable1
>> 3. glXSwapBuffers(dpy, drawable1); #1
>> 4. glXMakeCurrent(dpy, drawable2, context);
>> 5. glXXX render to drawable2
>> 6. glXSwapBuffers(dpy, drawable2);
>> // While the swap #1 is still pending for drawable1:
>> 7. glXMakeCurrent(dpy, drawable1, context);
>> 8. glXXX render to drawable1
>> 9. glXSwapBuffers(dpy, drawable1);
>>
>> Binding a different drawable2 to the same context via glXMakeCurrent
>> will cause its previous drawable1 to be released (cfe. dri3_bind_context
>> -> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
>> This unselects for Present notify event delivery on the associated
>> X-Window and loses all dri3 related state. If drawable1 is selected for
>> the context again [7], a new incarnation of loader_dri3_drawable is
>> created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
>> loader_dri3_drawable_init(), which again selects for Present notify
>> event delivery for the underlying X-Window, but the new incarnation lost
>> all state wrt. to previous rendering and swaps. The server now delivers
>> PresentPixmapIdle and PresentPixmapComplete events from the completed
>> previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
>> doesn't expect those. One problem is that the new incarnation has a
>> draw->send_sbc == 0, but now receives PresentPixmapComplete events with
>> sbc's > 0, therefore updating draw->recv_sbc to > 0 in
>> dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
>> misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
>> and ends up with a very large draw->recv_sbc. During the next swapbuffers
>> call [9], the totally wrong recv_sbc is used for calculating the target_msc
>> for the PresentPixmap request, leading to a target_msc billions of vblanks
>> in the future, leading to a swap that never completes and thereby frozen UI
>> and hang of the client.
>>
>> Make sure that a loader_dri3_drawable can only be destroyed after all
>> its pending swaps have completed, to prevent misdelivery of PresentNotify
>> events to the right X-Window, but the wrong incarnation of the associated
>> loader_dri3_drawable.
>>
>> Signed-off-by: Mario Kleiner 
>> Cc: xorg-de...@lists.x.org
>> Cc: dan...@fooishbar.org
>> Cc: eero.t.tammi...@intel.com
>> Cc: m...@fireburn.co.uk
>> ---
>>  src/loader/loader_dri3_helper.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/src/loader/loader_dri3_helper.c 
>> b/src/loader/loader_dri3_helper.c
>> index 6bb11c4..7bd79af 100644
>> --- a/src/loader/loader_dri3_helper.c
>> +++ b/src/loader/loader_dri3_helper.c
>> @@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable 
>> *draw)
>>  {
>> int i;
>>
>> +   if (draw->special_event)
>> +  loader_dri3_swapbuffer_barrier(draw);
>> +
>> draw->ext->core->destroyDrawable(draw->dri_drawable);
>>
>> for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
>> --
>> 2.7.4
>>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Mike Lothian]

Hi Mario

Again thanks for looking into this issue :D

Tested-by: Mike Lothian 

I'll give the other patch a whirl now

Cheers

Mike

On 4 May 2018 at 14:45, Mario Kleiner  wrote:
> Before destroying the loader_dri3_drawable, make sure all pending
> swaps for it have completed. This guides against the following scenario,
> which happens, e.g., with KDE Plasma-5's plasmashell (which uses
> QT-5's QtGui/QtQuick for rendering), when it repaints multiple
> UI elements, each represented by its own Window/GLXDrawable, using
> one common GLXContext for all GLXDrawable's:
>
> 1. glXMakeCurrent(dpy, drawable1, context);
> 2. glXXX render to drawable1
> 3. glXSwapBuffers(dpy, drawable1); #1
> 4. glXMakeCurrent(dpy, drawable2, context);
> 5. glXXX render to drawable2
> 6. glXSwapBuffers(dpy, drawable2);
> // While the swap #1 is still pending for drawable1:
> 7. glXMakeCurrent(dpy, drawable1, context);
> 8. glXXX render to drawable1
> 9. glXSwapBuffers(dpy, drawable1);
>
> Binding a different drawable2 to the same context via glXMakeCurrent
> will cause its previous drawable1 to be released (cfe. dri3_bind_context
> -> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
> This unselects for Present notify event delivery on the associated
> X-Window and loses all dri3 related state. If drawable1 is selected for
> the context again [7], a new incarnation of loader_dri3_drawable is
> created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
> loader_dri3_drawable_init(), which again selects for Present notify
> event delivery for the underlying X-Window, but the new incarnation lost
> all state wrt. to previous rendering and swaps. The server now delivers
> PresentPixmapIdle and PresentPixmapComplete events from the completed
> previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
> doesn't expect those. One problem is that the new incarnation has a
> draw->send_sbc == 0, but now receives PresentPixmapComplete events with
> sbc's > 0, therefore updating draw->recv_sbc to > 0 in
> dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
> misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
> and ends up with a very large draw->recv_sbc. During the next swapbuffers
> call [9], the totally wrong recv_sbc is used for calculating the target_msc
> for the PresentPixmap request, leading to a target_msc billions of vblanks
> in the future, leading to a swap that never completes and thereby frozen UI
> and hang of the client.
>
> Make sure that a loader_dri3_drawable can only be destroyed after all
> its pending swaps have completed, to prevent misdelivery of PresentNotify
> events to the right X-Window, but the wrong incarnation of the associated
> loader_dri3_drawable.
>
> Signed-off-by: Mario Kleiner 
> Cc: xorg-de...@lists.x.org
> Cc: dan...@fooishbar.org
> Cc: eero.t.tammi...@intel.com
> Cc: m...@fireburn.co.uk
> ---
>  src/loader/loader_dri3_helper.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index 6bb11c4..7bd79af 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable 
> *draw)
>  {
> int i;
>
> +   if (draw->special_event)
> +  loader_dri3_swapbuffer_barrier(draw);
> +
> draw->ext->core->destroyDrawable(draw->dri_drawable);
>
> for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
> --
> 2.7.4
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/2] loader_dri3: Wait for pending swaps to complete before drawable_fini.

[Mario Kleiner]

Before destroying the loader_dri3_drawable, make sure all pending
swaps for it have completed. This guides against the following scenario,
which happens, e.g., with KDE Plasma-5's plasmashell (which uses
QT-5's QtGui/QtQuick for rendering), when it repaints multiple
UI elements, each represented by its own Window/GLXDrawable, using
one common GLXContext for all GLXDrawable's:

1. glXMakeCurrent(dpy, drawable1, context);
2. glXXX render to drawable1
3. glXSwapBuffers(dpy, drawable1); #1
4. glXMakeCurrent(dpy, drawable2, context);
5. glXXX render to drawable2
6. glXSwapBuffers(dpy, drawable2);
// While the swap #1 is still pending for drawable1:
7. glXMakeCurrent(dpy, drawable1, context);
8. glXXX render to drawable1
9. glXSwapBuffers(dpy, drawable1);

Binding a different drawable2 to the same context via glXMakeCurrent
will cause its previous drawable1 to be released (cfe. dri3_bind_context
-> driReleaseDrawables), which in turn calls loader_dri3_drawable_fini().
This unselects for Present notify event delivery on the associated
X-Window and loses all dri3 related state. If drawable1 is selected for
the context again [7], a new incarnation of loader_dri3_drawable is
created in dri3_bind_context->driFetchDrawable->dri3_create_drawable->
loader_dri3_drawable_init(), which again selects for Present notify
event delivery for the underlying X-Window, but the new incarnation lost
all state wrt. to previous rendering and swaps. The server now delivers
PresentPixmapIdle and PresentPixmapComplete events from the completed
previous swapbuffers call #1 [3] to the new loader_dri3_drawable, which
doesn't expect those. One problem is that the new incarnation has a
draw->send_sbc == 0, but now receives PresentPixmapComplete events with
sbc's > 0, therefore updating draw->recv_sbc to > 0 in
dri3_handle_present_event(). The draw->recv_sbc > draw_send_sbc is
misinterpreted as sbc wraparound, triggers recv_sbc wraparound handling
and ends up with a very large draw->recv_sbc. During the next swapbuffers
call [9], the totally wrong recv_sbc is used for calculating the target_msc
for the PresentPixmap request, leading to a target_msc billions of vblanks
in the future, leading to a swap that never completes and thereby frozen UI
and hang of the client.

Make sure that a loader_dri3_drawable can only be destroyed after all
its pending swaps have completed, to prevent misdelivery of PresentNotify
events to the right X-Window, but the wrong incarnation of the associated
loader_dri3_drawable.

Signed-off-by: Mario Kleiner 
Cc: xorg-de...@lists.x.org
Cc: dan...@fooishbar.org
Cc: eero.t.tammi...@intel.com
Cc: m...@fireburn.co.uk
---
 src/loader/loader_dri3_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
index 6bb11c4..7bd79af 100644
--- a/src/loader/loader_dri3_helper.c
+++ b/src/loader/loader_dri3_helper.c
@@ -234,6 +234,9 @@ loader_dri3_drawable_fini(struct loader_dri3_drawable *draw)
 {
int i;
 
+   if (draw->special_event)
+  loader_dri3_swapbuffer_barrier(draw);
+
draw->ext->core->destroyDrawable(draw->dri_drawable);
 
for (i = 0; i < ARRAY_SIZE(draw->buffers); i++) {
-- 
2.7.4

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