bhyve reviewers group
Wed, Jan 16
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.
Tue, Jan 15
Mon, Jan 14
Added comment where vbc_seg_max is set to note why that value is clamped as it is (as recommended by pmooney_pfmooney.com).
Sun, Jan 13
This is effectively what we did in SmartOS/illumos to address the issue:
Thanks for the patch, I will take a look on within the next two weeks.
Fix cut-and-paste error in patch
Fri, Jan 4
Wed, Jan 2
This looks good to me as well...
LGTM! Could you please set a MFC?
LGTM! Do you mind set a MFC?
Mon, Dec 31
Use ONE_BASED() macro for CQ creation as well as SQ
Dec 11 2018
LGTM! Thanks for the patch!
Dec 10 2018
Dec 3 2018
Nov 5 2018
Sep 6 2018
Sep 4 2018
I have tested it with FreeBSD HEAD as a guest running for couple days.
Aug 28 2018
This looks right to me now. I'll try to test it locally in the next day or so.
Minimize diff with suggestions by jhb.
Reflect changes requested by both kib and jhb.
You should only be replacing the existing MAP_ANON / PROT_NONE mmap() calls with MAP_GUARD instead. You shouldn't be adding new mmap() calls. The "real" thing is already being mmap()'d later. As @kib notes, it's good to use 'svn diff -x -U 99999' or the like to generate context when uploading to phabricator.
The only useful feature of the phabricator is to easily see context around the patch, which you successfully botched.
Missed a spot. Cover another mapping with MAP_GUARD.
Update the patch to use John Baldwin's suggestion on mapping the entire range first with MAP_GUARD.
Adding @kib since he added MAP_GUARD. I think you should instead MAP_GUARD the entire range first and then remap the middle. The middle is already remapped on line 518, so instead of adding new mmap()'s just for the guards, you should replace the mmap() on line 496 with this: