Bug 1787022

(In reply to Quan Wenli from comment #2)
> Hello Vadim
> 
> Could you help review the results which comparison between build 175 and 177
> with 128k/256k/1m/2m fio tests
> 
> 
> - in Win2019 result, some degradation in read/write/randread tests.
> 
>  http://10.73.60.69/results/request/Bug1787022/raw.virtio_scsi.*.Win2019.
> x86_64.html 
> 
> - in Win2012r2 result, slightly degradation in read/write tests.
> 
>  http://10.73.60.69/results/request/Bug1787022/raw.virtio_scsi.*.Win2012.
> x86_64.r2.html

Thanks a lot for the testing.
Can you please run this test on NTFS-formatted volume?

Best,
Vadim.

(In reply to Vadim Rozenfeld from comment #3)
> (In reply to Quan Wenli from comment #2)
> > Hello Vadim
> > 
> > Could you help review the results which comparison between build 175 and 177
> > with 128k/256k/1m/2m fio tests
> > 
> > 
> > - in Win2019 result, some degradation in read/write/randread tests.
> > 
> >  http://10.73.60.69/results/request/Bug1787022/raw.virtio_scsi.*.Win2019.
> > x86_64.html 
> > 
> > - in Win2012r2 result, slightly degradation in read/write tests.
> > 
> >  http://10.73.60.69/results/request/Bug1787022/raw.virtio_scsi.*.Win2012.
> > x86_64.r2.html
> 
> Thanks a lot for the testing.
> Can you please run this test on NTFS-formatted volume?

OK, I will update the results after china holiday. 

> 
> Best,
> Vadim.

After some considerations I desided to make the maximum transfer size adjustable, rather 
then keep it hard coded. By default the maximum transfer size will be 256K (this is 
value was hard coded since the first release of viostor driver). While 256K works
quite good for file and nvme back-ended scnarious, for some cases, like scsi_dbg RAM
disk its performance is sub-optimal. 
With the lates changes 
https://github.com/virtio-win/kvm-guest-drivers-windows/commit/a18fc89a8139884dc51add0f641c466f2d9826b2
the maximum transfer lenght can be adjusted by creating and editing the following Registry key
[HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\vioscsi\Parameters\Device]
"PhysicalBreaks"=dword:000000ff

New Registry key "PhysicalBreaks" has been introduced in build 181 and available now.

 Please use the following .reg file to add "PhysicalBreaks" parameter to the Registry
    
    Windows Registry Editor Version 5.00
    
    [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\vioscsi\Parameters\Device]
    "PhysicalBreaks"=dword:000000ff

By modifying "PhysicalBreaks" we can redefine the maximum transfer lengte that miniport 
drivers reports to the rest of the OS. "PhysicalBreaks" can be adjusted in the range
16..255 which is 64K..1024K maximum transfer length. If "PhysicalBreaks" is not explicitly
specified in the Registry, then miniport driver will report the default value - 64 breaks/
256K maximum transfer length.
While we didn't go through the proper performance test cycle yet, some preliminary results 
show up to 10..15% performance improvident on particular back-ends (scsi_dbg, for example)
for 512K/1024K sequential read/write loads.

(In reply to Vadim Rozenfeld from comment #16)
> New Registry key "PhysicalBreaks" has been introduced in build 181 and
> available now.
> 
>  Please use the following .reg file to add "PhysicalBreaks" parameter to the
> Registry
>     
>     Windows Registry Editor Version 5.00
>     
>    
> [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\vioscsi\Parameters\Devi
> ce]
>     "PhysicalBreaks"=dword:000000ff
> 
> By modifying "PhysicalBreaks" we can redefine the maximum transfer lengte
> that miniport 
> drivers reports to the rest of the OS. "PhysicalBreaks" can be adjusted in
> the range
> 16..255 which is 64K..1024K maximum transfer length. If "PhysicalBreaks" is
> not explicitly
> specified in the Registry, then miniport driver will report the default
> value - 64 breaks/
> 256K maximum transfer length.
> While we didn't go through the proper performance test cycle yet, some
> preliminary results 
> show up to 10..15% performance improvident on particular back-ends
> (scsi_dbg, for example)
> for 512K/1024K sequential read/write loads.

Hi Vadim,

I want to confirm some questions with you about how to verify the bug. Do you think following tests can verify the bug? If no, could you tell us how to better verify it? 

Scenario 1:
Don't specify the "PhysicalBreaks", use the default 256k, then compare fio performance between virtio-win-prewhql-180 and virtio-win-prewhql-181 with following steps.

1. on host:
#mkfs.xfs /dev/sdb & mount /dev/sdb /mnt/test
/dev/sdb is a 60GB ssd.
#qemu-img create -f qcow2 /mnt/test/storage2.qcow2 40G

2.boot a guest with above image:
    -device virtio-blk-pci,id=image1,drive=drive_image1,bootindex=0,bus=pci.0,addr=0x4 \
    -drive id=drive_disk1,if=none,snapshot=off,aio=threads,cache=none,format=qcow2,file=/mnt/test/storage2.qcow2 \

3. running fio on the data disk created by step1 on guest:
echo select disk 1 > imDiskpart.script && echo create partition primary >> imDiskpart.script && echo assign letter=I >> imDiskpart.script&& echo exit >> imDiskpart.script && diskpart /s imDiskpart.script && format I: /FS:NTFS /V:local /Q /y

fio.exe --rw=%s --bs=%s --iodepth=%s --runtime=1m --direct=1 --filename=I\:\%s_%s_%s --name=job1 --ioengine=windowsaio --thread --group_reporting --numjobs=16 --size=512MB --time_based --output="C:\\fio_result"

- rw: read write randread randwrite randrw
- bs: 256k 512k 1M 2M
- iodepth: 1 8 64

4. expected results: no performance regression

Scenario 2:

Adjusting the "PhysicalBreaks" to run above test again, what value do you recommend?

Thanks Yanhui.

(In reply to Yanhui Ma from comment #17)
> (In reply to Vadim Rozenfeld from comment #16)
> > New Registry key "PhysicalBreaks" has been introduced in build 181 and
> > available now.
> > 
> >  Please use the following .reg file to add "PhysicalBreaks" parameter to the
> > Registry
> >     
> >     Windows Registry Editor Version 5.00
> >     
> >    
> > [HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\vioscsi\Parameters\Devi
> > ce]
> >     "PhysicalBreaks"=dword:000000ff
> > 
> > By modifying "PhysicalBreaks" we can redefine the maximum transfer lengte
> > that miniport 
> > drivers reports to the rest of the OS. "PhysicalBreaks" can be adjusted in
> > the range
> > 16..255 which is 64K..1024K maximum transfer length. If "PhysicalBreaks" is
> > not explicitly
> > specified in the Registry, then miniport driver will report the default
> > value - 64 breaks/
> > 256K maximum transfer length.
> > While we didn't go through the proper performance test cycle yet, some
> > preliminary results 
> > show up to 10..15% performance improvident on particular back-ends
> > (scsi_dbg, for example)
> > for 512K/1024K sequential read/write loads.
> 
> Hi Vadim,
> 
> I want to confirm some questions with you about how to verify the bug. Do
> you think following tests can verify the bug? If no, could you tell us how
> to better verify it? 
> 
> Scenario 1:
> Don't specify the "PhysicalBreaks", use the default 256k, then compare fio
> performance between virtio-win-prewhql-180 and virtio-win-prewhql-181 with
> following steps.
> 
> 1. on host:
> #mkfs.xfs /dev/sdb & mount /dev/sdb /mnt/test
> /dev/sdb is a 60GB ssd.
> #qemu-img create -f qcow2 /mnt/test/storage2.qcow2 40G
> 
> 2.boot a guest with above image:
>     -device
> virtio-blk-pci,id=image1,drive=drive_image1,bootindex=0,bus=pci.0,addr=0x4 \
>     -drive
> id=drive_disk1,if=none,snapshot=off,aio=threads,cache=none,format=qcow2,
> file=/mnt/test/storage2.qcow2 \
> 
> 3. running fio on the data disk created by step1 on guest:
> echo select disk 1 > imDiskpart.script && echo create partition primary >>
> imDiskpart.script && echo assign letter=I >> imDiskpart.script&& echo exit
> >> imDiskpart.script && diskpart /s imDiskpart.script && format I: /FS:NTFS
> /V:local /Q /y
> 
> fio.exe --rw=%s --bs=%s --iodepth=%s --runtime=1m --direct=1
> --filename=I\:\%s_%s_%s --name=job1 --ioengine=windowsaio --thread
> --group_reporting --numjobs=16 --size=512MB --time_based
> --output="C:\\fio_result"
> 
> - rw: read write randread randwrite randrw
> - bs: 256k 512k 1M 2M
> - iodepth: 1 8 64
> 
> 4. expected results: no performance regression
> 
> Scenario 2:
> 
> Adjusting the "PhysicalBreaks" to run above test again, what value do you
> recommend?
> 

Currently it limited by 255 as the upper limit and 16 as the lower. 
So any value greater than 0x100 will be trimmed down. In the future
we will probably adjust this value. But for now it should be absolutely
and reasonable safe to check two scenarios - default (no PhysicalBreaks
key created or PhysicalBreaks = 64 (0x40h)  vs PhysicalBreaks = 256 (0x100h)

Sorry again for delayed response.
Vadim.

 
> Thanks Yanhui.

(In reply to Vadim Rozenfeld from comment #18)

> > 
> > Scenario 2:
> > 
> > Adjusting the "PhysicalBreaks" to run above test again, what value do you
> > recommend?
> > 
> 
> Currently it limited by 255 as the upper limit and 16 as the lower. 
> So any value greater than 0x100 will be trimmed down. In the future
> we will probably adjust this value. But for now it should be absolutely
> and reasonable safe to check two scenarios - default (no PhysicalBreaks
> key created or PhysicalBreaks = 64 (0x40h)  vs PhysicalBreaks = 256 (0x100h)
> 
> Sorry again for delayed response.
> Vadim.
>

Here are results between default(no PhysicalBreaks key created) and PhysicalBreaks = 256 (0x100h) with virtio-win-prewhql-0.1-185:

http://kvm-perf.englab.nay.redhat.com/results/regression/bug1787022/qcow2.virtio_scsi.*.Win2019.x86_64.html

No regression, there are improvements for some data. But the standard deviation for some data is big. I will run it again.
 
>  
> > Thanks Yanhui.

(In reply to Yanhui Ma from comment #19)
> (In reply to Vadim Rozenfeld from comment #18)
> 
> > > 
> > > Scenario 2:
> > > 
> > > Adjusting the "PhysicalBreaks" to run above test again, what value do you
> > > recommend?
> > > 
> > 
> > Currently it limited by 255 as the upper limit and 16 as the lower. 
> > So any value greater than 0x100 will be trimmed down. In the future
> > we will probably adjust this value. But for now it should be absolutely
> > and reasonable safe to check two scenarios - default (no PhysicalBreaks
> > key created or PhysicalBreaks = 64 (0x40h)  vs PhysicalBreaks = 256 (0x100h)
> > 
> > Sorry again for delayed response.
> > Vadim.
> >
> 
> Here are results between default(no PhysicalBreaks key created) and
> PhysicalBreaks = 256 (0x100h) with virtio-win-prewhql-0.1-185:
> 
> http://kvm-perf.englab.nay.redhat.com/results/regression/bug1787022/qcow2.
> virtio_scsi.*.Win2019.x86_64.html
> 
> No regression, there are improvements for some data. But the standard
> deviation for some data is big. I will run it again.
> 

Here are the second test results:
http://kvm-perf.englab.nay.redhat.com/results/regression/bug1787022/repeat/qcow2.virtio_scsi.*.Win2019.x86_64.html 

Based on the results, set the bug verified.

> >  
> > > Thanks Yanhui.

(In reply to Yanhui Ma from comment #20)
> (In reply to Yanhui Ma from comment #19)
> > (In reply to Vadim Rozenfeld from comment #18)
> > 
> > > > 
> > > > Scenario 2:
> > > > 
> > > > Adjusting the "PhysicalBreaks" to run above test again, what value do you
> > > > recommend?
> > > > 
> > > 
> > > Currently it limited by 255 as the upper limit and 16 as the lower. 
> > > So any value greater than 0x100 will be trimmed down. In the future
> > > we will probably adjust this value. But for now it should be absolutely
> > > and reasonable safe to check two scenarios - default (no PhysicalBreaks
> > > key created or PhysicalBreaks = 64 (0x40h)  vs PhysicalBreaks = 256 (0x100h)
> > > 
> > > Sorry again for delayed response.
> > > Vadim.
> > >
> > 
> > Here are results between default(no PhysicalBreaks key created) and
> > PhysicalBreaks = 256 (0x100h) with virtio-win-prewhql-0.1-185:
> > 
> > http://kvm-perf.englab.nay.redhat.com/results/regression/bug1787022/qcow2.
> > virtio_scsi.*.Win2019.x86_64.html
> > 
> > No regression, there are improvements for some data. But the standard
> > deviation for some data is big. I will run it again.
> > 
> 
> Here are the second test results:
> http://kvm-perf.englab.nay.redhat.com/results/regression/bug1787022/repeat/
> qcow2.virtio_scsi.*.Win2019.x86_64.html 
> 
> Based on the results, set the bug verified.

Thank you a lot.
Vadim.
> 
> > >  
> > > > Thanks Yanhui.

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (virtio-win bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHEA-2020:4840