Akihiko Odaki <[email protected]> writes:
> virtio-gpu waits for the main thread to destroy resources and replace
> surfaces, but it occasionally results in deadlock, so remove the code
> to wait.
>
> In particular, when running a test case[1] the main thread may wait for
> the vCPUs to pause during shut down while a vCPU may be concurrently
> resetting virtio-gpu.
>
> vCPU actually does not need to perform resource destruction and surface
> replacement synchronously, but it only needs to ensure correct ordering
> among virtio-gpu operations. virtio-gpu-gl already exploits this fact to
> ensure that virglrenderer is reset on the main thread; instead of
> synchronously resetting virglrenderer when the device is being reset,
> it resets virglrenderer just before processing the first command after
> the device reset arrives.
>
> Take advantage of this fact by simply removing synchronization between
> the main thread and the resetting vCPU thread. The request to destroy
> resources and replace surfaces is scheduled earlier than any virtio-gpu
> command that may be queued after resetting ensures correct ordering, so
> we do not need to make additional effort to ensure ordering.
>
> [1]
> https://lore.kernel.org/qemu-devel/[email protected]/
This doesn't seem to make any difference for me with the current state
of the tree.
>
> Fixes: a41e2d97f92b ("virtio-gpu: reset gfx resources in main thread")
> Signed-off-by: Akihiko Odaki <[email protected]>
> ---
> include/hw/virtio/virtio-gpu.h | 2 --
> hw/display/virtio-gpu.c | 9 ---------
> 2 files changed, 11 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 9f16f89a36d2..77d2214238df 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -193,8 +193,6 @@ struct VirtIOGPU {
> QEMUBH *ctrl_bh;
> QEMUBH *cursor_bh;
> QEMUBH *reset_bh;
> - QemuCond reset_cond;
> - bool reset_finished;
>
> QTAILQ_HEAD(, virtio_gpu_simple_resource) reslist;
> QTAILQ_HEAD(, virtio_gpu_ctrl_command) cmdq;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 3a555125be60..1294e1d482ed 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -1522,7 +1522,6 @@ void virtio_gpu_device_realize(DeviceState *qdev, Error
> **errp)
> g->ctrl_bh = virtio_bh_new_guarded(qdev, virtio_gpu_ctrl_bh, g);
> g->cursor_bh = virtio_bh_new_guarded(qdev, virtio_gpu_cursor_bh, g);
> g->reset_bh = qemu_bh_new(virtio_gpu_reset_bh, g);
> - qemu_cond_init(&g->reset_cond);
> QTAILQ_INIT(&g->reslist);
> QTAILQ_INIT(&g->cmdq);
> QTAILQ_INIT(&g->fenceq);
> @@ -1535,7 +1534,6 @@ static void virtio_gpu_device_unrealize(DeviceState
> *qdev)
> g_clear_pointer(&g->ctrl_bh, qemu_bh_delete);
> g_clear_pointer(&g->cursor_bh, qemu_bh_delete);
> g_clear_pointer(&g->reset_bh, qemu_bh_delete);
> - qemu_cond_destroy(&g->reset_cond);
> virtio_gpu_base_device_unrealize(qdev);
> }
>
> @@ -1565,9 +1563,6 @@ static void virtio_gpu_reset_bh(void *opaque)
> for (i = 0; i < g->parent_obj.conf.max_outputs; i++) {
> dpy_gfx_replace_surface(g->parent_obj.scanout[i].con, NULL);
> }
> -
> - g->reset_finished = true;
> - qemu_cond_signal(&g->reset_cond);
> }
>
> void virtio_gpu_reset(VirtIODevice *vdev)
> @@ -1576,11 +1571,7 @@ void virtio_gpu_reset(VirtIODevice *vdev)
> struct virtio_gpu_ctrl_command *cmd;
>
> if (qemu_in_vcpu_thread()) {
> - g->reset_finished = false;
> qemu_bh_schedule(g->reset_bh);
> - while (!g->reset_finished) {
> - qemu_cond_wait_bql(&g->reset_cond);
> - }
> } else {
> aio_bh_call(g->reset_bh);
> }
>
> ---
> base-commit: 36076d24f04ea9dc3357c0fbe7bb14917375819c
> change-id: 20251029-gpu-c3f45747f7ba
>
> Best regards,
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
