[PATCH v2 0/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init

[PATCH v2 0/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init

[Joshua Daley]

Changelog v1 -> v2:

- Added 2 additional patches:
  - [PATCH v2 1/3] scsi: virtio_scsi: kick event_list unconditionally
    - Removes the conditions surrounding event_list operations (suggested by Stefan Hajnoczi <stefanha@redhat.com>)
  - [PATCH v2 2/3] scsi: virtio_scsi: remove unnecessary fn declaration
    - Removes virtscsi_handle_event() prototype (suggested by Eric Farman <farman@linux.ibm.com>)

- [PATCH 1/1] -> [PATCH v2 3/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init
  - Removed the condition surrounding INIT_WORK calls

-----

v1 cover letter:

This patch avoids a kernel warning that may occur if a virtio_scsi
controller is detached immediately following a disk detach. See the
commit message for details. The following are instructions to
produce the warning (without the proposed patch).

Timing matters--if all event work items call INIT_WORK before they are
flushed by cancel_work_sync, then the warning will not occur.

The warning will occur consistently if a sleep is added in
virtscsi_kick_event before the INIT_WORK call, like so:

#include <linux/delay.h>

static int virtscsi_kick_event(struct virtio_scsi *vscsi,
			       struct virtio_scsi_event_node *event_node)
{
    int err;
    struct scatterlist sg;
    unsigned long flags;

 -> msleep(1000);
    INIT_WORK(&event_node->work, virtscsi_handle_event);
	
    ...
}

Then, just detach a disk and its controller in quick succession:

virsh detach-device --domain <domain> disk.xml; \
virsh detach-device --domain <domain> controller.xml

where disk.xml and controller.xml are text files containing the XML
of the disk and controller.

Or, with the libvirt python module:

domain.detachDevice(str(disk_xml))
domain.detachDevice(str(controller_xml))

Joshua Daley (3):
  scsi: virtio_scsi: kick event_list unconditionally
  scsi: virtio_scsi: remove unnecessary fn declaration
  scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init

 drivers/scsi/virtio_scsi.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] scsi: virtio_scsi: kick event_list unconditionally

[Joshua Daley]

The event_list processes non-hotplug events (such as LUN capacity
changes), so remove the conditions that guard the initial kicks in
_probe() and _restore(), as well as the work cancellation in _remove().

Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>
---
 drivers/scsi/virtio_scsi.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 0ed8558dad72..982f49bc6c69 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -983,9 +983,7 @@ static int virtscsi_probe(struct virtio_device *vdev)
 		goto scsi_add_host_failed;
 
 	virtio_device_ready(vdev);
-
-	if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
-		virtscsi_kick_event_all(vscsi);
+	virtscsi_kick_event_all(vscsi);
 
 	scsi_scan_host(shost);
 	return 0;
@@ -1002,8 +1000,7 @@ static void virtscsi_remove(struct virtio_device *vdev)
 	struct Scsi_Host *shost = virtio_scsi_host(vdev);
 	struct virtio_scsi *vscsi = shost_priv(shost);
 
-	if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
-		virtscsi_cancel_event_work(vscsi);
+	virtscsi_cancel_event_work(vscsi);
 
 	scsi_remove_host(shost);
 	virtscsi_remove_vqs(vdev);
@@ -1028,9 +1025,7 @@ static int virtscsi_restore(struct virtio_device *vdev)
 		return err;
 
 	virtio_device_ready(vdev);
-
-	if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
-		virtscsi_kick_event_all(vscsi);
+	virtscsi_kick_event_all(vscsi);
 
 	return err;
 }
-- 
2.34.1

[PATCH v2 2/3] scsi: virtio_scsi: remove unnecessary fn declaration

[Joshua Daley]

virtscsi_handle_event() is not used before its definition, so remove
a prior declaration.

Suggested-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>
---
 drivers/scsi/virtio_scsi.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 982f49bc6c69..6efbeaa30f65 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -233,8 +233,6 @@ static void virtscsi_ctrl_done(struct virtqueue *vq)
 	virtscsi_vq_done(vscsi, &vscsi->ctrl_vq, virtscsi_complete_free);
 };
 
-static void virtscsi_handle_event(struct work_struct *work);
-
 static int virtscsi_kick_event(struct virtio_scsi *vscsi,
 			       struct virtio_scsi_event_node *event_node)
 {
-- 
2.34.1

[PATCH v2 3/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init

[Joshua Daley]

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 <jdaley@linux.ibm.com>
---
 drivers/scsi/virtio_scsi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 6efbeaa30f65..fe1bf37f1317 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -240,7 +240,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);
@@ -896,6 +895,9 @@ 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);
 
+	for (i = 0; i < VIRTIO_SCSI_EVENT_LEN; i++)
+		INIT_WORK(&vscsi->event_list[i].work, virtscsi_handle_event);
+
 	err = 0;
 
 out:
-- 
2.34.1

Re: [PATCH v2 0/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2361 bytes --]

On Thu, Mar 12, 2026 at 06:42:53PM +0100, Joshua Daley wrote:
> Changelog v1 -> v2:
> 
> - Added 2 additional patches:
>   - [PATCH v2 1/3] scsi: virtio_scsi: kick event_list unconditionally
>     - Removes the conditions surrounding event_list operations (suggested by Stefan Hajnoczi <stefanha@redhat.com>)
>   - [PATCH v2 2/3] scsi: virtio_scsi: remove unnecessary fn declaration
>     - Removes virtscsi_handle_event() prototype (suggested by Eric Farman <farman@linux.ibm.com>)
> 
> - [PATCH 1/1] -> [PATCH v2 3/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init
>   - Removed the condition surrounding INIT_WORK calls
> 
> -----
> 
> v1 cover letter:
> 
> This patch avoids a kernel warning that may occur if a virtio_scsi
> controller is detached immediately following a disk detach. See the
> commit message for details. The following are instructions to
> produce the warning (without the proposed patch).
> 
> Timing matters--if all event work items call INIT_WORK before they are
> flushed by cancel_work_sync, then the warning will not occur.
> 
> The warning will occur consistently if a sleep is added in
> virtscsi_kick_event before the INIT_WORK call, like so:
> 
> #include <linux/delay.h>
> 
> static int virtscsi_kick_event(struct virtio_scsi *vscsi,
> 			       struct virtio_scsi_event_node *event_node)
> {
>     int err;
>     struct scatterlist sg;
>     unsigned long flags;
> 
>  -> msleep(1000);
>     INIT_WORK(&event_node->work, virtscsi_handle_event);
> 	
>     ...
> }
> 
> Then, just detach a disk and its controller in quick succession:
> 
> virsh detach-device --domain <domain> disk.xml; \
> virsh detach-device --domain <domain> controller.xml
> 
> where disk.xml and controller.xml are text files containing the XML
> of the disk and controller.
> 
> Or, with the libvirt python module:
> 
> domain.detachDevice(str(disk_xml))
> domain.detachDevice(str(controller_xml))
> 
> Joshua Daley (3):
>   scsi: virtio_scsi: kick event_list unconditionally
>   scsi: virtio_scsi: remove unnecessary fn declaration
>   scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init
> 
>  drivers/scsi/virtio_scsi.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> -- 
> 2.34.1
> 

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 2/3] scsi: virtio_scsi: remove unnecessary fn declaration

[Matthew Rosato]

On 3/12/26 1:42 PM, Joshua Daley wrote:
> virtscsi_handle_event() is not used before its definition, so remove
> a prior declaration.
> 
> Suggested-by: Eric Farman <farman@linux.ibm.com>
> Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>
> ---
>  drivers/scsi/virtio_scsi.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 982f49bc6c69..6efbeaa30f65 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -233,8 +233,6 @@ static void virtscsi_ctrl_done(struct virtqueue *vq)
>  	virtscsi_vq_done(vscsi, &vscsi->ctrl_vq, virtscsi_complete_free);
>  };
>  
> -static void virtscsi_handle_event(struct work_struct *work);
> -

Hi Josh,

You can't make this change until after patch 3 where you move the reference to virtscsi_handle_event further down.

In other words, if you just apply patch 1 + this patch you will get:

drivers/scsi/virtio_scsi.c:383:13: warning: ‘virtscsi_handle_event’ defined but not used [-Wunused-function]

until you also apply patch 3.  This breaks bisectability.

Please either re-arrange this series so that this is the last patch OR squash patch 2 + 3 together.

If you choose the latter approach and keep this patch then you can also include:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

>  static int virtscsi_kick_event(struct virtio_scsi *vscsi,
>  			       struct virtio_scsi_event_node *event_node)
>  {

Re: [PATCH v2 1/3] scsi: virtio_scsi: kick event_list unconditionally

[Matthew Rosato]

On 3/12/26 1:42 PM, Joshua Daley wrote:
> The event_list processes non-hotplug events (such as LUN capacity
> changes), so remove the conditions that guard the initial kicks in
> _probe() and _restore(), as well as the work cancellation in _remove().
> 
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

> ---
>  drivers/scsi/virtio_scsi.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index 0ed8558dad72..982f49bc6c69 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -983,9 +983,7 @@ static int virtscsi_probe(struct virtio_device *vdev)
>  		goto scsi_add_host_failed;
>  
>  	virtio_device_ready(vdev);
> -
> -	if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
> -		virtscsi_kick_event_all(vscsi);
> +	virtscsi_kick_event_all(vscsi);
>  
>  	scsi_scan_host(shost);
>  	return 0;
> @@ -1002,8 +1000,7 @@ static void virtscsi_remove(struct virtio_device *vdev)
>  	struct Scsi_Host *shost = virtio_scsi_host(vdev);
>  	struct virtio_scsi *vscsi = shost_priv(shost);
>  
> -	if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
> -		virtscsi_cancel_event_work(vscsi);
> +	virtscsi_cancel_event_work(vscsi);
>  
>  	scsi_remove_host(shost);
>  	virtscsi_remove_vqs(vdev);
> @@ -1028,9 +1025,7 @@ static int virtscsi_restore(struct virtio_device *vdev)
>  		return err;
>  
>  	virtio_device_ready(vdev);
> -
> -	if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG))
> -		virtscsi_kick_event_all(vscsi);
> +	virtscsi_kick_event_all(vscsi);
>  
>  	return err;
>  }

Re: [PATCH v2 3/3] scsi: virtio_scsi: move INIT_WORK calls to virtscsi_init

[Matthew Rosato]

On 3/12/26 1:42 PM, 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 <jdaley@linux.ibm.com>

With patch 2 either squashed in OR moved after this one:

Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>

Re: [PATCH v2 2/3] scsi: virtio_scsi: remove unnecessary fn declaration

[Joshua Daley]

On 3/16/2026 9:55 AM, Matthew Rosato wrote:
> On 3/12/26 1:42 PM, Joshua Daley wrote:
>> virtscsi_handle_event() is not used before its definition, so remove
>> a prior declaration.
>>
>> Suggested-by: Eric Farman <farman@linux.ibm.com>
>> Signed-off-by: Joshua Daley <jdaley@linux.ibm.com>
>> ---
>>   drivers/scsi/virtio_scsi.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
>> index 982f49bc6c69..6efbeaa30f65 100644
>> --- a/drivers/scsi/virtio_scsi.c
>> +++ b/drivers/scsi/virtio_scsi.c
>> @@ -233,8 +233,6 @@ static void virtscsi_ctrl_done(struct virtqueue *vq)
>>   	virtscsi_vq_done(vscsi, &vscsi->ctrl_vq, virtscsi_complete_free);
>>   };
>>   
>> -static void virtscsi_handle_event(struct work_struct *work);
>> -
> 
> Hi Josh,
> 
> You can't make this change until after patch 3 where you move the reference to virtscsi_handle_event further down.
> 
> In other words, if you just apply patch 1 + this patch you will get:
> 
> drivers/scsi/virtio_scsi.c:383:13: warning: ‘virtscsi_handle_event’ defined but not used [-Wunused-function]
> 
> until you also apply patch 3.  This breaks bisectability.
> 
> Please either re-arrange this series so that this is the last patch OR squash patch 2 + 3 together.
> 
> If you choose the latter approach and keep this patch then you can also include:
> 
> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com>
> 

Thanks for catching this. I'll re-arrange the series so that this is the last patch.

>>   static int virtscsi_kick_event(struct virtio_scsi *vscsi,
>>   			       struct virtio_scsi_event_node *event_node)
>>   {
>