Re: [PATCH v1 2/4] virtio: increase virtuqueue size for virtio-scsi and virtio-blk

[Stefan Hajnoczi]

On Fri, Feb 07, 2020 at 11:48:05AM +0300, Denis Plotnikov wrote:
> 
> 
> On 05.02.2020 14:19, Stefan Hajnoczi wrote:
> > On Tue, Feb 04, 2020 at 12:59:04PM +0300, Denis Plotnikov wrote:
> > > 
> > > On 30.01.2020 17:58, Stefan Hajnoczi wrote:
> > > > On Wed, Jan 29, 2020 at 05:07:00PM +0300, Denis Plotnikov wrote:
> > > > > The goal is to reduce the amount of requests issued by a guest on
> > > > > 1M reads/writes. This rises the performance up to 4% on that kind of
> > > > > disk access pattern.
> > > > > 
> > > > > The maximum chunk size to be used for the guest disk accessing is
> > > > > limited with seg_max parameter, which represents the max amount of
> > > > > pices in the scatter-geather list in one guest disk request.
> > > > > 
> > > > > Since seg_max is virqueue_size dependent, increasing the virtqueue
> > > > > size increases seg_max, which, in turn, increases the maximum size
> > > > > of data to be read/write from guest disk.
> > > > > 
> > > > > More details in the original problem statment:
> > > > > https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html
> > > > > 
> > > > > Suggested-by: Denis V. Lunev <address@hidden>
> > > > > Signed-off-by: Denis Plotnikov <address@hidden>
> > > > > ---
> > > > >    hw/core/machine.c          | 3 +++
> > > > >    include/hw/virtio/virtio.h | 2 +-
> > > > >    2 files changed, 4 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > > > > index 3e288bfceb..8bc401d8b7 100644
> > > > > --- a/hw/core/machine.c
> > > > > +++ b/hw/core/machine.c
> > > > > @@ -28,6 +28,9 @@
> > > > >    #include "hw/mem/nvdimm.h"
> > > > >    GlobalProperty hw_compat_4_2[] = {
> > > > > +    { "virtio-blk-device", "queue-size", "128"},
> > > > > +    { "virtio-scsi-device", "virtqueue_size", "128"},
> > > > > +    { "vhost-blk-device", "virtqueue_size", "128"},
> > > > vhost-blk-device?!  Who has this?  It's not in qemu.git so please omit
> > > > this line. ;-)
> > > So in this case the line:
> > > 
> > > { "vhost-blk-device", "seg_max_adjust", "off"},
> > > 
> > > introduced by my patch:
> > > 
> > > commit 1bf8a989a566b2ba41c197004ec2a02562a766a4
> > > Author: Denis Plotnikov <address@hidden>
> > > Date:   Fri Dec 20 17:09:04 2019 +0300
> > > 
> > >      virtio: make seg_max virtqueue size dependent
> > > 
> > > is also wrong. It should be:
> > > 
> > > { "vhost-scsi-device", "seg_max_adjust", "off"},
> > > 
> > > Am I right?
> > It's just called "vhost-scsi":
> > 
> > include/hw/virtio/vhost-scsi.h:#define TYPE_VHOST_SCSI "vhost-scsi"
> > 
> > > > On the other hand, do you want to do this for the vhost-user-blk,
> > > > vhost-user-scsi, and vhost-scsi devices that exist in qemu.git?  Those
> > > > devices would benefit from better performance too.
> After thinking about that for a while, I think we shouldn't extend queue
> sizes for vhost-user-blk, vhost-user-scsi and vhost-scsi.
> This is because increasing the queue sizes seems to be just useless for
> them: the all thing is about increasing the queue sizes for increasing
> seg_max (it limits the max block query size from the guest). For
> virtio-blk-device and virtio-scsi-device it makes sense, since they have
> seg-max-adjust property which, if true, sets seg_max to virtqueue_size-2.
> vhost-scsi also have this property but it seems the property just doesn't
> affect anything (remove it?).
> Also vhost-user-blk, vhost-user-scsi and vhost-scsi don't do any seg_max
> settings. If I understand correctly, their backends are ment to be
> responsible for doing that.
> So, what about changing the queue sizes just for virtio-blk-device and
> virtio-scsi-device?

That's fine.  If it's beneficial to extend it to the vhost devices then
it can be done later.  I didn't look into it (and the way that
responsibility for these parameters is shared between QEMU and the
vhost-user device backend is a little complicated).

Stefan

signature.asc
Description: PGP signature