diff mbox series

[v4,5/5] vhost-user-blk: default num_queues to -smp N

Message ID 20200527102925.128013-6-stefanha@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtio-pci: enable blk and scsi multi-queue by default | expand

Commit Message

Stefan Hajnoczi May 27, 2020, 10:29 a.m. UTC 
Automatically size the number of request virtqueues to match the number
of vCPUs.  This ensures that completion interrupts are handled on the
same vCPU that submitted the request.  No IPI is necessary to complete
an I/O request and performance is improved.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
---
 include/hw/virtio/vhost-user-blk.h | 2 ++
 hw/block/vhost-user-blk.c          | 6 +++++-
 hw/core/machine.c                  | 1 +
 hw/virtio/vhost-user-blk-pci.c     | 4 ++++
 4 files changed, 12 insertions(+), 1 deletion(-)

Comments

Raphael Norwitz May 31, 2020, 2:42 a.m. UTC | #1 
I'm happy with the code but as David pointed out with virtio-scsi, we
should probably add a comment about virtio_pci_optimal_num_queues()
capping the number of VQs here too.


On Wed, May 27, 2020 at 6:34 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> Automatically size the number of request virtqueues to match the number
> of vCPUs.  This ensures that completion interrupts are handled on the
> same vCPU that submitted the request.  No IPI is necessary to complete
> an I/O request and performance is improved.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  include/hw/virtio/vhost-user-blk.h | 2 ++
>  hw/block/vhost-user-blk.c          | 6 +++++-
>  hw/core/machine.c                  | 1 +
>  hw/virtio/vhost-user-blk-pci.c     | 4 ++++
>  4 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
> index 34ad6f0c0e..292d17147c 100644
> --- a/include/hw/virtio/vhost-user-blk.h
> +++ b/include/hw/virtio/vhost-user-blk.h
> @@ -25,6 +25,8 @@
>  #define VHOST_USER_BLK(obj) \
>          OBJECT_CHECK(VHostUserBlk, (obj), TYPE_VHOST_USER_BLK)
>
> +#define VHOST_USER_BLK_AUTO_NUM_QUEUES UINT16_MAX
> +
>  typedef struct VHostUserBlk {
>      VirtIODevice parent_obj;
>      CharBackend chardev;
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9d8c0b3909..7a8639516f 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -385,6 +385,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    if (s->num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> +        s->num_queues = 1;
> +    }
>      if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
>          error_setg(errp, "vhost-user-blk: invalid number of IO queues");
>          return;
> @@ -496,7 +499,8 @@ static const VMStateDescription vmstate_vhost_user_blk = {
>
>  static Property vhost_user_blk_properties[] = {
>      DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> -    DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues, 1),
> +    DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
> +                       VHOST_USER_BLK_AUTO_NUM_QUEUES),
>      DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
>      DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
>      DEFINE_PROP_END_OF_LIST(),
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 4aba3bdd3c..8cc4b54eec 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -32,6 +32,7 @@ GlobalProperty hw_compat_5_0[] = {
>      { "virtio-blk-device", "num-queues", "1"},
>      { "virtio-scsi-device", "num_queues", "1"},
>      { "vhost-scsi", "num_queues", "1"},
> +    { "vhost-user-blk", "num-queues", "1"},
>      { "vhost-user-scsi", "num_queues", "1"},
>  };
>  const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
> diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
> index 58d7c31735..1c8ab7f5e6 100644
> --- a/hw/virtio/vhost-user-blk-pci.c
> +++ b/hw/virtio/vhost-user-blk-pci.c
> @@ -54,6 +54,10 @@ static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>      VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
>      DeviceState *vdev = DEVICE(&dev->vdev);
>
> +    if (dev->vdev.num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> +        dev->vdev.num_queues = virtio_pci_optimal_num_queues(0);
> +    }
> +
>      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
>          vpci_dev->nvectors = dev->vdev.num_queues + 1;
>      }
> --
> 2.25.4
>
Michael S. Tsirkin June 9, 2020, 3:36 p.m. UTC | #2 
On Sat, May 30, 2020 at 10:42:05PM -0400, Raphael Norwitz wrote:
> I'm happy with the code but as David pointed out with virtio-scsi, we
> should probably add a comment about virtio_pci_optimal_num_queues()
> capping the number of VQs here too.

Stefan could you add this tweak and repost pls? Thanks!


> 
> On Wed, May 27, 2020 at 6:34 AM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > Automatically size the number of request virtqueues to match the number
> > of vCPUs.  This ensures that completion interrupts are handled on the
> > same vCPU that submitted the request.  No IPI is necessary to complete
> > an I/O request and performance is improved.
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  include/hw/virtio/vhost-user-blk.h | 2 ++
> >  hw/block/vhost-user-blk.c          | 6 +++++-
> >  hw/core/machine.c                  | 1 +
> >  hw/virtio/vhost-user-blk-pci.c     | 4 ++++
> >  4 files changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
> > index 34ad6f0c0e..292d17147c 100644
> > --- a/include/hw/virtio/vhost-user-blk.h
> > +++ b/include/hw/virtio/vhost-user-blk.h
> > @@ -25,6 +25,8 @@
> >  #define VHOST_USER_BLK(obj) \
> >          OBJECT_CHECK(VHostUserBlk, (obj), TYPE_VHOST_USER_BLK)
> >
> > +#define VHOST_USER_BLK_AUTO_NUM_QUEUES UINT16_MAX
> > +
> >  typedef struct VHostUserBlk {
> >      VirtIODevice parent_obj;
> >      CharBackend chardev;
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > index 9d8c0b3909..7a8639516f 100644
> > --- a/hw/block/vhost-user-blk.c
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -385,6 +385,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
> >          return;
> >      }
> >
> > +    if (s->num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> > +        s->num_queues = 1;
> > +    }
> >      if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
> >          error_setg(errp, "vhost-user-blk: invalid number of IO queues");
> >          return;
> > @@ -496,7 +499,8 @@ static const VMStateDescription vmstate_vhost_user_blk = {
> >
> >  static Property vhost_user_blk_properties[] = {
> >      DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
> > -    DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues, 1),
> > +    DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
> > +                       VHOST_USER_BLK_AUTO_NUM_QUEUES),
> >      DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
> >      DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
> >      DEFINE_PROP_END_OF_LIST(),
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 4aba3bdd3c..8cc4b54eec 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -32,6 +32,7 @@ GlobalProperty hw_compat_5_0[] = {
> >      { "virtio-blk-device", "num-queues", "1"},
> >      { "virtio-scsi-device", "num_queues", "1"},
> >      { "vhost-scsi", "num_queues", "1"},
> > +    { "vhost-user-blk", "num-queues", "1"},
> >      { "vhost-user-scsi", "num_queues", "1"},
> >  };
> >  const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
> > diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
> > index 58d7c31735..1c8ab7f5e6 100644
> > --- a/hw/virtio/vhost-user-blk-pci.c
> > +++ b/hw/virtio/vhost-user-blk-pci.c
> > @@ -54,6 +54,10 @@ static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
> >      VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
> >      DeviceState *vdev = DEVICE(&dev->vdev);
> >
> > +    if (dev->vdev.num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
> > +        dev->vdev.num_queues = virtio_pci_optimal_num_queues(0);
> > +    }
> > +
> >      if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> >          vpci_dev->nvectors = dev->vdev.num_queues + 1;
> >      }
> > --
> > 2.25.4
> >
diff mbox series

Patch

diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index 34ad6f0c0e..292d17147c 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -25,6 +25,8 @@ 
 #define VHOST_USER_BLK(obj) \
         OBJECT_CHECK(VHostUserBlk, (obj), TYPE_VHOST_USER_BLK)
 
+#define VHOST_USER_BLK_AUTO_NUM_QUEUES UINT16_MAX
+
 typedef struct VHostUserBlk {
     VirtIODevice parent_obj;
     CharBackend chardev;
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9d8c0b3909..7a8639516f 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -385,6 +385,9 @@  static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    if (s->num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
+        s->num_queues = 1;
+    }
     if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
         error_setg(errp, "vhost-user-blk: invalid number of IO queues");
         return;
@@ -496,7 +499,8 @@  static const VMStateDescription vmstate_vhost_user_blk = {
 
 static Property vhost_user_blk_properties[] = {
     DEFINE_PROP_CHR("chardev", VHostUserBlk, chardev),
-    DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues, 1),
+    DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
+                       VHOST_USER_BLK_AUTO_NUM_QUEUES),
     DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
     DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 4aba3bdd3c..8cc4b54eec 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -32,6 +32,7 @@  GlobalProperty hw_compat_5_0[] = {
     { "virtio-blk-device", "num-queues", "1"},
     { "virtio-scsi-device", "num_queues", "1"},
     { "vhost-scsi", "num_queues", "1"},
+    { "vhost-user-blk", "num-queues", "1"},
     { "vhost-user-scsi", "num_queues", "1"},
 };
 const size_t hw_compat_5_0_len = G_N_ELEMENTS(hw_compat_5_0);
diff --git a/hw/virtio/vhost-user-blk-pci.c b/hw/virtio/vhost-user-blk-pci.c
index 58d7c31735..1c8ab7f5e6 100644
--- a/hw/virtio/vhost-user-blk-pci.c
+++ b/hw/virtio/vhost-user-blk-pci.c
@@ -54,6 +54,10 @@  static void vhost_user_blk_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     VHostUserBlkPCI *dev = VHOST_USER_BLK_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
 
+    if (dev->vdev.num_queues == VHOST_USER_BLK_AUTO_NUM_QUEUES) {
+        dev->vdev.num_queues = virtio_pci_optimal_num_queues(0);
+    }
+
     if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
         vpci_dev->nvectors = dev->vdev.num_queues + 1;
     }