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

[Denis Plotnikov <dplotnikov@virtuozzo.com>]

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