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

Create UVM honoring NUMA configuration parameters #2198

Merged
merged 12 commits into from
Jul 17, 2024

Conversation

apurv15
Copy link
Contributor

@apurv15 apurv15 commented Jul 10, 2024

Expose NUMA config to containers. Use HCS device affinity so that UVM is configured on same NUMA node as the device. Expose SLIT configuration to UVM to gather NUMA node distances.

Changes under internal/hcs/schema2 are for using new HCS APIs related to NUMA HCS parameters and its dependencies. These files are auto-generated and could be skipped for a thorough review.
Other changes are business logic and should be reviewed thoroughly.

Some tests around device affinity and Hybrid/Virtual MemoryBackingType are pending. Creating PR to get early feedback.

@apurv15 apurv15 requested a review from a team as a code owner July 10, 2024 15:52
@katiewasnothere
Copy link
Contributor

Can we remove any schemas that aren't actively being used in this PR? I'm fine with including some extra fields for some of the leaf node schemas but for larger schemas like VirtualMachine I think we should remove anything we aren't using.

… is configured on same NUMA node as the device. Expose SLIT configuration to UVM to gather NUMA node distances.

Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

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

a few comments, similar to what @katiewasnothere already mentioned.

internal/uvm/create_lcow.go Outdated Show resolved Hide resolved
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
internal/uvm/vnuma.go Outdated Show resolved Hide resolved
internal/vm/hcs/pci.go Outdated Show resolved Hide resolved
Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
…or better readability

Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
internal/vm/hcs/pci.go Outdated Show resolved Hide resolved
Copy link
Contributor

@anmaxvl anmaxvl left a comment

Choose a reason for hiding this comment

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

LGTM with a few nits

Copy link
Contributor

@katiewasnothere katiewasnothere left a comment

Choose a reason for hiding this comment

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

LGTM after Maksim's nits are addressed

Signed-off-by: Apurv Barve <apurvbarve@microsoft.com>
@apurv15 apurv15 merged commit 8ad5a97 into microsoft:main Jul 17, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants