On 10/22/24 08:11, Akihiko Odaki wrote:
> On 2024/10/19 6:31, Dmitry Osipenko wrote:
>> On 10/18/24 08:28, Akihiko Odaki wrote:
>>>> +static void virgl_write_context_fence(void *opaque, uint32_t ctx_id,
>>>> + uint32_t ring_idx, uint64_t
>>>> fence)
>>>> +{
>>>> + VirtIOGPU *g = opaque;
>>>
>>> What about taking the BQL here instead of having a QEMUBH?
>>
>> That will block virglrenderer thread writing the fence, which in turns
>> might block other virglrenderer threads.
>
> Looking at virglrenderer's source code, the thread writing the fence is
> the only thread it creates. Otherwise virglrenderer's code should be
> executed only in the QEMU thread calling virglrenderer's functions,
> which always holds the BQL. So taking the BQL here will not interfere
> with another thread.
There are other problems with that BQL approach:
1. virgl_renderer_context_destroy() is executed from QEMU's main-loop
and it will terminate the virglrenderer's fence-sync threads which at
the same time will take the same BQL if fence fires while VM is shutting
down and then this condition will result in a deadlock.
2. In a case of vrend, the fence-sync thread uses a different GL context
than the QEMU's main-loop vrend thread. Feels like too much potential
problems with GL context switching here. We shouldn't be able to jump
into QEMU's GL code from a non-vrend GL thread.
Too many complications with the BQL approach for a little benefit, IMO.
--
Best regards,
Dmitry
