[ 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 <fred...@kde.org> wrote: >> On Tuesday 24 October 2017, Henri Verbeet wrote: >>> On 24 October 2017 at 16:11, Fredrik Höglund <fred...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev