Re: [PATCH] system/qdev-monitor: move drain_call_rcu call under if (!dev) in qmp_device_add()

[Marc Hartmayer]

On Fri, Nov 03, 2023 at 01:56 PM +0300, Dmitrii Gavrilov 
<ds-gavr@yandex-team.ru> wrote:
> Original goal of addition of drain_call_rcu to qmp_device_add was to cover
> the failure case of qdev_device_add. It seems call of drain_call_rcu was
> misplaced in 7bed89958bfbf40df what led to waiting for pending RCU callbacks
> under happy path too. What led to overall performance degradation of
> qmp_device_add.
>
> In this patch call of drain_call_rcu moved under handling of failure of
> qdev_device_add.
>
> Signed-off-by: Dmitrii Gavrilov <ds-gavr@yandex-team.ru>

I don't know the exact reason, but this commit caused udev events to
show up much slower than before (~3s vs. ~23s) when a virtio-scsi device
is hotplugged (I’ve tested this only on s390x). Importantly, this only
happens when asynchronous SCSI scanning is disabled in the *guest*
kernel (scsi_mod.scan=sync or CONFIG_SCSI_SCAN_ASYNC=n).

The `udevadm monitor` output captured while hotplugging the device
(using QEMU 012b170173bc ("system/qdev-monitor: move drain_call_rcu call
under if (!dev) in qmp_device_add()")):

…
KERNEL[2.166575] add      /devices/css0/0.0.0002/0.0.0002 (ccw)
KERNEL[2.166594] bind     /devices/css0/0.0.0002/0.0.0002 (ccw)
KERNEL[2.166826] add      /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)
UDEV  [2.166846] add      /devices/css0/0.0.0002/0.0.0002 (ccw)
UDEV  [2.167013] bind     /devices/css0/0.0.0002/0.0.0002 (ccw)
KERNEL[2.167560] add      /devices/virtual/workqueue/scsi_tmf_0 (workqueue)
UDEV  [2.167977] add      /devices/virtual/workqueue/scsi_tmf_0 (workqueue)
KERNEL[2.167987] add      /devices/css0/0.0.0002/0.0.0002/virtio2/host0 (scsi)
KERNEL[2.167996] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/scsi_host/host0 (scsi_host)
KERNEL[2.169113] change   /0:0:0:0 (scsi)
UDEV  [2.169212] change   /0:0:0:0 (scsi)
KERNEL[2.199500] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0 (scsi)
KERNEL[2.199513] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
KERNEL[2.199523] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_device/0:0:0:0
 (scsi_device)
KERNEL[2.199532] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_disk/0:0:0:0
 (scsi_disk)
KERNEL[2.199564] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0
 (scsi_generic)
KERNEL[2.199586] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/bsg/0:0:0:0 
(bsg)
KERNEL[2.280482] add      /devices/virtual/bdi/8:0 (bdi)
UDEV  [2.280634] add      /devices/virtual/bdi/8:0 (bdi)
KERNEL[3.060145] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/block/sda 
(block)
KERNEL[3.060160] bind     
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
KERNEL[22.160147] bind     /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)
KERNEL[22.160161] add      /bus/virtio/drivers/virtio_scsi (drivers)
KERNEL[22.160169] add      /module/virtio_scsi (module)
UDEV  [22.161078] add      /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)
UDEV  [22.161339] add      /bus/virtio/drivers/virtio_scsi (drivers)
UDEV  [22.161860] add      /devices/css0/0.0.0002/0.0.0002/virtio2/host0 (scsi)
UDEV  [22.161869] add      /module/virtio_scsi (module)
UDEV  [22.161880] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0 (scsi)
UDEV  [22.161890] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/scsi_host/host0 (scsi_host)
UDEV  [22.161901] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
UDEV  [22.161911] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_disk/0:0:0:0
 (scsi_disk)
UDEV  [22.161924] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/bsg/0:0:0:0 
(bsg)
UDEV  [22.161937] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0
 (scsi_generic)
UDEV  [22.162123] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_device/0:0:0:0
 (scsi_device)
UDEV  [22.468924] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/block/sda 
(block)
UDEV  [22.473955] bind     
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
UDEV  [22.473970] bind     /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)


The `udevadm monitor` output without this commit (QEMU 9876359990dd 
("hw/scsi/lsi53c895a: add timer to scripts processing")):

…
KERNEL[2.091114] add      /devices/virtual/workqueue/scsi_tmf_0 (workqueue)
UDEV  [2.091218] add      /devices/virtual/workqueue/scsi_tmf_0 (workqueue)
KERNEL[2.091408] add      /devices/css0/0.0.0002/0.0.0002/virtio2/host0 (scsi)
KERNEL[2.091418] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/scsi_host/host0 (scsi_host)
KERNEL[2.200461] bind     /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)
KERNEL[2.200473] add      /bus/virtio/drivers/virtio_scsi (drivers)
KERNEL[2.200481] add      /module/virtio_scsi (module)
UDEV  [2.200634] add      /module/virtio_scsi (module)
UDEV  [2.200678] add      /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)
UDEV  [2.200746] add      /bus/virtio/drivers/virtio_scsi (drivers)
UDEV  [2.200830] add      /devices/css0/0.0.0002/0.0.0002/virtio2/host0 (scsi)
UDEV  [2.200972] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/scsi_host/host0 (scsi_host)
UDEV  [2.201148] bind     /devices/css0/0.0.0002/0.0.0002/virtio2 (virtio)
KERNEL[2.201699] change   /0:0:0:0 (scsi)
KERNEL[2.201734] change   /0:0:0:0 (scsi)
UDEV  [2.201815] change   /0:0:0:0 (scsi)
UDEV  [2.201888] change   /0:0:0:0 (scsi)
KERNEL[2.222062] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0 (scsi)
KERNEL[2.222074] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
KERNEL[2.222083] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_device/0:0:0:0
 (scsi_device)
KERNEL[2.222092] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_disk/0:0:0:0
 (scsi_disk)
KERNEL[2.222104] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0
 (scsi_generic)
KERNEL[2.222127] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/bsg/0:0:0:0 
(bsg)
UDEV  [2.222241] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0 (scsi)
UDEV  [2.222486] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
UDEV  [2.222667] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_disk/0:0:0:0
 (scsi_disk)
UDEV  [2.222715] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/bsg/0:0:0:0 
(bsg)
UDEV  [2.222877] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_device/0:0:0:0
 (scsi_device)
UDEV  [2.223116] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/scsi_generic/sg0
 (scsi_generic)
KERNEL[2.303063] add      /devices/virtual/bdi/8:0 (bdi)
UDEV  [2.303197] add      /devices/virtual/bdi/8:0 (bdi)
KERNEL[2.394175] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/block/sda 
(block)
KERNEL[2.394186] bind     
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)
UDEV  [2.706054] add      
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0/block/sda 
(block)
UDEV  [2.706075] bind     
/devices/css0/0.0.0002/0.0.0002/virtio2/host0/target0:0:0/0:0:0:0 (scsi)

I’ve used as host kernel 6.7.0-rc3-00033-ge72f947b4f0d and guest kernel
v6.5.0.

QEMU 'info qtree' output when the device was hotplugged:

bus: main-system-bus
  type System
  dev: s390-pcihost, id ""
    x-config-reg-migration-enabled = true
    bypass-iommu = false
    bus: s390-pcibus.0
      type s390-pcibus
    bus: pci.0
      type PCI
  dev: virtual-css-bridge, id ""
    css_dev_path = true
    bus: virtual-css
      type virtual-css-bus
      dev: virtio-scsi-ccw, id "scsi0"
        ioeventfd = true
        max_revision = 2 (0x2)
        devno = "fe.0.0002"
        dev_id = "fe.0.0002"
        subch_id = "fe.0.0002"
        bus: virtio-bus
          type virtio-ccw-bus
          dev: virtio-scsi-device, id ""
            num_queues = 1 (0x1)
            virtqueue_size = 256 (0x100)
            seg_max_adjust = true
            max_sectors = 65535 (0xffff)
            cmd_per_lun = 128 (0x80)
            hotplug = true
            param_change = true
            indirect_desc = true
            event_idx = true
            notify_on_empty = true
            any_layout = true
            iommu_platform = false
            packed = false
            queue_reset = true
            use-started = true
            use-disabled-flag = true
            x-disable-legacy-check = false
            bus: scsi0.0
              type SCSI
              dev: scsi-generic, id "hostdev0"
                drive = "libvirt-1-backend"
                share-rw = false
                io_timeout = 30 (0x1e)
                channel = 0 (0x0)
                scsi-id = 0 (0x0)
                lun = 0 (0x0)
…

Any ideas?

Thanks in advance.

Kind regards,
 Marc