On 3/8/2026 11:06 PM, Stefan Hajnoczi wrote:
On Thu, Feb 26, 2026 at 09:43:45PM +0100, Joshua Daley wrote:
The last step of virtscsi_handle_event is to call virtscsi_kick_event,
which calls INIT_WORK on it's own work item. INIT_WORK resets the
work item's data bits to 0.

If this occurs while the work item is being flushed by
cancel_work_sync, then kernel/workqueue.c/work_offqd_enable triggers a
kernel warning, as it expects the "disable" bit to be 1:

[   21.450115] workqueue: work disable count underflowed
[   21.450117] WARNING: CPU: 1 PID: 56 at kernel/workqueue.c:4328 
enable_work+0x10a/0x120
...
[   21.450171] Call Trace:
[   21.450173]  [<000003db2e5bdc3e>] enable_work+0x10e/0x120
[   21.450176] ([<000003db2e5bdc3a>] enable_work+0x10a/0x120)
[   21.450178]  [<000003db2e5bdd86>] cancel_work_sync+0x86/0xa0
[   21.450181]  [<000003daae97d9e4>] virtscsi_remove+0xb4/0xd0 [virtio_scsi]
[   21.450184]  [<000003db2ef3b5ca>] virtio_dev_remove+0x6a/0xd0
[   21.450186]  [<000003db2ef9106c>] device_release_driver_internal+0x1ac/0x260
[   21.450190]  [<000003db2ef8edc8>] bus_remove_device+0xf8/0x190
[   21.450192]  [<000003db2ef88d72>] device_del+0x142/0x340
[   21.450194]  [<000003db2ef88fa0>] device_unregister+0x30/0xa0
[   21.450196]  [<000003db2ef3b2fa>] unregister_virtio_device+0x2a/0x40

This warning may occur if a controller is detached immediately
following a disk detach.

Move the INIT_WORK call to prevent this. Don't re-init event list
work items in virtscsi_kick_event, init them only once in
virtscsi_init instead.

Signed-off-by: Joshua Daley <[email protected]>
---
  drivers/scsi/virtio_scsi.c | 6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0ed8558dad72..173092931df6 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -242,7 +242,6 @@ static int virtscsi_kick_event(struct virtio_scsi *vscsi,
        struct scatterlist sg;
        unsigned long flags;
- INIT_WORK(&event_node->work, virtscsi_handle_event);
        sg_init_one(&sg, event_node->event, sizeof(struct virtio_scsi_event));
spin_lock_irqsave(&vscsi->event_vq.vq_lock, flags);
@@ -898,6 +897,11 @@ static int virtscsi_init(struct virtio_device *vdev,
        virtscsi_config_set(vdev, cdb_size, VIRTIO_SCSI_CDB_SIZE);
        virtscsi_config_set(vdev, sense_size, VIRTIO_SCSI_SENSE_SIZE);
+ if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
+               for (i = 0; i < VIRTIO_SCSI_EVENT_LEN; i++)
+                       INIT_WORK(&vscsi->event_list[i].work, 
virtscsi_handle_event);
+       }

The eventq should be populated unconditionally so that non-hotplug
events are processed even when F_HOTPLUG is not negotiated. For example,
LUN capacity changes are reported via the VIRTIO_SCSI_T_PARAM_CHANGE
event. LUN capacity changes depend on F_CHANGE, not F_HOTPLUG.

There is a related bug here: the other if (virtio_has_feature(vdev,
VIRTIO_SCSI_F_HOTPLUG)) conditionals in this file need to be revisited
so that LUN capacity changes are reported even when F_HOTPLUG is not
negotiated. You can test this bug with QEMU's -device
virtio-scsi-pci,hotplug=off parameter and the 'block_resize' QEMU
monitor command.

Do you want to write a patch or do you want me to send a follow-up?

Thanks,
Stefan

I can write a patch. Thanks for your review.

There are 3 other if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) 
conditionals in this file, for:

1. virtscsi_kick_event_all() called in virtscsi_probe()
2. virtscsi_cancel_event_work() called in virtscsi_remove()
3. virtscsi_kick_event_all() called in virtscsi_restore()

Should the eventq be populated truly unconditionally? Then I would just remove the conditions from these calls. Or would it be better to just change the conditions to also check the other feature:
if (... || virtio_has_feature(vdev, VIRTIO_SCSI_F_CHANGE))

Reply via email to