Page MenuHomeFreeBSD

vmm: remove unneccessary rendezvous assertion
ClosedPublic

Authored by corvink on Thu, Nov 17, 6:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Dec 3, 7:37 AM
Unknown Object (File)
Sat, Dec 3, 7:37 AM
Unknown Object (File)
Sat, Dec 3, 7:37 AM
Unknown Object (File)
Mon, Nov 28, 4:40 PM
Unknown Object (File)
Sun, Nov 27, 12:42 PM

Details

Summary

When a vcpu sees that a rendezvous is ongoing, it exits and tries to
handle the rendezvous. The vcpu doesn't check if it's part of the
rendezvous or not. If the vcpu isn't part of the rendezvous, the
rendezvous could be done before it reaches the assertion. This will
cause a panic.

The assertion isn't needed at all because vm_handle_rendezvous properly
handles a spurious rendezvous. So, we can just remove it.

PR: 267779

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Thu, Nov 17, 2:12 PM
sys/amd64/vmm/vmm.c
1803–1804

Correct me if I am wrong, vm_handle_rendezvous() will set all threads sleeping even if thread VCPU is not in dest (rendezvous_req_cpus) ?

sys/amd64/vmm/vmm.c
1803–1804

Yes, that's true but it's only a performance issue. Additionally, if vm_handle_rendezvous wouldn't set the thread to sleep, the thread calls vm_run again, notices that a rendezvous is still in progress and enters vm_handle_rendezvous again. IMHO sleeping is better than looping.
A better solution to tackle this performance issue would be to avoid spurius rendezvous. D37390 is intended to do that.

sys/amd64/vmm/vmm.c
1803–1804

That's fine. I guess that info about sleeping should be added in commit message. Currently it has just mention "... vm_handle_rendezvous properly handles a spurious rendezvous".

Regarding the PR; bhyve has been in a boot-reboot-loop doing my test for more than 12 hours now without crash; haven't had that in the last weeks; so this seems to help.

sys/amd64/vmm/vmm.c
1803–1804

Keep in mind the places where rendezvouses are used. The INIT IPIs are only used during bootup and are not perf critical. The other use case is for dealing with level triggered I/O APIC interrupts and those rendezvous always go to all vCPUs, so you won't get spurious sleeping vCPUs for those. Given that, I don't think the spurious sleeps are worth worrying about.