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

rust: Add topology module to utils crate #143

Merged
merged 2 commits into from
Feb 20, 2024
Merged

rust: Add topology module to utils crate #143

merged 2 commits into from
Feb 20, 2024

Conversation

Byte-Lab
Copy link
Contributor

scx_rusty has logic in the scheduler to inspect the host to automatically build scheduling domains across every L3 cache. This would be generically useful for many different types of schedulers, so let's add it to the scx_utils crate so it can be used by others.

@Byte-Lab Byte-Lab requested a review from htejun February 20, 2024 17:18
@Byte-Lab
Copy link
Contributor Author

This could probably be a done a bit cleaner in terms of encapsulation, etc, but should hopefully be good enough for now. We can further clean up once we have more callers.

Copy link
Contributor

@arighi arighi left a comment

Choose a reason for hiding this comment

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

Thanks for this, I'm definitely going to reuse it in scx_rustland!

BTW, I think it would be really helpful to add at the top a simple description of how to use this crate, maybe with a simple example, like how do I initialize a structure that allows me to determine the list of siblings of a specific CPU? Or how can I use the crate to print the CPU topology of my system?

@Byte-Lab
Copy link
Contributor Author

Thanks for this, I'm definitely going to reuse it in scx_rustland!

BTW, I think it would be really helpful to add at the top a simple description of how to use this crate, maybe with a simple example, like how do I initialize a structure that allows me to determine the list of siblings of a specific CPU? Or how can I use the crate to print the CPU topology of my system?

Ack, I'll add that!

@arighi
Copy link
Contributor

arighi commented Feb 20, 2024

Also, nice that you triggered a bug with rustland 😄

thread 'main' panicked at src/bpf.rs:162:43:
misaligned pointer dereference: address must be a multiple of 0x8 but is 0x56516a51e004
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Good thing we have CI (looking at the bug).

scx_rusty has logic in the scheduler to inspect the host to
automatically build scheduling domains across every L3 cache. This would
be generically useful for many different types of schedulers, so let's
add it to the scx_utils crate so it can be used by others.

Signed-off-by: David Vernet <[email protected]>
Signed-off-by: David Vernet <[email protected]>
@Byte-Lab
Copy link
Contributor Author

Byte-Lab commented Feb 20, 2024

@arighi I added quite a bit of documentation to the crate. I'm going to merge this for now, but please feel free to take a look and provide any additional suggestions, improvements, etc. A couple of things that I want to highlight:

  1. I view this very much as in an intermediate state. It should be usable and correct by schedulers, but I think we need to make it a lot more generic. For example, by not allowing the user to create a Topology object from cpumasks, and to instead just always create a fully descriptive representation of the host topology.

  2. It doesn't yet have support for identifying NUMA nodes.

With both of those said, the APIs here are subject to change, and likely will fairly soon. Of course, please feel free to use it for scx_rustland in the intermediate. We can update whichever schedulers are using it as required when the APIs change.

@Byte-Lab Byte-Lab merged commit 8814286 into main Feb 20, 2024
1 check passed
@Byte-Lab Byte-Lab deleted the rusty_topology branch February 20, 2024 21:16
@arighi
Copy link
Contributor

arighi commented Feb 20, 2024

@Decave I think it looks good for now. I'll try to see if I can reuse something in rustland even if the API is not stable. That's totally fine, we can improve and change it later in the future. Thanks!

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.

3 participants