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

[Stefano Garzarella <sgarzare@redhat.com>]

Message bounced, I use new Denis's email address.

On Wed, Sep 08, 2021 at 03:17:16PM +0200, Stefano Garzarella wrote:
>Hi Denis,
>I just found this discussion since we still have the following line in 
>hw/core/machine.c:
>    { "vhost-blk-device", "seg_max_adjust", "off"}
>
>IIUC it was a typo, and I think we should fix it since in the future 
>we can have `vhost-blk-device`.
>
>So, I think we have 2 options:
>1. remove that line since for now is useless
>2. replace with "vhost-scsi"
>
>I'm not sure which is the best, what do you suggest?
>
>Thanks,
>Stefano
>
>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 <den@openvz.org>
>>>>>>Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com>
>>>>>>---
>>>>>>  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 <dplotnikov@virtuozzo.com>
>>>>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?
>>
>>Denis
>>
>>>>It seems to be so. We also have the test checking those settings:
>>>>tests/acceptance/virtio_seg_max_adjust.py
>>>>For now it checks virtio-scsi-pci and virtio-blk-pci.
>>>>I'm going to extend it for the virtqueue size checking.
>>>>If I change vhost-user-blk, vhost-user-scsi and vhost-scsi it's worth
>>>>to check those devices too. But I don't know how to form a command line
>>>>for that 3 devices since they should involve some third party components as
>>>>backends (kernel modules, DPDK, etc.) and they seems to be not available in
>>>>the
>>>>qemu git.
>>>>Is there any way to do it with some qit.qemu available stubs or something
>>>>else?
>>>>If so, could you please point out the proper way to do it?
>>>qemu.git has contrib/vhost-user-blk/ and contrib/vhost-user-scsi/ if
>>>you need to test those vhost-user devices without external dependencies.
>>>
>>>Stefan
>>
>>