libcontainer/intelrdt: support ClosID parameter #2920
Conversation
|
Looks good overall, added some notes. Also, the first commit performs a nice refactoring (which I like), but it is kind of hard to review because it also introduces ClosID support. Can you maybe split it into (1) refactoring (2) adding ClosID? |
|
Thanks for the review @kolyshkin!
Yeah, I see 😊 I now split the first commit into two |
|
BTW, related, I submitted a PR against runtime-spec in an attempt to make it easier read: |
|
Ping @kolyshkin. Could you enable testing? |
|
Rebased and added validation for ClosID parameter ping @kolyshkin |
|
@kolyshkin anything I can do to advance this? The latest review comment(s) should be addressed, now |
|
ping @kolyshkin |
|
How to re-run tests?? |
|
ping @kolyshkin. How to get this forward? |
|
We need to manually retrigger them, there isn't a bot to control them unfortunately. But you can retrigger it by force pushing a new commit ( (I've retriggered it.) |
One other way is to close and reopen a PR. |
Handle ClosID parameter of IntelRdt. Makes it possible to use pre-configured classes/ClosIDs and avoid running out of available IDs which easily happens with per-container classes. Remove validator checks for empty L3CacheSchema and MemBwSchema fields in order to be able to leave them empty, and only specify ClosID for a pre-configured class. Signed-off-by: Markus Lehtonen <[email protected]>
Check that the ClosID directory pre-exists if no L3 or MB schema has been specified. Conform with the following line from runtime-spec (config-linux): If closID is set, and neither of l3CacheSchema and memBwSchema are set, runtime MUST check if corresponding pre-configured directory closID is present in mounted resctrl. If such pre-configured directory closID exists, runtime MUST assign container to this closID and generate an error if directory does not exist. Add a TODO note for verifying existing schemata against L3/MB parameters. Signed-off-by: Markus Lehtonen <[email protected]>
|
Rebased |
Run unit tests irrespective of the underlying system configuration, i.e. even if RDT has not been enabled or is not supported. The tests do not depend on real kernel interfaces. Signed-off-by: Markus Lehtonen <[email protected]>
|
Re-triggered tests by force push as there was some spurious test failure in i386 😕 |
|
ping @crosbymichael @mrunalp @dqminh @hqhq @cyphar @AkihiroSuda @kolyshkin ❗️ This starts to be critical as RDT support was merged in CRI-O and we really need the runc to support it, too. |
libcontainer/intelrdt/intelrdt.go
Outdated
| @@ -532,14 +518,18 @@ func IsMBAScEnabled() bool { | |||
| } | |||
|
|
|||
| // Get the 'container_id' path in Intel RDT "resource control" filesystem | |||
There was a problem hiding this comment.
Perhaps “container_id” -> “ClosID” ?
There was a problem hiding this comment.
Good catch, thanks. I addressed this in a separate patch, modifying some other code comments as well. I used the term clos group
Use the term "clos group" instead of "container_id group" as the group that a container belongs to is not necessarily tied to its container id. Signed-off-by: Markus Lehtonen <[email protected]>
|
Signed-off-by was missing from the last patch 😭 |
|
ping @kolyshkin |
Handle ClosID parameter of IntelRdt. Makes it possible to use
pre-configured classes/ClosIDs and avoid running out of available IDs
which easily happens with per-container classes.
libcontainer/intelrdt: verify ClosID existence
Check that the ClosID directory pre-exists if no L3 or MB schema has
been specified. Conform with the following line from runtime-spec
(config-linux):
If closID is set, and neither of l3CacheSchema and memBwSchema are
set, runtime MUST check if corresponding pre-configured directory
closID is present in mounted resctrl. If such pre-configured directory
closID exists, runtime MUST assign container to this closID and
generate an error if directory does not exist.
Add a TODO note for verifying existing schemata against L3/MB
parameters.
Run unit tests irrespective of the underlying system configuration, i.e.
even if RDT has not been enabled or is not supported. The tests do not
depend on real kernel interfaces.