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

add HugePage info to memory and topology #374

Merged
merged 2 commits into from
Sep 18, 2024
Merged

Conversation

Traumeel
Copy link
Contributor

#371 fix

Add huge page info to Topology and Memory functionality.

Copy link
Owner

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

@Traumeel thank you so much for the PR! :) I left a few notes inline for you but overall, I like this quite a bit!

// Default system huge pages size, in bytes
DefaultHugePageSize uint64 `json:"default_huge_page_size"`
// Amount of memory, in bytes, consumed by huge pages of all sizes.
HugeTLBSize int64 `json:"huge_tlb_size"`
Copy link
Owner

Choose a reason for hiding this comment

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

the TLB is the lookaside buffer used for looking up the huge page physical location from a virtual address. It's also a Linux-specific thing and I'd prefer to keep the Area struct system-agnostic as much as possible.

How about we call this field TotalHugePageBytes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

SupportedPageSizes []uint64 `json:"supported_page_sizes"`
Modules []*Module `json:"modules"`
SupportedPageSizes []uint64 `json:"supported_page_sizes"`
// Default system huge pages size, in bytes
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
// Default system huge pages size, in bytes
// Default system huge page size, in bytes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -29,6 +29,15 @@ type Module struct {
Vendor string `json:"vendor"`
}

// HugePage describes huge page info
type HugePage struct {
Copy link
Owner

Choose a reason for hiding this comment

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

Let's call this struct HugePageAmounts since it's not describing the huge page itself but rather a set of distinct finite amounts of huge pages of a particular size within a memory area.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Maksym Aryefyev <maksym.aryefyev@gmail.com>
Signed-off-by: Maksym Aryefyev <maksym.aryefyev@gmail.com>
@Traumeel
Copy link
Contributor Author

@Traumeel thank you so much for the PR! :) I left a few notes inline for you but overall, I like this quite a bit!

@jaypipes thank you for the swift review) I am glad you like it!

Copy link
Owner

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

thanks @Traumeel !

@jaypipes jaypipes merged commit 30c83ba into jaypipes:main Sep 18, 2024
5 of 12 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.

2 participants