[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCS
From: |
Liu, Changpeng |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block |
Date: |
Tue, 19 Dec 2017 09:59:21 +0000 |
> -----Original Message-----
> From: Roman Kagan [mailto:address@hidden
> Sent: Tuesday, December 19, 2017 4:58 PM
> To: Denis V. Lunev <address@hidden>
> Cc: Felipe Franciosi <address@hidden>; Harris, James R
> <address@hidden>; Stefan Hajnoczi <address@hidden>; Kevin Wolf
> <address@hidden>; Eduardo Habkost <address@hidden>; Michael S.
> Tsirkin <address@hidden>; address@hidden; Max Reitz
> <address@hidden>; Paolo Bonzini <address@hidden>; Liu, Changpeng
> <address@hidden>; Richard Henderson <address@hidden>
> Subject: Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio
> SCSI/block
>
> On Mon, Dec 18, 2017 at 10:42:35PM +0300, Denis V. Lunev wrote:
> > On 12/18/2017 10:35 PM, Felipe Franciosi wrote:
> > >> On 18 Dec 2017, at 16:16, Harris, James R <address@hidden> wrote:
> > >>> On Dec 18, 2017, at 6:38 AM, Stefan Hajnoczi <address@hidden> wrote:
> > >>> On Fri, Dec 15, 2017 at 06:02:50PM +0300, Denis V. Lunev wrote:
> > >>>> diff --git a/include/hw/compat.h b/include/hw/compat.h
> > >>>> index 026fee9..b9be5d7 100644
> > >>>> --- a/include/hw/compat.h
> > >>>> +++ b/include/hw/compat.h
> > >>>> @@ -2,6 +2,23 @@
> > >>>> #define HW_COMPAT_H
> > >>>>
> > >>>> #define HW_COMPAT_2_11 \
> > >>>> + {\
> > >>>> + .driver = "virtio-blk-device",\
> > >>>> + .property = "max_segments",\
> > >>>> + .value = "126",\
> > >>>> + },{\
> > >>>> + .driver = "vhost-scsi",\
> > >>>> + .property = "max_segments",\
> > >>>> + .value = "126",\
> > >>>> + },{\
> > >>>> + .driver = "vhost-user-scsi",\
> > >>>> + .property = "max_segments",\
> > >>>> + .value = "126",\
> > >>> Existing vhost-user-scsi slave programs might not expect up to 1022
> > >>> segments. Hopefully we can get away with this change since there are
> > >>> relatively few vhost-user-scsi slave programs.
> > >>>
> > >>> CCed Felipe (Nutanix) and Jim (SPDK) in case they have comments.
> > >> SPDK vhost-user targets only expect max 128 segments. They also
> > >> pre-allocate
> I/O task structures when QEMU connects to the vhost-user device.
> > >>
> > >> Supporting up to 1022 segments would result in significantly higher
> > >> memory
> usage, reduction in I/O queue depth processed by the vhost-user target, or
> having
> to dynamically allocate I/O task structures - none of which are ideal.
> > >>
> > >> What if this was just bumped from 126 to 128? I guess I’m trying to
> understand the level of guest and host I/O performance that is gained with
> this
> patch. One I/O per 512KB vs. one I/O per 4MB - we are still only talking
> about a
> few hundred IO/s difference.
> > > SeaBIOS also makes the assumption that the queue size is not bigger than
> > > 128
> elements.
> > > https://review.coreboot.org/cgit/seabios.git/tree/src/hw/virtio-ring.h#n23
> > >
> > > Perhaps a better approach is to make the value configurable (ie. add the
> "max_segments" property), but set the default to 128-2. In addition to what
> Jim
> pointed out, I think there may be other legacy front end drivers which can
> assume
> the ring will be at most 128 entries in size.
> > >
> > > With that, hypervisors can choose to bump the value higher if it's known
> > > to be
> safe for their host+guest configuration.
> >
> > This should not be a problem at all IMHO. The guest is not obliged
> > to use the message of entire possible size. The guest initiates
> > request with 128 elements. Fine. QEMU is ready to this.
>
> QEMU is, but vhost-user slaves may not be. And there seems to be no
> vhost-user protocol message type that would allow to negotiate this
> value between the master and the slave.
>
> So apparently the default for vhost-user-scsi has to stay the same in
> order not to break existing slaves. I guess having it tunable via a
> property may still turn out useful.
Actually I wrote a new patch set recently for support vhost-user-blk host
device,
and added 2 extra vhost-user messages, GET_CONFIG/SET_CONFIG which can let
host device get those parameters
from vhost-user slave target, the new added messages can get virtio device's
configuration space from slave target,
so vhost-user-scsi may use that as well.
>
> Roman.
- [Qemu-devel] [PATCH v3 0/2] virtio: fix IO request length in virtio SCSI/block, Denis V. Lunev, 2017/12/15
- [Qemu-devel] [PATCH 1/2] pc, q35: add 2.12 machine types, Denis V. Lunev, 2017/12/15
- [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Denis V. Lunev, 2017/12/15
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Stefan Hajnoczi, 2017/12/18
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Harris, James R, 2017/12/18
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Felipe Franciosi, 2017/12/18
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Denis V. Lunev, 2017/12/18
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Roman Kagan, 2017/12/19
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block,
Liu, Changpeng <=
- Re: [Qemu-devel] [PATCH 2/2] virtio: fix IO request length in virtio SCSI/block, Michael S. Tsirkin, 2017/12/19
Re: [Qemu-devel] [PATCH v3 0/2] virtio: fix IO request length in virtio SCSI/block, Stefan Hajnoczi, 2017/12/18
Re: [Qemu-devel] [PATCH v3 0/2] virtio: fix IO request length in virtio SCSI/block, Denis V. Lunev, 2017/12/19
Re: [Qemu-devel] [PATCH v3 0/2] virtio: fix IO request length in virtio SCSI/block, Michael S. Tsirkin, 2017/12/19