topology-aware: add CPU class support. - #729
Conversation
65af27a to
a216b23
Compare
askervin
left a comment
askervin
left a comment
There was a problem hiding this comment.
Adding one note... I'll continue review soon...
| errs = append(errs, err) | ||
| } | ||
|
|
||
| if c.SharedCpuClass == "" { |
There was a problem hiding this comment.
For convenience, balloons policy handles CPU class name "default" differently than others: if a balloon type does not specify any CPU class, the "default" CPU class will be applied on CPUs of its instances.
https://containers.github.io/nri-plugins/stable/docs/resource-policy/policy/balloons.html#cpu-tuning
I was thinking if it would make sense to use c.{Shared,Reserved,DefaultExclusive}CpuClass = "default" when a user has specified "default" CPU class. Pros: one could use the same CPU class configuration if evaluating and switching between policies, and it might be more intuitive than exclusive allocations using sharedCPUClass by default. Cons: if the existence of the "default" CPU class is not required and the current mechanism for figuring out a default class would be used in that case, it would make make the whole thing unnecessarily complex.
Having special semantics for a class named "default" fits quite naturally in balloons, as there is the "default" balloon type, too. In T-A, there is no similar context, so this might not be a good idea.
There was a problem hiding this comment.
and it might be more intuitive than exclusive allocations using sharedCPUClass by default.
Well, we don't do that ( that explicitly) by default, although that is the effective end result (for non-isolated CPUs) if there is no DefaultExclusiveCpuClass set and the CPUs were ever part of a shared pool after which they get assigned exclusively to a container which does not have an annotated class.
Maybe we could use a defined default class as DefaultExclusiveCpuClass if one is not given, and log this fact. Then again, if we did that then there would be 2 ways of setting a default class, and one of them would be quite implicit, not obvious without reading the documentation: does default apply by default to all CPUs, shared CPUs only, exclusively allocated CPUs only, or something else ?
There was a problem hiding this comment.
@askervin I don't have too strong opinions about this, but so far I have not implemented this additional implicit semantic interpretation if a default CPU class is defined. Mostly because I did not want multiple ways for defining a default exclusive CPU class.
Let's think more about this, and let me know if you still feel it would make sense. Related to this, I thing I'm still considering is to change the pool-related CPU class configuration variables from {shared,reserved}CPUClass to {shared,reserved}PoolCPUClass to be more explicit in the naming about their semantics...
7641336 to
2d213d9
Compare
Avoid using 'balloon' in cpuclass configuration comments / description. Use the more neutral 'policy' term. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Always debug log cpufreq class enforcement intent. This helps with log-based verification in e2e tests with repeated class assignments. Other otherwise some assignment of a cpufreq-only class could be omitted from logs if it does not change cpufreq state for a CPU. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
2d213d9 to
a70767f
Compare
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Rework pool CPU allocation, splitting out exclusive and shared CPU picking logic to separate functions and making them dry-runnable. This should allow scoring to evaluate the actual pool offered CPUs at will. Add pool/supply.GetCPUOffer() which dry-runs pool CPU allocation for a given request. Rename existing GetOffer() to GetMemOffer() for symmetricity/better consistency. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
c5916d0 to
9458c6d
Compare
Add basic CPU class support. Exlusively allocated CPUs (of Guaranteed QoS class containers), CPUs of shared pools and CPUs of the reserved pool can be assigned to CPU classes. Shared and reserved pool classes are configured. Exclusive CPU classes can be configured (default class) or annotated using an effective annotation with key 'cpu-class' on the container's pod. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Grant the policy access to node extended resources, and host devices for SST access if PCT is enabled. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
9458c6d to
815c092
Compare
Move CPU class definition documentation to common functionality and reference it from the balloons documentation and howto's. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add minimal documentation about the CPU class functionality present in the topology-aware policy. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
815c092 to
0aa79b3
Compare
|
Closing in favor of stacked #742 |
2 participants
Please, review/merge #736 first.
This patch series adds basic CPU class support to the topology-aware policy.
With these patches in place, CPU classes can now be assigned to
CPU class support is turned on by defining some CPU classes in the configuration and specifying at least the CPU class for shared pool by setting
sharedCPUClassin the configuration. This is the minimal valid configuration.Additionally, an optional reserved pool CPU class can be specified for CPUs of the reserved pool by setting
reservedCPUClassin the configuration. An unset reserved pool class defaults to the shared pool class. Furthermore, also optionally a default CPU class for exclusive CPUs can be specified by settingdefaultExclusiveCPUClassin the configuration. If set exclusive CPUs of otherwise unnannotated containers are assigned to the default exclusive class.Containers can also be explicitly annotated with a CPU class using the
cpu-class[.resource-policy.nri.io]effective annotation base key. Exclusive CPUs of annotated containers are assigned to their annotated class. Once CPUs are released from exclusive use and returned to a shared pool, they are (re-)assigned to the shared pool CPU class.Note: This PR is stacked on top of #736. It is not github-stacked because (I think) PRs can't be github-stacked if the PR branches are not originating from the same source repository.