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.

Sign up for GitHub

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

Jump to bottom

Add support to Intel RDT. #1822

Merged
merged 2 commits into from
Apr 27, 2023
Merged

Add support to Intel RDT. #1822

merged 2 commits into from
Apr 27, 2023

Conversation

ipuustin
Copy link
Contributor

Add support to Intel RDT.

Fixes: #443.

This PR implements also the suggested additions to OCI runtime spec here: opencontainers/runtime-spec#1196

@codecov-commenter
Copy link

Codecov Report

Merging #1822 (a0c5640) into main (1faa881) will increase coverage by 0.01%.
The diff coverage is 68.43%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1822      +/-   ##
==========================================
+ Coverage   68.63%   68.64%   +0.01%     
==========================================
  Files         121      122       +1     
  Lines       13327    13606     +279     
==========================================
+ Hits         9147     9340     +193     
- Misses       4180     4266      +86

@utam0k
Copy link
Member

utam0k commented Apr 18, 2023

@ipuustin Thanks for your PR. Since I need some time to learn Interl RDT, I'll check this next weekend.

@ipuustin ipuustin force-pushed the intel-rdt branch 3 times, most recently from 8ea32da to 50a9ccd Compare April 21, 2023 09:53
@ipuustin
Copy link
Contributor Author

Note that this PR is needed to oci-spec-rs in order for the closID spec setting to have any effect: containers/oci-spec-rs#128

utam0k
utam0k reviewed Apr 23, 2023
View reviewed changes
crates/libcontainer/Cargo.toml Outdated
@@ -42,6 +42,8 @@ serde_json = "1.0"
syscalls = "0.6.9"
rust-criu = "0.4.0"
clone3 = "0.2.3"
regex = "1.7.3"
lazy_static = "1.4.0"
Copy link
Member

Choose a reason for hiding this comment

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

Since we are using once_cell, please use it instead of lazy_static 🙏

utam0k
utam0k reviewed Apr 23, 2023
View reviewed changes
crates/libcontainer/src/process/container_main_process.rs Outdated
@@ -69,6 +72,7 @@ pub fn container_main_process(container_args: &ContainerArgs) -> Result<Pid> {
// The intermediate process will send the init pid once it forks the init
// process. The intermediate process should exit after this point.
let init_pid = main_receiver.wait_for_intermediate_ready()?;
let mut need_to_clean_up_intel_rdt_subdirectory: bool = false;
Copy link
Member

Choose a reason for hiding this comment

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

nits

Suggested change
let mut need_to_clean_up_intel_rdt_subdirectory: bool = false;
let mut need_to_clean_up_intel_rdt_subdirectory = false;

utam0k
utam0k requested changes Apr 23, 2023
View reviewed changes
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

Wait for updating oci-spec-rs

@ipuustin
Copy link
Contributor Author

@utam0k Thanks for reviewing! I implemented the suggested changes.

@utam0k
Copy link
Member

utam0k commented Apr 24, 2023

@ipuustin Thanks 😍 May I ask you to give me the instructions to confirm the behavior you have implemented?

@ipuustin
Copy link
Contributor Author

The documentation of the kernel resctrl interface (which this PR uses) is here: https://docs.kernel.org/x86/resctrl.html

Briefly, you mount the resctrl filesystem (mount -t resctrl resctrl /sys/fs/resctrl) and then you should be set. The code doesn't really concern itself with what the resctrl actually does: it just maps the OCI config values to the /sys/fs/resctrl/<container_id>/schemata file, with the instructions speficied in the OCI spec. In case you specify the closID value, the changes are done to /sys/fs/resctrl/<clos_id>/schemata file instead (if the directory doesn't exist already, otherwise the proposed changes will just be compared with the existing schemata file).

Which changes can you do to the schemata files depend on the computer system you have in use. A good approximation would be to see what the kernel has prepopulated to the schemata files and try to modify those values.

The OCI config spec has this example for the config.json file:

"linux": {
    "intelRdt": {
        "closID": "guaranteed_group",
        "l3CacheSchema": "L3:0=7f0;1=1f",
        "memBwSchema": "MB:0=20;1=70"
    }
}

@ipuustin
Intel RDT OCI support.
5241604 
Signed-off-by: Ismo Puustinen <[email protected]>
@ipuustin
Add integration tests for Intel RDT feature.
359744b 
Note: this requires resctrl filesystem to be mounted.

Signed-off-by: Ismo Puustinen <[email protected]>
@ipuustin
Copy link
Contributor Author

Rebased to fix conflict in Cargo.lock.

utam0k
utam0k approved these changes Apr 27, 2023
View reviewed changes
Copy link
Member

@utam0k utam0k left a comment

Choose a reason for hiding this comment

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

💯

@utam0k utam0k merged commit 7d9d091 into containers:main Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@utam0k utam0k utam0k approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for Intelrdt
3 participants
@ipuustin @codecov-commenter @utam0k