[Mesa-dev] [PATCH] vulkan/wsi: Avoid waiting indefinitely for present completion in x11_manage_fifo_queues().

[Michel Dänzer]

[ Resurrecting an old thread... ]

On 2017-10-24 8:24 p.m., Henri Verbeet wrote:
> On 24 October 2017 at 20:13, Fredrik Höglund <fredrik at kde.org> wrote:
>> On Tuesday 24 October 2017, Henri Verbeet wrote:
>>> On 24 October 2017 at 16:11, Fredrik Höglund <fredrik at kde.org> wrote:
>>>>> @@ -934,9 +938,18 @@ x11_manage_fifo_queues(void *state)
>>>>>
>>>>>        while (chain->last_present_msc < target_msc) {
>>>>>           xcb_generic_event_t *event =
>>>>> -            xcb_wait_for_special_event(chain->conn, chain->special_event);
>>>>> -         if (!event)
>>>>> -            goto fail;
>>>>> +            xcb_poll_for_special_event(chain->conn, chain->special_event);
>>>>> +         if (!event) {
>>>>> +            int ret = poll(&pfds, 1, 100);
>>>>
>>>> There is a race condition here where another thread can read the event
>>>> from the file descriptor in the time between the calls to
>>>> xcb_poll_for_special_event() and poll().
>>>>
>>> Is that a scenario we care about? Unless I'm misunderstanding
>>> something, the same kind of thing could happen between
>>> x11_present_to_x11() and xcb_wait_for_special_event().
>>
>> It cannot, because if another thread reads a special event, xcb will
>> insert it into the corresponding special event queue and wake the
>> waiting thread. That's the point of having special event queues.
>>
>> But the reason I know this to be a problem is that I have tried to fix
>> this bug in the same way, and I noticed that it resulted in frequent
>> random stutters in some apps because poll() was timing out.
>>
> Oh, I think I see what you mean. The event wouldn't get lost, but we'd
> have to wait for the poll to timeout to get to it because it's already
> read from the fd into the queue.

One way to address this is as in
src/loader/loader_dri3_helper.c:dri3_wait_for_event_locked: the first
thread to enter waits for and processes a special event, then wakes up
other threads which have entered in the meantime.


https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/5368 is my
proposed solution for the same issue in dri3_wait_for_event_locked. (In
contrast to this patch, it explicitly checks if the window still exists
and only bails early if it doesn't)


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer