Re: [PATCH 1/1] virtio-blk-ccw: tweak the default for num_queues

[Christian Borntraeger]

On 09.11.20 16:48, Halil Pasic wrote:
> Currently the default value of num_queues is effectively 1 for
> virtio-blk-ccw. Recently 9445e1e15e ("virtio-blk-pci: default num_queues
> to -smp N") changed the default for pci to the number of vcpus, citing
> interrupt better locality and better performance as a rationale.
> 
> While virtio-blk-ccw does not yet benefit from better interrupt
> locality, measurements have shown that for secure VMs multiqueue does
> help with performance. Since the bounce buffering happens with the queue
> lock held (in the guest) this is hardly a surprise.
> 
> As for non-secure VMs the extra queues shouldn't hurt too much.

In fact we have also seen improvments for multiqueues for non secure guests
as one virtqueue seems to have a hard capping in terms of bandwidth that can
be smaller than newer storage devices.

> 
> Suggested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> ---
> 
> We would prefer to land this for 5.2. If we do then commit 9445e1e15e
> ("virtio-blk-pci: default num_queues to -smp N") took care of all the
> necessary compat handling.
> 
> If that's not possible, I will send a version that does the necessary
> compat handling.

Right. At the moment the original patch has all the necessary compat handling
for 5.1 for all platforms. Adding multi-queue for s390x for > 5.2 would mean
that we would need to have additional platform specific compat handling and it
would increase the patch size unnecessarily. 
In the end you could consider this an enhancement (not calling it fix) of the
original patch.

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  hw/s390x/virtio-ccw-blk.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/s390x/virtio-ccw-blk.c b/hw/s390x/virtio-ccw-blk.c
> index 2294ce1ce4..7296140dde 100644
> --- a/hw/s390x/virtio-ccw-blk.c
> +++ b/hw/s390x/virtio-ccw-blk.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "hw/boards.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/virtio/virtio.h"
>  #include "qapi/error.h"
> @@ -20,6 +21,11 @@ static void virtio_ccw_blk_realize(VirtioCcwDevice 
> *ccw_dev, Error **errp)
>  {
>      VirtIOBlkCcw *dev = VIRTIO_BLK_CCW(ccw_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
> +    VirtIOBlkConf *conf = &dev->vdev.conf;
> +
> +    if (conf->num_queues == VIRTIO_BLK_AUTO_NUM_QUEUES) {
> +        conf->num_queues = MIN(4, current_machine->smp.cpus);
> +    }
>  
>      qdev_realize(vdev, BUS(&ccw_dev->bus), errp);
>  }
> 
> base-commit: 2a190a7256a3e0563b29ffd67e0164097b4a6dac
>