On Thu, 16 Jan 2020 14:16:54 +0000 Stefan Hajnoczi <stefa...@redhat.com> wrote:
> On Thu, Jan 16, 2020 at 12:53:49PM +0100, Cornelia Huck wrote:
> > On Thu, 16 Jan 2020 10:58:40 +0000
> > Stefan Hajnoczi <stefa...@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.
> > >
> > > Remember that virtqueue numbering assumptions are being removed from the
> > > virtio-pci proxy object, so the Control and Event virtqueues are counted
> > > by ->get_num_virtqueues() and we only add 1 for the Configuration Change
> > > interrupt:
> > >
> > > if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> > > - vpci_dev->nvectors = vs->conf.num_queues + 3;
> > > + vpci_dev->nvectors =
> > > vdc->get_num_virtqueues(VIRTIO_DEVICE(vdev)) + 1;
> > > }
> > >
> > > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> > > ---
> > > hw/core/machine.c | 3 +++
> > > hw/scsi/vhost-scsi.c | 2 ++
> > > hw/scsi/vhost-user-scsi.c | 2 ++
> > > hw/scsi/virtio-scsi.c | 18 ++++++++++++++++++
> > > hw/virtio/vhost-scsi-pci.c | 4 ++--
> > > hw/virtio/vhost-user-scsi-pci.c | 4 ++--
> > > hw/virtio/virtio-scsi-pci.c | 4 ++--
> > > include/hw/virtio/virtio-scsi.h | 1 +
> > > 8 files changed, 32 insertions(+), 6 deletions(-)
> > >
> >
> > > @@ -878,6 +879,18 @@ static struct SCSIBusInfo virtio_scsi_scsi_info = {
> > > .load_request = virtio_scsi_load_request,
> > > };
> > >
> > > +static uint32_t virtio_scsi_get_num_virtqueues(VirtIODevice *vdev)
> > > +{
> > > + VirtIOSCSICommon *s = VIRTIO_SCSI_COMMON(vdev);
> > > + uint32_t request_queues = s->conf.num_queues;
> > > +
> > > + if (s->conf.num_queues == 1 && s->conf.auto_num_queues) {
> > > + request_queues = current_machine->smp.cpus;
> > > + }
> > > +
> > > + return VIRTIO_SCSI_VQ_NUM_FIXED + request_queues;
> >
> > I'm not sure doing this at the device level is the right thing to do.
> > For now, only virtio-pci will call this function, and there basing the
> > number of virtqueues off the number of cpus makes sense; but that's a
> > property of the transport, not of the device.
> >
> > Consider e.g. a virtio-scsi-ccw device: If we wanted to introduce a way
> > to automatically pick a good number of virtqueues there, this functions
> > likely would not return a particularly useful value, as queue interrupt
> > processing does not really relate to the number of cpus with adapter
> > interrupts. It's not a problem right now, as only virtio-pci calls
> > this, but someone looking at this callback is likely getting the
> > impression that this is a generically useful function.
>
> That's a great point. Maybe the direction should be reversed so that
> the device asks the transport to suggest the optimal number of queues
> when auto-num-queues is enabled.
Maybe the device also specifying that it needs at least m queues, and
the transport can then return max(m, n) (with n being the number of
cpus in the pci case).
My next problem is that I'm not sure what 'optimal number of queues'
would mean from a ccw viewpoint. "Remaining free bits in the indicator
area" will be way too much in the general case :) Giving an upper limit
is easy, but not a value that we'd want to autoconfigure, and I really
don't want to return 42 all the time. Would it make sense to make this
feature opt-in per transport?
pgpOfn5TCYvTS.pgp
Description: OpenPGP digital signature
