Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[performance] Move set_up_hypervisor_partition to inside the hv handler thread #26

Merged

Conversation

danbugs
Copy link
Contributor

@danbugs danbugs commented Nov 6, 2024

As per KVM docs, vCPU ioctls should be issued from the same thread that was used to create the vCPU. In accordance to that, this PR moves set_up_hypervisor_partition to inside the hv handler thread, which manages vCPU interactions like init and dispatching guest funciton calls.

@danbugs danbugs force-pushed the move-hv-partition-setup-to-hv-handler branch from 1c4354a to 3e2bb6a Compare November 6, 2024 23:44
@danbugs danbugs changed the title [performance] Move set_up_hypervisor_partition to inside the hv handler thread [performance/github-actions] Move set_up_hypervisor_partition to inside the hv handler thread and increase our test matrix to include intel runners Nov 7, 2024
@danbugs danbugs changed the title [performance/github-actions] Move set_up_hypervisor_partition to inside the hv handler thread and increase our test matrix to include intel runners [performance/github-actions] Move set_up_hypervisor_partition to inside the hv handler thread and increase our test matrix to include Intel runners Nov 7, 2024
@danbugs danbugs force-pushed the move-hv-partition-setup-to-hv-handler branch 2 times, most recently from 9ec82f1 to 3e2bb6a Compare November 7, 2024 02:04
@danbugs danbugs changed the title [performance/github-actions] Move set_up_hypervisor_partition to inside the hv handler thread and increase our test matrix to include Intel runners [performance/github-actions] Move set_up_hypervisor_partition to inside the hv handler thread Nov 7, 2024
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job, I will be intersting to see if this increases performance :D. Just some minor questions

EDIT: also noticed this PR has github-actions in the PR title which I think might be inaccurate.

src/hyperlight_host/src/hypervisor/hypervisor_handler.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/hypervisor/hypervisor_handler.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/hypervisor/hypervisor_handler.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/hypervisor/kvm.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/mem/shared_mem.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@syntactically syntactically left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be simplified a little bit, and avoid some extra synchronisation and potential issues with aliasing the shared memory.

src/hyperlight_host/src/hypervisor/hypervisor_handler.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/hypervisor/hypervisor_handler.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/hypervisor/hypervisor_handler.rs Outdated Show resolved Hide resolved
src/hyperlight_host/src/mem/shared_mem.rs Outdated Show resolved Hide resolved
@danbugs danbugs changed the title [performance/github-actions] Move set_up_hypervisor_partition to inside the hv handler thread [performance] Move set_up_hypervisor_partition to inside the hv handler thread Nov 25, 2024
@danbugs danbugs force-pushed the move-hv-partition-setup-to-hv-handler branch 2 times, most recently from 4a56fcf to ff1ec39 Compare November 26, 2024 00:09
Copy link
Contributor

@syntactically syntactically left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

…er thread

As per KVM docs, vCPU ioctls should be issued from the same thread that was used to create the vCPU.
In accordance to that, this PR moves set_up_hypervisor_partition to inside the hv handler thread,
which manages vCPU interactions like init and dispatching guest funciton calls.

Signed-off-by: danbugs <[email protected]>
@danbugs danbugs force-pushed the move-hv-partition-setup-to-hv-handler branch from ff1ec39 to e39936a Compare November 26, 2024 17:45
@danbugs danbugs merged commit e39936a into hyperlight-dev:main Nov 26, 2024
45 checks passed
@danbugs danbugs deleted the move-hv-partition-setup-to-hv-handler branch November 26, 2024 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants