On Thu, 2026-02-26 at 21:43 +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]>
The fact that the INIT_WORK points to virtscsi_handle_event(), which itself calls virtscsi_kick_event() and re-inits the workqueue struct today, does seem odd. Moving this to _init, as part of the _probe() process, seems correct to me. One nit below, but FWIW: Reviewed-by: Eric Farman <[email protected]> Tested-by: Eric Farman <[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, Just before this hunk is a prototype for virtscsi_handle_event(), since it was previously used in this function but defined afterwards. I suspect it can be removed now? > 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); > + } > + > err = 0; > > out:
