-
Notifications
You must be signed in to change notification settings - Fork 593
config-linux: define default clos for linux.intelRdt #1289
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
config-linux: define default clos for linux.intelRdt #1289
Conversation
aa75def to
ad23560
Compare
config-linux.md
Outdated
| The following parameters can be specified for the container: | ||
|
|
||
| * **`closID`** *(string, OPTIONAL)* - specifies the identity for RDT Class of Service (CLOS). | ||
| As a special case, value `.` means that the container should be assigned to the default CLOS (the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would '/' be more explicit? The value '.' looks to me like the expectation is the "current CLOS"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with this, too. I was struggling with the name myself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No other comments so I now changed this to /.
ad23560 to
2cfef25
Compare
|
Updated: as per @giuseppe's suggestion, changed name of the root/default group to |
config-linux.md
Outdated
| The following parameters can be specified for the container: | ||
|
|
||
| * **`closID`** *(string, OPTIONAL)* - specifies the identity for RDT Class of Service (CLOS). | ||
| As a special case, value `/` means that the container should be assigned to the default CLOS (the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Changed to MUST
Specify "/" as an explicit value for linux.intelRdt.closID to assign a container to the default CLOS, corresponding to the root of the resctrl filesystem. This addition is important after the recently introduced intelRdt.enableMonitoring field. There is no way to express "enable monitoring but keep the container in the default CLOS". Users would otherwise have to rely on pre-created CLOSes or may quickly exhaust available CLOS entries - in some configurations the number of available CLOSes (on top of the default) may be as low as three. Signed-off-by: Markus Lehtonen <[email protected]>
2cfef25 to
e51a839
Compare
Makes it possible e.g. to enable monitoring (linux.intelRdt.enableMonitoring) without creating a CLOS (resctrl group) for the container. Implements opencontainers/runtime-spec#1289. Signed-off-by: Markus Lehtonen <[email protected]>
kolyshkin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
implement the change specified here: opencontainers/runtime-spec#1289 Signed-off-by: Giuseppe Scrivano <[email protected]>
implement the change specified here: opencontainers/runtime-spec#1289 Signed-off-by: Giuseppe Scrivano <[email protected]>
Makes it possible e.g. to enable monitoring (linux.intelRdt.enableMonitoring) without creating a CLOS (resctrl group) for the container. Implements opencontainers/runtime-spec#1289. Signed-off-by: Markus Lehtonen <[email protected]>
Makes it possible e.g. to enable monitoring (linux.intelRdt.enableMonitoring) without creating a CLOS (resctrl group) for the container. Implements opencontainers/runtime-spec#1289. Signed-off-by: Markus Lehtonen <[email protected]>
implement the change specified here: opencontainers/runtime-spec#1289 Signed-off-by: Giuseppe Scrivano <[email protected]>
implement the change specified here: opencontainers/runtime-spec#1289 Signed-off-by: Giuseppe Scrivano <[email protected]>
Enables:
opencontainers/runtime-spec#1289
Signed-off-by: Markus Lehtonen <[email protected]>
Specify "/" as an explicit value for linux.intelRdt.closID to assign a container to the default CLOS, corresponding to the root of the resctrl filesystem.
This addition is important after the recently introduced intelRdt.enableMonitoring field. There is no way to express "enable monitoring but keep the container in the default CLOS". Users would otherwise have to rely on pre-created CLOSes or may quickly exhaust available CLOS entries - in some configurations the number of available CLOSes (on top of the default) may be as low as three.
Note
Alternative names I considered were e.g.
.,/default,/root,default/,root/(i.e. something that is not a possible/valid > name in the resctrl fs).