Thanks for the patch, I will take a look on within the next two weeks.

This is effectively what we did in SmartOS/illumos to address the issue:
https://github.com/joyent/illumos-joyent/commit/192e1e6405f98e4b0a12f9488793c5dd000f3f7e

I would suggest including a comment where vbc_seg_max is set to note why that value is clamped as it is.

In D18831#402095, @pmooney_pfmooney.com wrote:

This is effectively what we did in SmartOS/illumos to address the issue:
https://github.com/joyent/illumos-joyent/commit/192e1e6405f98e4b0a12f9488793c5dd000f3f7e

I would suggest including a comment where vbc_seg_max is set to note why that value is clamped as it is.

Thanks Patrick, I've included the comment from your patch as suggested.

The SmartOS patch was the source of my patch - I only raised it here so that it might find its way into the upstream code-base, as both other attempts at this seem to have stalled.
Definitely happy for the original author to claim it.

Hi Richard,

Do you mind share the tests you made??

Best,

In D18831#402109, @araujo wrote:

Hi Richard,

Do you mind share the tests you made??

Best,

I've currently got these changes running in a FreeNAS-11.2-RELEASE-U1 instance (FreeBSD 11.2 kernel) running a couple of Windows 2016 VMs plus one Centos 7 - all using virtio-blk disk devices without issue. I also have pfSense (FreeBSD 11.2) and OpenSuSE (Leap 42.3) VMs which I haven't tested yet - but can do if required.

In D18831#402110, @richard_meric.id.au wrote:

In D18831#402109, @araujo wrote:

Hi Richard,

Do you mind share the tests you made??

Best,

I've currently got these changes running in a FreeNAS-11.2-RELEASE-U1 instance (FreeBSD 11.2 kernel) running a couple of Windows 2016 VMs plus one Centos 7 - all using virtio-blk disk devices without issue. I also have pfSense (FreeBSD 11.2) and OpenSuSE (Leap 42.3) VMs which I haven't tested yet - but can do if required.

OK! Thanks for sharing!
I have a test-bed here where I will test this patch and make some disk stress testing during couple days.

Not sure about the seg_max value (even in the current code).
Looking at the qemu implementation, I suspect it should be queue length -2, which I guess is what the patched code happens to achieve (based on the definition of VTBLK_RINGSZ), but it could be more explicit.

Only remaining thing is whether we should do something different for the ahci device, as it's currently getting a 128 element IOV, but really only needs 32 (33?). I'm not sure if it is a problem having it larger?