Skip to content

Conversation

jwake
Copy link

@jwake jwake commented May 14, 2025

Added parsing code for /proc/buddyinfo, and unit tests for the parser.

Also added a Pages type over u64 to make it more obvious that the data in /proc/buddyinfo/ is measured in terms of memory pages and not bytes, kilobytes or anything else, rather than making the parsing code also query the system page size. I'm a little torn about whether or not to add conversions eg. from Pages to usize that factor in the current system page size, or leave it up to the API consumer.

Justin Wake added 2 commits May 14, 2025 10:10
Adds a Pages newtype over u64 for type-safe storage of quantities of memory pages.

Mark /proc/buddyinfo as supported.
@eminence
Copy link
Owner

Sorry for taking so long to review this. It looks good to me. I don't understand the CI failures, and they happened so long ago that I can't retry them.

I'm a little torn about whether or not to add conversions eg. from Pages to usize that factor in the current system page size, or leave it up to the API consumer.

There is some precedent for this, for example in the rss_bytes API. I don't love this type of API because with the current separation of procfs-core and procfs, the interface ends up a little awkward and it would probably be easier for the API consumer to just get the page size and do the multiplication themselves. But it's there.

}
}

wrap_numeric!(Pages, u64, "A quantity of pages of memory");
Copy link
Owner

Choose a reason for hiding this comment

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

I think this is missing some bits for handling the serde stuff:

error[E0277]: the trait bound `types::Pages: Deserialize<'_>` is not satisfied                                                                                                                
    --> procfs-core/src/buddyinfo.rs:20:18                                                     
     |                                                                                                                                                                                        
20   |     free_chunks: HashMap<Pages, u64>,                                                   
     |                  ^^^^^^^^^^^^^^^^^^^ the trait `Deserialize<'_>` is not implemented for `types::Pages`

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