* [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock
@ 2026-06-30 9:16 Ryosuke Yasuoka
2026-06-30 9:36 ` sashiko-bot
2026-06-30 13:46 ` Dmitry Osipenko
0 siblings, 2 replies; 12+ messages in thread
From: Ryosuke Yasuoka @ 2026-06-30 9:16 UTC (permalink / raw)
To: David Airlie, Gerd Hoffmann, Dmitry Osipenko, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas
Cc: dri-devel, virtualization, linux-kernel, Ryosuke Yasuoka
A probe-time deadlock can occur between the dequeue worker and
drm_client_register(). During probe, drm_client_register() holds
clientlist_mutex and calls the fbdev hotplug callback, which triggers an
atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
waiting for virtqueue space. The dequeue worker that would free that
space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
to acquire the same clientlist_mutex. Since wake_up() is only called
after the resp_cb loop, the probe thread is never woken and both threads
deadlock.
Fix this by deferring the hotplug notification from
virtio_gpu_cmd_get_display_info_cb() to a separate work item. The
display data (outputs[i].info) is still updated synchronously in the
callback, and the deferred work only triggers a re-probe notification to
DRM clients.
Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++
drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++
drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++--
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 7449907754a4..27ffa4697ae9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -264,6 +264,8 @@ struct virtio_gpu_device {
struct work_struct config_changed_work;
+ struct work_struct hotplug_work;
+
struct work_struct obj_free_work;
spinlock_t obj_free_lock;
struct list_head obj_free_list;
@@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
uint32_t x, uint32_t y,
struct virtio_gpu_object_array *objs,
struct virtio_gpu_fence *fence);
+void virtio_gpu_hotplug_work_func(struct work_struct *work);
void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev,
uint32_t resource_id,
uint32_t x, uint32_t y,
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index cfde9f573df6..cfb532ba43a4 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev)
INIT_WORK(&vgdev->config_changed_work,
virtio_gpu_config_changed_work_func);
+ INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func);
+
INIT_WORK(&vgdev->obj_free_work,
virtio_gpu_array_put_free_work);
INIT_LIST_HEAD(&vgdev->obj_free_list);
@@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev)
flush_work(&vgdev->obj_free_work);
flush_work(&vgdev->ctrlq.dequeue_work);
flush_work(&vgdev->cursorq.dequeue_work);
+ flush_work(&vgdev->hotplug_work);
flush_work(&vgdev->config_changed_work);
virtio_reset_device(vgdev->vdev);
vgdev->vdev->config->del_vqs(vgdev->vdev);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 67865810a2e7..084d98f5dc7b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev,
virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence);
}
+void virtio_gpu_hotplug_work_func(struct work_struct *work)
+{
+ struct virtio_gpu_device *vgdev =
+ container_of(work, struct virtio_gpu_device, hotplug_work);
+
+ if (!drm_helper_hpd_irq_event(vgdev->ddev))
+ drm_kms_helper_hotplug_event(vgdev->ddev);
+}
+
static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
@@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
spin_unlock(&vgdev->display_info_lock);
wake_up(&vgdev->resp_wq);
- if (!drm_helper_hpd_irq_event(vgdev->ddev))
- drm_kms_helper_hotplug_event(vgdev->ddev);
+ schedule_work(&vgdev->hotplug_work);
}
static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260619-virtiogpu_syzbot-bdab508ffcd5
Best regards,
--
Ryosuke Yasuoka <ryasuoka@redhat.com>
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-06-30 9:16 [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock Ryosuke Yasuoka @ 2026-06-30 9:36 ` sashiko-bot 2026-06-30 13:46 ` Dmitry Osipenko 1 sibling, 0 replies; 12+ messages in thread From: sashiko-bot @ 2026-06-30 9:36 UTC (permalink / raw) To: Ryosuke Yasuoka; +Cc: dri-devel Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] Flawed driver teardown order allows work items (including the newly added `hotplug_work`) to be re-queued and executed after the device structure and virtqueues are freed. -- commit 81a0fe4ee345c5c5f16f0048bb1284a83aa82e58 Author: Ryosuke Yasuoka <ryasuoka@redhat.com> drm/virtio: defer hotplug event from dequeue worker to avoid deadlock A probe-time deadlock can occur between the dequeue worker and drm_client_register(). This patch defers the hotplug notification from virtio_gpu_cmd_get_display_info_cb() to a separate work item to prevent this deadlock. Link: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 Link: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c > index cfde9f573df6e..cfb532ba43a47 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c [ ... ] > @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) > flush_work(&vgdev->obj_free_work); > flush_work(&vgdev->ctrlq.dequeue_work); > flush_work(&vgdev->cursorq.dequeue_work); > + flush_work(&vgdev->hotplug_work); > flush_work(&vgdev->config_changed_work); > virtio_reset_device(vgdev->vdev); > vgdev->vdev->config->del_vqs(vgdev->vdev); [Severity: High] This is a pre-existing issue, but does this teardown sequence allow work items to be re-queued after they are flushed? Here in virtio_gpu_deinit(), the work items are flushed before the device is reset via virtio_reset_device() and the virtqueues are deleted via del_vqs(). Because the device is still active during the flush, if responses arrive for shutdown commands (like those queued by drm_atomic_helper_shutdown()), the interrupt handler can re-queue the work: virtio_gpu_ctrl_ack() schedule_work(&vgdev->ctrlq.dequeue_work); This re-queued dequeue_work can then execute after del_vqs() has freed the virtqueues, leading to a use-after-free. Furthermore, it could schedule the newly added hotplug_work: virtio_gpu_cmd_get_display_info_cb() schedule_work(&vgdev->hotplug_work); This would cause further use-after-free access on the freed vgdev structure. Should the flush_work() calls be moved after virtio_reset_device() to ensure no new interrupts can re-queue these work items? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com?part=1 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-06-30 9:16 [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock Ryosuke Yasuoka 2026-06-30 9:36 ` sashiko-bot @ 2026-06-30 13:46 ` Dmitry Osipenko 2026-07-01 9:23 ` Ryosuke Yasuoka 1 sibling, 1 reply; 12+ messages in thread From: Dmitry Osipenko @ 2026-06-30 13:46 UTC (permalink / raw) To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel Hi, On 6/30/26 12:16, Ryosuke Yasuoka wrote: > A probe-time deadlock can occur between the dequeue worker and > drm_client_register(). During probe, drm_client_register() holds > clientlist_mutex and calls the fbdev hotplug callback, which triggers an > atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() > waiting for virtqueue space. The dequeue worker that would free that > space calls virtio_gpu_cmd_get_display_info_cb(), which invokes > drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting > to acquire the same clientlist_mutex. Since wake_up() is only called > after the resp_cb loop, the probe thread is never woken and both threads > deadlock. > > Fix this by deferring the hotplug notification from > virtio_gpu_cmd_get_display_info_cb() to a separate work item. The > display data (outputs[i].info) is still updated synchronously in the > callback, and the deferred work only triggers a re-probe notification to > DRM clients. > > Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") > Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 > Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> > --- > drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ > drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ > drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- > 3 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h > index 7449907754a4..27ffa4697ae9 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_drv.h > +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h > @@ -264,6 +264,8 @@ struct virtio_gpu_device { > > struct work_struct config_changed_work; > > + struct work_struct hotplug_work; > + > struct work_struct obj_free_work; > spinlock_t obj_free_lock; > struct list_head obj_free_list; > @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, > uint32_t x, uint32_t y, > struct virtio_gpu_object_array *objs, > struct virtio_gpu_fence *fence); > +void virtio_gpu_hotplug_work_func(struct work_struct *work); > void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, > uint32_t resource_id, > uint32_t x, uint32_t y, > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c > index cfde9f573df6..cfb532ba43a4 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c > @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) > INIT_WORK(&vgdev->config_changed_work, > virtio_gpu_config_changed_work_func); > > + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); > + > INIT_WORK(&vgdev->obj_free_work, > virtio_gpu_array_put_free_work); > INIT_LIST_HEAD(&vgdev->obj_free_list); > @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) > flush_work(&vgdev->obj_free_work); > flush_work(&vgdev->ctrlq.dequeue_work); > flush_work(&vgdev->cursorq.dequeue_work); > + flush_work(&vgdev->hotplug_work); > flush_work(&vgdev->config_changed_work); > virtio_reset_device(vgdev->vdev); > vgdev->vdev->config->del_vqs(vgdev->vdev); > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 67865810a2e7..084d98f5dc7b 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, > virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); > } > > +void virtio_gpu_hotplug_work_func(struct work_struct *work) > +{ > + struct virtio_gpu_device *vgdev = > + container_of(work, struct virtio_gpu_device, hotplug_work); > + > + if (!drm_helper_hpd_irq_event(vgdev->ddev)) > + drm_kms_helper_hotplug_event(vgdev->ddev); > +} > + > static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, > struct virtio_gpu_vbuffer *vbuf) > { > @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, > spin_unlock(&vgdev->display_info_lock); > wake_up(&vgdev->resp_wq); > > - if (!drm_helper_hpd_irq_event(vgdev->ddev)) > - drm_kms_helper_hotplug_event(vgdev->ddev); > + schedule_work(&vgdev->hotplug_work); > } > > static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-06-30 13:46 ` Dmitry Osipenko @ 2026-07-01 9:23 ` Ryosuke Yasuoka 2026-07-01 23:43 ` Hillf Danton 2026-07-02 11:26 ` Dmitry Osipenko 0 siblings, 2 replies; 12+ messages in thread From: Ryosuke Yasuoka @ 2026-07-01 9:23 UTC (permalink / raw) To: Dmitry Osipenko, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel On 30/06/2026 16:46, Dmitry Osipenko wrote: > Hi, > > On 6/30/26 12:16, Ryosuke Yasuoka wrote: >> A probe-time deadlock can occur between the dequeue worker and >> drm_client_register(). During probe, drm_client_register() holds >> clientlist_mutex and calls the fbdev hotplug callback, which triggers an >> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() >> waiting for virtqueue space. The dequeue worker that would free that >> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes >> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting >> to acquire the same clientlist_mutex. Since wake_up() is only called >> after the resp_cb loop, the probe thread is never woken and both threads >> deadlock. >> >> Fix this by deferring the hotplug notification from >> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The >> display data (outputs[i].info) is still updated synchronously in the >> callback, and the deferred work only triggers a re-probe notification to >> DRM clients. >> >> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") >> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >> --- >> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ >> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ >> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- >> 3 files changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h >> index 7449907754a4..27ffa4697ae9 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h >> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h >> @@ -264,6 +264,8 @@ struct virtio_gpu_device { >> >> struct work_struct config_changed_work; >> >> + struct work_struct hotplug_work; >> + >> struct work_struct obj_free_work; >> spinlock_t obj_free_lock; >> struct list_head obj_free_list; >> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, >> uint32_t x, uint32_t y, >> struct virtio_gpu_object_array *objs, >> struct virtio_gpu_fence *fence); >> +void virtio_gpu_hotplug_work_func(struct work_struct *work); >> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, >> uint32_t resource_id, >> uint32_t x, uint32_t y, >> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c >> index cfde9f573df6..cfb532ba43a4 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) >> INIT_WORK(&vgdev->config_changed_work, >> virtio_gpu_config_changed_work_func); >> >> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); >> + >> INIT_WORK(&vgdev->obj_free_work, >> virtio_gpu_array_put_free_work); >> INIT_LIST_HEAD(&vgdev->obj_free_list); >> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) >> flush_work(&vgdev->obj_free_work); >> flush_work(&vgdev->ctrlq.dequeue_work); >> flush_work(&vgdev->cursorq.dequeue_work); >> + flush_work(&vgdev->hotplug_work); >> flush_work(&vgdev->config_changed_work); >> virtio_reset_device(vgdev->vdev); >> vgdev->vdev->config->del_vqs(vgdev->vdev); >> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c >> index 67865810a2e7..084d98f5dc7b 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, >> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); >> } >> >> +void virtio_gpu_hotplug_work_func(struct work_struct *work) >> +{ >> + struct virtio_gpu_device *vgdev = >> + container_of(work, struct virtio_gpu_device, hotplug_work); >> + >> + if (!drm_helper_hpd_irq_event(vgdev->ddev)) >> + drm_kms_helper_hotplug_event(vgdev->ddev); >> +} >> + >> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >> struct virtio_gpu_vbuffer *vbuf) >> { >> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >> spin_unlock(&vgdev->display_info_lock); >> wake_up(&vgdev->resp_wq); >> >> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >> - drm_kms_helper_hotplug_event(vgdev->ddev); >> + schedule_work(&vgdev->hotplug_work); >> } >> >> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, > Hi, Thank you for your review. > Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() into virtio_gpu_init() after wait_event_timeout() would not prevent the issue. Looking at the syzbot call traces[1][2], the deadlock occurs during drm_client_setup(), which runs after virtio_gpu_init() has already returned. The display_info_cb that triggers the deadlock is called from the dequeue worker while drm_client_register() holds clientlist_mutex. Thread A: virtio_gpu_probe() -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s -> drm_dev_register() -> drm_client_setup() // deadlock happens HERE -> drm_client_register() // holds clientlist_mutex ... -> virtio_gpu_queue_fenced_ctrl_buffer() -> wait_event() // waits for free space Thread B: virtio_gpu_dequeue_ctrl_func() -> reclaim_vbufs() // make free space -> resp_cb() -> virtio_gpu_cmd_get_display_info_cb -> drm_helper_hpd_irq_event() -> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug() // need to lock clientlist_mutex -> wake_up() // never reached IIUC the hotplug notification in display_info_cb is needed because it notifies DRM after updating by the callback with fresh data from the host. This work_struct ensures display_info_cb never blocks on clientlist_mutex in the dequeue worker, while preserving the hotplug notification with fresh data. [1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 [2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e Best regards, Ryosuke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-01 9:23 ` Ryosuke Yasuoka @ 2026-07-01 23:43 ` Hillf Danton 2026-07-02 7:12 ` Ryosuke Yasuoka 2026-07-02 11:26 ` Dmitry Osipenko 1 sibling, 1 reply; 12+ messages in thread From: Hillf Danton @ 2026-07-01 23:43 UTC (permalink / raw) To: Ryosuke Yasuoka Cc: Dmitry Osipenko, Dmitry Baryshkov, Javier Martinez Canillas, dri-devel, virtualization, linux-kernel On Wed, 1 Jul 2026 09:23:05 +0000 Ryosuke Yasuoka wrote: >On 30/06/2026 16:46, Dmitry Osipenko wrote: >> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), >> placing it after wait_event_timeout(display_info_pending)? This will avoid >> additional work_struct that otherwise needs to be cancelled in >> virtio_gpu_init() on the timeout. > >IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >into virtio_gpu_init() after wait_event_timeout() would not prevent the >issue. > >Looking at the syzbot call traces[1][2], the deadlock occurs during >drm_client_setup(), which runs after virtio_gpu_init() has already >returned. The display_info_cb that triggers the deadlock is called from >the dequeue worker while drm_client_register() holds clientlist_mutex. > >Thread A: >virtio_gpu_probe() > -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s > -> drm_dev_register() > -> drm_client_setup() // deadlock happens HERE > -> drm_client_register() // holds clientlist_mutex > ... > -> virtio_gpu_queue_fenced_ctrl_buffer() > -> wait_event() // waits for free space > >Thread B: >virtio_gpu_dequeue_ctrl_func() > -> reclaim_vbufs() // make free space I wonder if the deadlock could be cured by alternatively adding a wakeup before the responce cb, given that free space is available. > -> resp_cb() > -> virtio_gpu_cmd_get_display_info_cb > -> drm_helper_hpd_irq_event() > -> drm_kms_helper_hotplug_event() > -> drm_client_dev_hotplug() // need to lock clientlist_mutex > -> wake_up() // never reached > >IIUC the hotplug notification in display_info_cb is needed because it >notifies DRM after updating by the callback with fresh data from the host. > >This work_struct ensures display_info_cb never blocks on >clientlist_mutex in the dequeue worker, while preserving the hotplug >notification with fresh data. > >[1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >[2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e > >Best regards, >Ryosuke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-01 23:43 ` Hillf Danton @ 2026-07-02 7:12 ` Ryosuke Yasuoka 2026-07-02 8:25 ` Hillf Danton 0 siblings, 1 reply; 12+ messages in thread From: Ryosuke Yasuoka @ 2026-07-02 7:12 UTC (permalink / raw) To: Hillf Danton Cc: Dmitry Osipenko, Dmitry Baryshkov, Javier Martinez Canillas, dri-devel, virtualization, linux-kernel On 02/07/2026 07:43, Hillf Danton wrote: > On Wed, 1 Jul 2026 09:23:05 +0000 Ryosuke Yasuoka wrote: >>On 30/06/2026 16:46, Dmitry Osipenko wrote: >>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), >>> placing it after wait_event_timeout(display_info_pending)? This will avoid >>> additional work_struct that otherwise needs to be cancelled in >>> virtio_gpu_init() on the timeout. >> >>IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >>into virtio_gpu_init() after wait_event_timeout() would not prevent the >>issue. >> >>Looking at the syzbot call traces[1][2], the deadlock occurs during >>drm_client_setup(), which runs after virtio_gpu_init() has already >>returned. The display_info_cb that triggers the deadlock is called from >>the dequeue worker while drm_client_register() holds clientlist_mutex. >> >>Thread A: >>virtio_gpu_probe() >> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s >> -> drm_dev_register() >> -> drm_client_setup() // deadlock happens HERE >> -> drm_client_register() // holds clientlist_mutex >> ... >> -> virtio_gpu_queue_fenced_ctrl_buffer() >> -> wait_event() // waits for free space >> >>Thread B: >>virtio_gpu_dequeue_ctrl_func() >> -> reclaim_vbufs() // make free space > > I wonder if the deadlock could be cured by alternatively adding a wakeup > before the responce cb, given that free space is available. Thanks for your comment. I agree that moving wake_up() before response cb would help in most cases, and I think it is a reasonable aproach. However, there are a few rare scenarios where the deadlock can still occur: 1. If reclaim_vbufs() does not free enough slots to satisfy num_free >= elemcnt, wake_up() fires but the wait_event() condition evaluates false. Thread A remains asleep holding clientlist_mutex, and the dequeue worker proceeds to display_info_cb which blocks on clientlist_mutex. 2. Even if enough slots are freed, there is a small window between wait_event() returning and Thread A acquiring spin_lock(&qlock) at the "goto again" path in virtio_gpu_queue_ctrl_sgs(). Another thread can consume the freed slots during this window, causing Thread A to loop back to wait_event() still holding clientlist_mutex. 3. Reordering wake_up() before resp_cb inverts the natural "process response, then signal completion" pattern, which could introduce subtle issues if a future resp_cb depends on side effects that should happen after wake_up(), I believe. These scenarios are likely rare in practice, but the schedule_work() approach avoids all of them entirely by ensuring the dequeue worker never touches clientlist_mutex. It also follows the same pattern already used by config_changed_work in this driver. Best regards Ryosuke > >> -> resp_cb() >> -> virtio_gpu_cmd_get_display_info_cb >> -> drm_helper_hpd_irq_event() >> -> drm_kms_helper_hotplug_event() >> -> drm_client_dev_hotplug() // need to lock clientlist_mutex >> -> wake_up() // never reached >> >>IIUC the hotplug notification in display_info_cb is needed because it >>notifies DRM after updating by the callback with fresh data from the host. >> >>This work_struct ensures display_info_cb never blocks on >>clientlist_mutex in the dequeue worker, while preserving the hotplug >>notification with fresh data. >> >>[1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>[2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >> >>Best regards, >>Ryosuke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-02 7:12 ` Ryosuke Yasuoka @ 2026-07-02 8:25 ` Hillf Danton 0 siblings, 0 replies; 12+ messages in thread From: Hillf Danton @ 2026-07-02 8:25 UTC (permalink / raw) To: Ryosuke Yasuoka Cc: Dmitry Osipenko, Dmitry Baryshkov, Javier Martinez Canillas, dri-devel, virtualization, linux-kernel On Thu, 2 Jul 2026 07:12:44 +0000 Ryosuke Yasuoka wrote: >On 02/07/2026 07:43, Hillf Danton wrote: >> On Wed, 1 Jul 2026 09:23:05 +0000 Ryosuke Yasuoka wrote: >>>On 30/06/2026 16:46, Dmitry Osipenko wrote: >>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), >>>> placing it after wait_event_timeout(display_info_pending)? This will avoid >>>> additional work_struct that otherwise needs to be cancelled in >>>> virtio_gpu_init() on the timeout. >>> >>>IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >>>into virtio_gpu_init() after wait_event_timeout() would not prevent the >>>issue. >>> >>>Looking at the syzbot call traces[1][2], the deadlock occurs during >>>drm_client_setup(), which runs after virtio_gpu_init() has already >>>returned. The display_info_cb that triggers the deadlock is called from >>>the dequeue worker while drm_client_register() holds clientlist_mutex. >>> >>>Thread A: >>>virtio_gpu_probe() >>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s >>> -> drm_dev_register() >>> -> drm_client_setup() // deadlock happens HERE >>> -> drm_client_register() // holds clientlist_mutex >>> ... >>> -> virtio_gpu_queue_fenced_ctrl_buffer() >>> -> wait_event() // waits for free space >>> >>>Thread B: >>>virtio_gpu_dequeue_ctrl_func() >>> -> reclaim_vbufs() // make free space >> >> I wonder if the deadlock could be cured by alternatively adding a wakeup >> before the responce cb, given that free space is available. > >Thanks for your comment. I agree that moving wake_up() before response >cb would help in most cases, and I think it is a reasonable aproach. >However, there are a few rare scenarios where the deadlock can still >occur: > >1. If reclaim_vbufs() does not free enough slots to satisfy >num_free >= elemcnt, wake_up() fires but the wait_event() condition >evaluates false. Thread A remains asleep holding clientlist_mutex, and >the dequeue worker proceeds to display_info_cb which blocks on >clientlist_mutex. task0 task1 lock clientlist_mutex wait_event(for enough free slots) free some slots lock clientlist_mutex unlock clientlist_mutex wakeup deadlock The clientlist_mutex will not be released without enough slots freed, so more free slots and wakeup are needed to cure the deadlock, with nothing to do with acquiring the mutex in the workqueue context. Your workqueue approach works by removing the dependence of wakeup on acquiring the mutex, without enough free slots guaranteed. In short, wakeup before acquiring the mutex is the right thing to do, I mean it is not the only one at all. >2. Even if enough slots are freed, there is a small window between >wait_event() returning and Thread A acquiring spin_lock(&qlock) at the >"goto again" path in virtio_gpu_queue_ctrl_sgs(). Another thread can >consume the freed slots during this window, causing Thread A to loop >back to wait_event() still holding clientlist_mutex. Ditto >3. Reordering wake_up() before resp_cb inverts the natural "process >response, then signal completion" pattern, which could introduce subtle >issues if a future resp_cb depends on side effects that should happen >after wake_up(), I believe. > Different taste >These scenarios are likely rare in practice, but the schedule_work() >approach avoids all of them entirely by ensuring the dequeue worker >never touches clientlist_mutex. It also follows the same pattern >already used by config_changed_work in this driver. > >Best regards >Ryosuke > >> >>> -> resp_cb() >>> -> virtio_gpu_cmd_get_display_info_cb >>> -> drm_helper_hpd_irq_event() >>> -> drm_kms_helper_hotplug_event() >>> -> drm_client_dev_hotplug() // need to lock clientlist_mutex >>> -> wake_up() // never reached >>> >>>IIUC the hotplug notification in display_info_cb is needed because it >>>notifies DRM after updating by the callback with fresh data from the host. >>> >>>This work_struct ensures display_info_cb never blocks on >>>clientlist_mutex in the dequeue worker, while preserving the hotplug >>>notification with fresh data. >>> >>>[1] https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>>[2] https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >>> >>>Best regards, >>>Ryosuke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-01 9:23 ` Ryosuke Yasuoka 2026-07-01 23:43 ` Hillf Danton @ 2026-07-02 11:26 ` Dmitry Osipenko 2026-07-03 5:58 ` Ryosuke Yasuoka 1 sibling, 1 reply; 12+ messages in thread From: Dmitry Osipenko @ 2026-07-02 11:26 UTC (permalink / raw) To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel On 7/1/26 12:23, Ryosuke Yasuoka wrote: > > > On 30/06/2026 16:46, Dmitry Osipenko wrote: >> Hi, >> >> On 6/30/26 12:16, Ryosuke Yasuoka wrote: >>> A probe-time deadlock can occur between the dequeue worker and >>> drm_client_register(). During probe, drm_client_register() holds >>> clientlist_mutex and calls the fbdev hotplug callback, which triggers an >>> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() >>> waiting for virtqueue space. The dequeue worker that would free that >>> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes >>> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting >>> to acquire the same clientlist_mutex. Since wake_up() is only called >>> after the resp_cb loop, the probe thread is never woken and both threads >>> deadlock. >>> >>> Fix this by deferring the hotplug notification from >>> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The >>> display data (outputs[i].info) is still updated synchronously in the >>> callback, and the deferred work only triggers a re-probe notification to >>> DRM clients. >>> >>> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") >>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >>> --- >>> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ >>> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ >>> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- >>> 3 files changed, 16 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h >>> index 7449907754a4..27ffa4697ae9 100644 >>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h >>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h >>> @@ -264,6 +264,8 @@ struct virtio_gpu_device { >>> >>> struct work_struct config_changed_work; >>> >>> + struct work_struct hotplug_work; >>> + >>> struct work_struct obj_free_work; >>> spinlock_t obj_free_lock; >>> struct list_head obj_free_list; >>> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, >>> uint32_t x, uint32_t y, >>> struct virtio_gpu_object_array *objs, >>> struct virtio_gpu_fence *fence); >>> +void virtio_gpu_hotplug_work_func(struct work_struct *work); >>> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, >>> uint32_t resource_id, >>> uint32_t x, uint32_t y, >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c >>> index cfde9f573df6..cfb532ba43a4 100644 >>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >>> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) >>> INIT_WORK(&vgdev->config_changed_work, >>> virtio_gpu_config_changed_work_func); >>> >>> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); >>> + >>> INIT_WORK(&vgdev->obj_free_work, >>> virtio_gpu_array_put_free_work); >>> INIT_LIST_HEAD(&vgdev->obj_free_list); >>> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) >>> flush_work(&vgdev->obj_free_work); >>> flush_work(&vgdev->ctrlq.dequeue_work); >>> flush_work(&vgdev->cursorq.dequeue_work); >>> + flush_work(&vgdev->hotplug_work); >>> flush_work(&vgdev->config_changed_work); >>> virtio_reset_device(vgdev->vdev); >>> vgdev->vdev->config->del_vqs(vgdev->vdev); >>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c >>> index 67865810a2e7..084d98f5dc7b 100644 >>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >>> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, >>> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); >>> } >>> >>> +void virtio_gpu_hotplug_work_func(struct work_struct *work) >>> +{ >>> + struct virtio_gpu_device *vgdev = >>> + container_of(work, struct virtio_gpu_device, hotplug_work); >>> + >>> + if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>> + drm_kms_helper_hotplug_event(vgdev->ddev); >>> +} >>> + >>> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>> struct virtio_gpu_vbuffer *vbuf) >>> { >>> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>> spin_unlock(&vgdev->display_info_lock); >>> wake_up(&vgdev->resp_wq); >>> >>> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>> - drm_kms_helper_hotplug_event(vgdev->ddev); >>> + schedule_work(&vgdev->hotplug_work); >>> } >>> >>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, >> > > Hi, > Thank you for your review. > >> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. > > IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() > into virtio_gpu_init() after wait_event_timeout() would not prevent the > issue. > > Looking at the syzbot call traces[1][2], the deadlock occurs during > drm_client_setup(), which runs after virtio_gpu_init() has already > returned. The display_info_cb that triggers the deadlock is called from > the dequeue worker while drm_client_register() holds clientlist_mutex. > > Thread A: > virtio_gpu_probe() > -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s You mean that the timeout happens and it's again syzkaller report for a broken host. A day ago I applied [1] that should fix this "bogus" syzkaller report. [1] https://patchwork.freedesktop.org/patch/735301/ -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-02 11:26 ` Dmitry Osipenko @ 2026-07-03 5:58 ` Ryosuke Yasuoka 2026-07-07 15:10 ` Dmitry Osipenko 0 siblings, 1 reply; 12+ messages in thread From: Ryosuke Yasuoka @ 2026-07-03 5:58 UTC (permalink / raw) To: Dmitry Osipenko, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel On 02/07/2026 14:26, Dmitry Osipenko wrote: > On 7/1/26 12:23, Ryosuke Yasuoka wrote: >> >> >> On 30/06/2026 16:46, Dmitry Osipenko wrote: >>> Hi, >>> >>> On 6/30/26 12:16, Ryosuke Yasuoka wrote: >>>> A probe-time deadlock can occur between the dequeue worker and >>>> drm_client_register(). During probe, drm_client_register() holds >>>> clientlist_mutex and calls the fbdev hotplug callback, which triggers an >>>> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() >>>> waiting for virtqueue space. The dequeue worker that would free that >>>> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes >>>> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting >>>> to acquire the same clientlist_mutex. Since wake_up() is only called >>>> after the resp_cb loop, the probe thread is never woken and both threads >>>> deadlock. >>>> >>>> Fix this by deferring the hotplug notification from >>>> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The >>>> display data (outputs[i].info) is still updated synchronously in the >>>> callback, and the deferred work only triggers a re-probe notification to >>>> DRM clients. >>>> >>>> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") >>>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >>>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >>>> --- >>>> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ >>>> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ >>>> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- >>>> 3 files changed, 16 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>> index 7449907754a4..27ffa4697ae9 100644 >>>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h >>>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>> @@ -264,6 +264,8 @@ struct virtio_gpu_device { >>>> >>>> struct work_struct config_changed_work; >>>> >>>> + struct work_struct hotplug_work; >>>> + >>>> struct work_struct obj_free_work; >>>> spinlock_t obj_free_lock; >>>> struct list_head obj_free_list; >>>> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, >>>> uint32_t x, uint32_t y, >>>> struct virtio_gpu_object_array *objs, >>>> struct virtio_gpu_fence *fence); >>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work); >>>> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, >>>> uint32_t resource_id, >>>> uint32_t x, uint32_t y, >>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>> index cfde9f573df6..cfb532ba43a4 100644 >>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) >>>> INIT_WORK(&vgdev->config_changed_work, >>>> virtio_gpu_config_changed_work_func); >>>> >>>> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); >>>> + >>>> INIT_WORK(&vgdev->obj_free_work, >>>> virtio_gpu_array_put_free_work); >>>> INIT_LIST_HEAD(&vgdev->obj_free_list); >>>> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) >>>> flush_work(&vgdev->obj_free_work); >>>> flush_work(&vgdev->ctrlq.dequeue_work); >>>> flush_work(&vgdev->cursorq.dequeue_work); >>>> + flush_work(&vgdev->hotplug_work); >>>> flush_work(&vgdev->config_changed_work); >>>> virtio_reset_device(vgdev->vdev); >>>> vgdev->vdev->config->del_vqs(vgdev->vdev); >>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>> index 67865810a2e7..084d98f5dc7b 100644 >>>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >>>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, >>>> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); >>>> } >>>> >>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work) >>>> +{ >>>> + struct virtio_gpu_device *vgdev = >>>> + container_of(work, struct virtio_gpu_device, hotplug_work); >>>> + >>>> + if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>> + drm_kms_helper_hotplug_event(vgdev->ddev); >>>> +} >>>> + >>>> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>> struct virtio_gpu_vbuffer *vbuf) >>>> { >>>> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>> spin_unlock(&vgdev->display_info_lock); >>>> wake_up(&vgdev->resp_wq); >>>> >>>> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>> - drm_kms_helper_hotplug_event(vgdev->ddev); >>>> + schedule_work(&vgdev->hotplug_work); >>>> } >>>> >>>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, >>> >> >> Hi, >> Thank you for your review. >> >>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. >> >> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >> into virtio_gpu_init() after wait_event_timeout() would not prevent the >> issue. >> >> Looking at the syzbot call traces[1][2], the deadlock occurs during >> drm_client_setup(), which runs after virtio_gpu_init() has already >> returned. The display_info_cb that triggers the deadlock is called from >> the dequeue worker while drm_client_register() holds clientlist_mutex. >> >> Thread A: >> virtio_gpu_probe() >> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s > > You mean that the timeout happens and it's again syzkaller report for a > broken host. A day ago I applied [1] that should fix this "bogus" > syzkaller report. > > [1] https://patchwork.freedesktop.org/patch/735301/ Thank you for [1] and the clarificaiton. I agree that [1] addresses the broken host scenario where virtio_gpu_init() times out and probe continues with a pending display_info response. I think there is another scenario causing the same deadlock reported by syzbot and my patch addresses both cases. The deadlock can also occur with the following scenario: after virtio_gpu_init() completes successfully (wait_event_timeout returns), if a config_changed event arrives before drm_client_setup() completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its display_info_cb fires in the dequeue worker and blocks on clientlist_mutex held by drm_client_register(). This is the same deadlock path reported by syzbot. Since [1] only prevents the timeout case, I believe the schedule_work() approach is still needed to prevent display_info_cb from blocking on clientlist_mutex in the dequeue worker context, regardless of the trigger. Best regards, Ryosuke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-03 5:58 ` Ryosuke Yasuoka @ 2026-07-07 15:10 ` Dmitry Osipenko 2026-07-09 12:30 ` Ryosuke Yasuoka 0 siblings, 1 reply; 12+ messages in thread From: Dmitry Osipenko @ 2026-07-07 15:10 UTC (permalink / raw) To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel On 7/3/26 08:58, Ryosuke Yasuoka wrote: > > > On 02/07/2026 14:26, Dmitry Osipenko wrote: >> On 7/1/26 12:23, Ryosuke Yasuoka wrote: >>> >>> >>> On 30/06/2026 16:46, Dmitry Osipenko wrote: >>>> Hi, >>>> >>>> On 6/30/26 12:16, Ryosuke Yasuoka wrote: >>>>> A probe-time deadlock can occur between the dequeue worker and >>>>> drm_client_register(). During probe, drm_client_register() holds >>>>> clientlist_mutex and calls the fbdev hotplug callback, which triggers an >>>>> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() >>>>> waiting for virtqueue space. The dequeue worker that would free that >>>>> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes >>>>> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting >>>>> to acquire the same clientlist_mutex. Since wake_up() is only called >>>>> after the resp_cb loop, the probe thread is never woken and both threads >>>>> deadlock. >>>>> >>>>> Fix this by deferring the hotplug notification from >>>>> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The >>>>> display data (outputs[i].info) is still updated synchronously in the >>>>> callback, and the deferred work only triggers a re-probe notification to >>>>> DRM clients. >>>>> >>>>> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") >>>>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>>>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >>>>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >>>>> --- >>>>> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ >>>>> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ >>>>> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- >>>>> 3 files changed, 16 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>> index 7449907754a4..27ffa4697ae9 100644 >>>>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>> @@ -264,6 +264,8 @@ struct virtio_gpu_device { >>>>> >>>>> struct work_struct config_changed_work; >>>>> >>>>> + struct work_struct hotplug_work; >>>>> + >>>>> struct work_struct obj_free_work; >>>>> spinlock_t obj_free_lock; >>>>> struct list_head obj_free_list; >>>>> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, >>>>> uint32_t x, uint32_t y, >>>>> struct virtio_gpu_object_array *objs, >>>>> struct virtio_gpu_fence *fence); >>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work); >>>>> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, >>>>> uint32_t resource_id, >>>>> uint32_t x, uint32_t y, >>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>> index cfde9f573df6..cfb532ba43a4 100644 >>>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) >>>>> INIT_WORK(&vgdev->config_changed_work, >>>>> virtio_gpu_config_changed_work_func); >>>>> >>>>> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); >>>>> + >>>>> INIT_WORK(&vgdev->obj_free_work, >>>>> virtio_gpu_array_put_free_work); >>>>> INIT_LIST_HEAD(&vgdev->obj_free_list); >>>>> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) >>>>> flush_work(&vgdev->obj_free_work); >>>>> flush_work(&vgdev->ctrlq.dequeue_work); >>>>> flush_work(&vgdev->cursorq.dequeue_work); >>>>> + flush_work(&vgdev->hotplug_work); >>>>> flush_work(&vgdev->config_changed_work); >>>>> virtio_reset_device(vgdev->vdev); >>>>> vgdev->vdev->config->del_vqs(vgdev->vdev); >>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>> index 67865810a2e7..084d98f5dc7b 100644 >>>>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, >>>>> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); >>>>> } >>>>> >>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work) >>>>> +{ >>>>> + struct virtio_gpu_device *vgdev = >>>>> + container_of(work, struct virtio_gpu_device, hotplug_work); >>>>> + >>>>> + if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>>> + drm_kms_helper_hotplug_event(vgdev->ddev); >>>>> +} >>>>> + >>>>> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>>> struct virtio_gpu_vbuffer *vbuf) >>>>> { >>>>> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>>> spin_unlock(&vgdev->display_info_lock); >>>>> wake_up(&vgdev->resp_wq); >>>>> >>>>> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>>> - drm_kms_helper_hotplug_event(vgdev->ddev); >>>>> + schedule_work(&vgdev->hotplug_work); >>>>> } >>>>> >>>>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, >>>> >>> >>> Hi, >>> Thank you for your review. >>> >>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. >>> >>> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >>> into virtio_gpu_init() after wait_event_timeout() would not prevent the >>> issue. >>> >>> Looking at the syzbot call traces[1][2], the deadlock occurs during >>> drm_client_setup(), which runs after virtio_gpu_init() has already >>> returned. The display_info_cb that triggers the deadlock is called from >>> the dequeue worker while drm_client_register() holds clientlist_mutex. >>> >>> Thread A: >>> virtio_gpu_probe() >>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s >> >> You mean that the timeout happens and it's again syzkaller report for a >> broken host. A day ago I applied [1] that should fix this "bogus" >> syzkaller report. >> >> [1] https://patchwork.freedesktop.org/patch/735301/ > > Thank you for [1] and the clarificaiton. I agree that [1] addresses the > broken host scenario where virtio_gpu_init() times out and probe > continues with a pending display_info response. I think there is another > scenario causing the same deadlock reported by syzbot and my patch > addresses both cases. > > The deadlock can also occur with the following scenario: > after virtio_gpu_init() completes successfully (wait_event_timeout > returns), if a config_changed event arrives before drm_client_setup() > completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its > display_info_cb fires in the dequeue worker and blocks on > clientlist_mutex held by drm_client_register(). This is the same > deadlock path reported by syzbot. > > Since [1] only prevents the timeout case, I believe the schedule_work() > approach is still needed to prevent display_info_cb from blocking on > clientlist_mutex in the dequeue worker context, regardless of the > trigger. The config_changed_work_func() already invokes drm_helper_hpd_irq_event() by itself [1]. [1] https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/virtio/virtgpu_kms.c#L52 The drm_helper_hpd_irq_event() itself calls drm_kms_helper_hotplug_event() [2]. [2] https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_probe_helper.c#L1113 Isn't drm_helper_hpd_irq_event() call done by virtio_gpu_cmd_get_display_info_cb() already partially redundant? I.e. only it's needed by virtio_gpu_init() and not by config_changed_work_func(). And then we can move drm_helper_hpd_irq_event() to virtio_gpu_init(): ``` diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 922782bdc9cd..9805283683c1 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -303,6 +303,7 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) virtio_gpu_notify(vgdev); wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending, 5 * HZ); + drm_helper_hpd_irq_event(vgdev->ddev); } vgdev->pm_nb.notifier_call = virtio_gpu_pm_notifier; diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 68d097ad9d1d..aa6440a9b81e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -880,9 +880,6 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, vgdev->display_info_pending = false; spin_unlock(&vgdev->display_info_lock); wake_up(&vgdev->resp_wq); - - if (!drm_helper_hpd_irq_event(vgdev->ddev)) - drm_kms_helper_hotplug_event(vgdev->ddev); } static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, ``` Isn't it a bug that config_changed_work_func() doesn't wait for display_info_pending event? ``` diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 922782bdc9cd..dd2d5c896c39 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -50,6 +50,8 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work) virtio_gpu_cmd_get_edids(vgdev); virtio_gpu_cmd_get_display_info(vgdev); virtio_gpu_notify(vgdev); + wait_event_timeout(vgdev->resp_wq, !vgdev->display_info_pending, + 5 * HZ); drm_helper_hpd_irq_event(vgdev->ddev); } events_clear |= VIRTIO_GPU_EVENT_DISPLAY; ``` -- Best regards, Dmitry ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-07 15:10 ` Dmitry Osipenko @ 2026-07-09 12:30 ` Ryosuke Yasuoka 2026-07-09 12:55 ` Dmitry Osipenko 0 siblings, 1 reply; 12+ messages in thread From: Ryosuke Yasuoka @ 2026-07-09 12:30 UTC (permalink / raw) To: Dmitry Osipenko, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel On 07/07/2026 18:10, Dmitry Osipenko wrote: > On 7/3/26 08:58, Ryosuke Yasuoka wrote: >> >> >> On 02/07/2026 14:26, Dmitry Osipenko wrote: >>> On 7/1/26 12:23, Ryosuke Yasuoka wrote: >>>> >>>> >>>> On 30/06/2026 16:46, Dmitry Osipenko wrote: >>>>> Hi, >>>>> >>>>> On 6/30/26 12:16, Ryosuke Yasuoka wrote: >>>>>> A probe-time deadlock can occur between the dequeue worker and >>>>>> drm_client_register(). During probe, drm_client_register() holds >>>>>> clientlist_mutex and calls the fbdev hotplug callback, which triggers an >>>>>> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() >>>>>> waiting for virtqueue space. The dequeue worker that would free that >>>>>> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes >>>>>> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting >>>>>> to acquire the same clientlist_mutex. Since wake_up() is only called >>>>>> after the resp_cb loop, the probe thread is never woken and both threads >>>>>> deadlock. >>>>>> >>>>>> Fix this by deferring the hotplug notification from >>>>>> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The >>>>>> display data (outputs[i].info) is still updated synchronously in the >>>>>> callback, and the deferred work only triggers a re-probe notification to >>>>>> DRM clients. >>>>>> >>>>>> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") >>>>>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>>>>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >>>>>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >>>>>> --- >>>>>> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ >>>>>> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ >>>>>> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- >>>>>> 3 files changed, 16 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>>> index 7449907754a4..27ffa4697ae9 100644 >>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>>> @@ -264,6 +264,8 @@ struct virtio_gpu_device { >>>>>> >>>>>> struct work_struct config_changed_work; >>>>>> >>>>>> + struct work_struct hotplug_work; >>>>>> + >>>>>> struct work_struct obj_free_work; >>>>>> spinlock_t obj_free_lock; >>>>>> struct list_head obj_free_list; >>>>>> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, >>>>>> uint32_t x, uint32_t y, >>>>>> struct virtio_gpu_object_array *objs, >>>>>> struct virtio_gpu_fence *fence); >>>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work); >>>>>> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, >>>>>> uint32_t resource_id, >>>>>> uint32_t x, uint32_t y, >>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>>> index cfde9f573df6..cfb532ba43a4 100644 >>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>>> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) >>>>>> INIT_WORK(&vgdev->config_changed_work, >>>>>> virtio_gpu_config_changed_work_func); >>>>>> >>>>>> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); >>>>>> + >>>>>> INIT_WORK(&vgdev->obj_free_work, >>>>>> virtio_gpu_array_put_free_work); >>>>>> INIT_LIST_HEAD(&vgdev->obj_free_list); >>>>>> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) >>>>>> flush_work(&vgdev->obj_free_work); >>>>>> flush_work(&vgdev->ctrlq.dequeue_work); >>>>>> flush_work(&vgdev->cursorq.dequeue_work); >>>>>> + flush_work(&vgdev->hotplug_work); >>>>>> flush_work(&vgdev->config_changed_work); >>>>>> virtio_reset_device(vgdev->vdev); >>>>>> vgdev->vdev->config->del_vqs(vgdev->vdev); >>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>>> index 67865810a2e7..084d98f5dc7b 100644 >>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>>> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, >>>>>> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); >>>>>> } >>>>>> >>>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work) >>>>>> +{ >>>>>> + struct virtio_gpu_device *vgdev = >>>>>> + container_of(work, struct virtio_gpu_device, hotplug_work); >>>>>> + >>>>>> + if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>>>> + drm_kms_helper_hotplug_event(vgdev->ddev); >>>>>> +} >>>>>> + >>>>>> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>>>> struct virtio_gpu_vbuffer *vbuf) >>>>>> { >>>>>> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>>>> spin_unlock(&vgdev->display_info_lock); >>>>>> wake_up(&vgdev->resp_wq); >>>>>> >>>>>> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>>>> - drm_kms_helper_hotplug_event(vgdev->ddev); >>>>>> + schedule_work(&vgdev->hotplug_work); >>>>>> } >>>>>> >>>>>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, >>>>> >>>> >>>> Hi, >>>> Thank you for your review. >>>> >>>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. >>>> >>>> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >>>> into virtio_gpu_init() after wait_event_timeout() would not prevent the >>>> issue. >>>> >>>> Looking at the syzbot call traces[1][2], the deadlock occurs during >>>> drm_client_setup(), which runs after virtio_gpu_init() has already >>>> returned. The display_info_cb that triggers the deadlock is called from >>>> the dequeue worker while drm_client_register() holds clientlist_mutex. >>>> >>>> Thread A: >>>> virtio_gpu_probe() >>>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s >>> >>> You mean that the timeout happens and it's again syzkaller report for a >>> broken host. A day ago I applied [1] that should fix this "bogus" >>> syzkaller report. >>> >>> [1] https://patchwork.freedesktop.org/patch/735301/ >> >> Thank you for [1] and the clarificaiton. I agree that [1] addresses the >> broken host scenario where virtio_gpu_init() times out and probe >> continues with a pending display_info response. I think there is another >> scenario causing the same deadlock reported by syzbot and my patch >> addresses both cases. >> >> The deadlock can also occur with the following scenario: >> after virtio_gpu_init() completes successfully (wait_event_timeout >> returns), if a config_changed event arrives before drm_client_setup() >> completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its >> display_info_cb fires in the dequeue worker and blocks on >> clientlist_mutex held by drm_client_register(). This is the same >> deadlock path reported by syzbot. >> >> Since [1] only prevents the timeout case, I believe the schedule_work() >> approach is still needed to prevent display_info_cb from blocking on >> clientlist_mutex in the dequeue worker context, regardless of the >> trigger. > > The config_changed_work_func() already invokes > drm_helper_hpd_irq_event() by itself [1]. > > [1] > https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/virtio/virtgpu_kms.c#L52 > > The drm_helper_hpd_irq_event() itself calls > drm_kms_helper_hotplug_event() [2]. > > [2] > https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_probe_helper.c#L1113 > > Isn't drm_helper_hpd_irq_event() call done by > virtio_gpu_cmd_get_display_info_cb() already partially redundant? I.e. > only it's needed by virtio_gpu_init() and not by > config_changed_work_func(). And then we can move > drm_helper_hpd_irq_event() to virtio_gpu_init(): > > ``` > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c > b/drivers/gpu/drm/virtio/virtgpu_kms.c > index 922782bdc9cd..9805283683c1 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c > @@ -303,6 +303,7 @@ int virtio_gpu_init(struct virtio_device *vdev, > struct drm_device *dev) > virtio_gpu_notify(vgdev); > wait_event_timeout(vgdev->resp_wq, > !vgdev->display_info_pending, > 5 * HZ); > + drm_helper_hpd_irq_event(vgdev->ddev); > } > > vgdev->pm_nb.notifier_call = virtio_gpu_pm_notifier; > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c > b/drivers/gpu/drm/virtio/virtgpu_vq.c > index 68d097ad9d1d..aa6440a9b81e 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c > @@ -880,9 +880,6 @@ static void > virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, > vgdev->display_info_pending = false; > spin_unlock(&vgdev->display_info_lock); > wake_up(&vgdev->resp_wq); > - > - if (!drm_helper_hpd_irq_event(vgdev->ddev)) > - drm_kms_helper_hotplug_event(vgdev->ddev); > } > > static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device > *vgdev, > ``` > > Isn't it a bug that config_changed_work_func() doesn't wait for > display_info_pending event? > > > ``` > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c > b/drivers/gpu/drm/virtio/virtgpu_kms.c > index 922782bdc9cd..dd2d5c896c39 100644 > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c > @@ -50,6 +50,8 @@ static void virtio_gpu_config_changed_work_func(struct > work_struct *work) > virtio_gpu_cmd_get_edids(vgdev); > virtio_gpu_cmd_get_display_info(vgdev); > virtio_gpu_notify(vgdev); > + wait_event_timeout(vgdev->resp_wq, > !vgdev->display_info_pending, > + 5 * HZ); > drm_helper_hpd_irq_event(vgdev->ddev); > } > events_clear |= VIRTIO_GPU_EVENT_DISPLAY; > > ``` Thank you for your suggestion. I agree that removing the hotplug calls from display_info_cb and waiting for the response in config_changed_work_func() and virtio_gpu_init() is a cleaner approach. I'll send a v2 based on this. Best regards, Ryosuke ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock 2026-07-09 12:30 ` Ryosuke Yasuoka @ 2026-07-09 12:55 ` Dmitry Osipenko 0 siblings, 0 replies; 12+ messages in thread From: Dmitry Osipenko @ 2026-07-09 12:55 UTC (permalink / raw) To: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh, Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas Cc: dri-devel, virtualization, linux-kernel On 7/9/26 15:30, Ryosuke Yasuoka wrote: > > > On 07/07/2026 18:10, Dmitry Osipenko wrote: >> On 7/3/26 08:58, Ryosuke Yasuoka wrote: >>> >>> >>> On 02/07/2026 14:26, Dmitry Osipenko wrote: >>>> On 7/1/26 12:23, Ryosuke Yasuoka wrote: >>>>> >>>>> >>>>> On 30/06/2026 16:46, Dmitry Osipenko wrote: >>>>>> Hi, >>>>>> >>>>>> On 6/30/26 12:16, Ryosuke Yasuoka wrote: >>>>>>> A probe-time deadlock can occur between the dequeue worker and >>>>>>> drm_client_register(). During probe, drm_client_register() holds >>>>>>> clientlist_mutex and calls the fbdev hotplug callback, which triggers an >>>>>>> atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs() >>>>>>> waiting for virtqueue space. The dequeue worker that would free that >>>>>>> space calls virtio_gpu_cmd_get_display_info_cb(), which invokes >>>>>>> drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting >>>>>>> to acquire the same clientlist_mutex. Since wake_up() is only called >>>>>>> after the resp_cb loop, the probe thread is never woken and both threads >>>>>>> deadlock. >>>>>>> >>>>>>> Fix this by deferring the hotplug notification from >>>>>>> virtio_gpu_cmd_get_display_info_cb() to a separate work item. The >>>>>>> display data (outputs[i].info) is still updated synchronously in the >>>>>>> callback, and the deferred work only triggers a re-probe notification to >>>>>>> DRM clients. >>>>>>> >>>>>>> Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client") >>>>>>> Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224 >>>>>>> Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e >>>>>>> Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com> >>>>>>> --- >>>>>>> drivers/gpu/drm/virtio/virtgpu_drv.h | 3 +++ >>>>>>> drivers/gpu/drm/virtio/virtgpu_kms.c | 3 +++ >>>>>>> drivers/gpu/drm/virtio/virtgpu_vq.c | 12 ++++++++++-- >>>>>>> 3 files changed, 16 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>>>> index 7449907754a4..27ffa4697ae9 100644 >>>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h >>>>>>> @@ -264,6 +264,8 @@ struct virtio_gpu_device { >>>>>>> >>>>>>> struct work_struct config_changed_work; >>>>>>> >>>>>>> + struct work_struct hotplug_work; >>>>>>> + >>>>>>> struct work_struct obj_free_work; >>>>>>> spinlock_t obj_free_lock; >>>>>>> struct list_head obj_free_list; >>>>>>> @@ -350,6 +352,7 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev, >>>>>>> uint32_t x, uint32_t y, >>>>>>> struct virtio_gpu_object_array *objs, >>>>>>> struct virtio_gpu_fence *fence); >>>>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work); >>>>>>> void virtio_gpu_panic_cmd_resource_flush(struct virtio_gpu_device *vgdev, >>>>>>> uint32_t resource_id, >>>>>>> uint32_t x, uint32_t y, >>>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>>>> index cfde9f573df6..cfb532ba43a4 100644 >>>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >>>>>>> @@ -154,6 +154,8 @@ int virtio_gpu_init(struct virtio_device *vdev, struct drm_device *dev) >>>>>>> INIT_WORK(&vgdev->config_changed_work, >>>>>>> virtio_gpu_config_changed_work_func); >>>>>>> >>>>>>> + INIT_WORK(&vgdev->hotplug_work, virtio_gpu_hotplug_work_func); >>>>>>> + >>>>>>> INIT_WORK(&vgdev->obj_free_work, >>>>>>> virtio_gpu_array_put_free_work); >>>>>>> INIT_LIST_HEAD(&vgdev->obj_free_list); >>>>>>> @@ -293,6 +295,7 @@ void virtio_gpu_deinit(struct drm_device *dev) >>>>>>> flush_work(&vgdev->obj_free_work); >>>>>>> flush_work(&vgdev->ctrlq.dequeue_work); >>>>>>> flush_work(&vgdev->cursorq.dequeue_work); >>>>>>> + flush_work(&vgdev->hotplug_work); >>>>>>> flush_work(&vgdev->config_changed_work); >>>>>>> virtio_reset_device(vgdev->vdev); >>>>>>> vgdev->vdev->config->del_vqs(vgdev->vdev); >>>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>>>> index 67865810a2e7..084d98f5dc7b 100644 >>>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >>>>>>> @@ -816,6 +816,15 @@ virtio_gpu_cmd_resource_detach_backing(struct virtio_gpu_device *vgdev, >>>>>>> virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, fence); >>>>>>> } >>>>>>> >>>>>>> +void virtio_gpu_hotplug_work_func(struct work_struct *work) >>>>>>> +{ >>>>>>> + struct virtio_gpu_device *vgdev = >>>>>>> + container_of(work, struct virtio_gpu_device, hotplug_work); >>>>>>> + >>>>>>> + if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>>>>> + drm_kms_helper_hotplug_event(vgdev->ddev); >>>>>>> +} >>>>>>> + >>>>>>> static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>>>>> struct virtio_gpu_vbuffer *vbuf) >>>>>>> { >>>>>>> @@ -841,8 +850,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >>>>>>> spin_unlock(&vgdev->display_info_lock); >>>>>>> wake_up(&vgdev->resp_wq); >>>>>>> >>>>>>> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >>>>>>> - drm_kms_helper_hotplug_event(vgdev->ddev); >>>>>>> + schedule_work(&vgdev->hotplug_work); >>>>>>> } >>>>>>> >>>>>>> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev, >>>>>> >>>>> >>>>> Hi, >>>>> Thank you for your review. >>>>> >>>>>> Could you please move drm_kms_helper_hotplug_event() to virtio_gpu_init(), placing it after wait_event_timeout(display_info_pending)? This will avoid additional work_struct that otherwise needs to be cancelled in virtio_gpu_init() on the timeout. >>>>> >>>>> IIUC, moving the drm_kms_helper_hotplug_event() and _hpd_irq_event() >>>>> into virtio_gpu_init() after wait_event_timeout() would not prevent the >>>>> issue. >>>>> >>>>> Looking at the syzbot call traces[1][2], the deadlock occurs during >>>>> drm_client_setup(), which runs after virtio_gpu_init() has already >>>>> returned. The display_info_cb that triggers the deadlock is called from >>>>> the dequeue worker while drm_client_register() holds clientlist_mutex. >>>>> >>>>> Thread A: >>>>> virtio_gpu_probe() >>>>> -> virtio_gpu_init() // sends GET_DISPLAY_INFO and waits up to 5s >>>> >>>> You mean that the timeout happens and it's again syzkaller report for a >>>> broken host. A day ago I applied [1] that should fix this "bogus" >>>> syzkaller report. >>>> >>>> [1] https://patchwork.freedesktop.org/patch/735301/ >>> >>> Thank you for [1] and the clarificaiton. I agree that [1] addresses the >>> broken host scenario where virtio_gpu_init() times out and probe >>> continues with a pending display_info response. I think there is another >>> scenario causing the same deadlock reported by syzbot and my patch >>> addresses both cases. >>> >>> The deadlock can also occur with the following scenario: >>> after virtio_gpu_init() completes successfully (wait_event_timeout >>> returns), if a config_changed event arrives before drm_client_setup() >>> completes, config_changed_work_func() sends a new GET_DISPLAY_INFO. Its >>> display_info_cb fires in the dequeue worker and blocks on >>> clientlist_mutex held by drm_client_register(). This is the same >>> deadlock path reported by syzbot. >>> >>> Since [1] only prevents the timeout case, I believe the schedule_work() >>> approach is still needed to prevent display_info_cb from blocking on >>> clientlist_mutex in the dequeue worker context, regardless of the >>> trigger. >> >> The config_changed_work_func() already invokes >> drm_helper_hpd_irq_event() by itself [1]. >> >> [1] >> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/virtio/virtgpu_kms.c#L52 >> >> The drm_helper_hpd_irq_event() itself calls >> drm_kms_helper_hotplug_event() [2]. >> >> [2] >> https://elixir.bootlin.com/linux/v7.1.2/source/drivers/gpu/drm/drm_probe_helper.c#L1113 >> >> Isn't drm_helper_hpd_irq_event() call done by >> virtio_gpu_cmd_get_display_info_cb() already partially redundant? I.e. >> only it's needed by virtio_gpu_init() and not by >> config_changed_work_func(). And then we can move >> drm_helper_hpd_irq_event() to virtio_gpu_init(): >> >> ``` >> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c >> b/drivers/gpu/drm/virtio/virtgpu_kms.c >> index 922782bdc9cd..9805283683c1 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >> @@ -303,6 +303,7 @@ int virtio_gpu_init(struct virtio_device *vdev, >> struct drm_device *dev) >> virtio_gpu_notify(vgdev); >> wait_event_timeout(vgdev->resp_wq, >> !vgdev->display_info_pending, >> 5 * HZ); >> + drm_helper_hpd_irq_event(vgdev->ddev); >> } >> >> vgdev->pm_nb.notifier_call = virtio_gpu_pm_notifier; >> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c >> b/drivers/gpu/drm/virtio/virtgpu_vq.c >> index 68d097ad9d1d..aa6440a9b81e 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c >> @@ -880,9 +880,6 @@ static void >> virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev, >> vgdev->display_info_pending = false; >> spin_unlock(&vgdev->display_info_lock); >> wake_up(&vgdev->resp_wq); >> - >> - if (!drm_helper_hpd_irq_event(vgdev->ddev)) >> - drm_kms_helper_hotplug_event(vgdev->ddev); >> } >> >> static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device >> *vgdev, >> ``` >> >> Isn't it a bug that config_changed_work_func() doesn't wait for >> display_info_pending event? >> >> >> ``` >> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c >> b/drivers/gpu/drm/virtio/virtgpu_kms.c >> index 922782bdc9cd..dd2d5c896c39 100644 >> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c >> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c >> @@ -50,6 +50,8 @@ static void virtio_gpu_config_changed_work_func(struct >> work_struct *work) >> virtio_gpu_cmd_get_edids(vgdev); >> virtio_gpu_cmd_get_display_info(vgdev); >> virtio_gpu_notify(vgdev); >> + wait_event_timeout(vgdev->resp_wq, >> !vgdev->display_info_pending, >> + 5 * HZ); >> drm_helper_hpd_irq_event(vgdev->ddev); >> } >> events_clear |= VIRTIO_GPU_EVENT_DISPLAY; >> >> ``` > > Thank you for your suggestion. I agree that removing the hotplug calls > from display_info_cb and waiting for the response in > config_changed_work_func() and virtio_gpu_init() is a cleaner approach. > I'll send a v2 based on this. Thanks, please also check whether drm_helper_hpd_irq_event() needed at all in in virtio_gpu_init(). Suppose the hotplug event does nothing before DRM device/client has been registered. -- Best regards, Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-07-09 12:56 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-30 9:16 [PATCH] drm/virtio: defer hotplug event from dequeue worker to avoid deadlock Ryosuke Yasuoka 2026-06-30 9:36 ` sashiko-bot 2026-06-30 13:46 ` Dmitry Osipenko 2026-07-01 9:23 ` Ryosuke Yasuoka 2026-07-01 23:43 ` Hillf Danton 2026-07-02 7:12 ` Ryosuke Yasuoka 2026-07-02 8:25 ` Hillf Danton 2026-07-02 11:26 ` Dmitry Osipenko 2026-07-03 5:58 ` Ryosuke Yasuoka 2026-07-07 15:10 ` Dmitry Osipenko 2026-07-09 12:30 ` Ryosuke Yasuoka 2026-07-09 12:55 ` Dmitry Osipenko
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.