-
Notifications
You must be signed in to change notification settings - Fork 930
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
WIP: Add limits.cpu.pin_strategy
setting to disable CPU auto pinning by default
#14171
base: main
Are you sure you want to change the base?
WIP: Add limits.cpu.pin_strategy
setting to disable CPU auto pinning by default
#14171
Conversation
460df45
to
ffe7a87
Compare
Heads up @mionaalex - the "Documentation" label was applied to this issue. |
ffe7a87
to
bc4de3b
Compare
ab52340
to
96fdbdf
Compare
limits.cpu.pin_strategy
setting to disable VM CPU auto pinning by defaultlimits.cpu.pin_strategy
setting to disable VM CPU auto pinning by default
96fdbdf
to
a8e75ba
Compare
If we modify the current tests in Currently, the checks in if journalctl --quiet --no-hostname --no-pager --boot=0 --unit=snap.lxd.daemon.service | grep "Scheduler: virtual-machine"; then
... However, due to my implementation of checking the config setting in if c.Type() == instancetype.VM {
if conf["limits.cpu.pin_strategy"] == "none" {
continue
} else if conf["limits.cpu.pin_strategy"] == "auto" {
logger.Debugf("CPU auto pinning enabled for %q", c.Name())
}
} Therefore, I think the only change required (for now) to ensure we don't introduce a regression to For reference: canonical/lxd-ci@972ef98. Please let me know if you have any suggestions 😄 |
a8e75ba
to
d83c50c
Compare
You will need to introduce a new API extension for the new config key and can test for support of that when testing to enable the functionality. |
d83c50c
to
7d85357
Compare
@kadinsayani I couldn't find any reference to back that but I think it's best for the commit adding the API extension to be the first in the PR. This way, when @tomponline does the backporting (if any, dunno for this one) he can quickly identify that a collection of commits need to be considered at once for the backport. |
Sounds good, thanks for letting me know! |
7d85357
to
0ca7deb
Compare
@tomponline will we be back porting the commit containing the new API extension to 6.1? Currently, the tests in https://github.com/mihalicyn/lxd-ci/blob/972ef98d660c3c2efba3d8aa7408126a1b555ff9/tests/cpu-vm#L12 |
yes we can because it defaults to disabled |
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
@kadinsayani please can you ensure that the change to disable auto pin for containers is separate so i can choose to backport it or not. |
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Unless `limits.cpu.pin_strategy` is set to auto, VM CPU auto pinning is disabled Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
Signed-off-by: Kadin Sayani <kadin.sayani@canonical.com>
0ca7deb
to
3c071ad
Compare
limits.cpu.pin_strategy
setting to disable VM CPU auto pinning by defaultlimits.cpu.pin_strategy
setting to disable CPU auto pinning by default
limits.cpu.pin_strategy
setting to disable CPU auto pinning by defaultlimits.cpu.pin_strategy
setting to disable CPU auto pinning by default
Fixes #14133.
This pull request introduces a configuration option for instances,
limits.cpu.pin_strategy
. This option allows users to specify the strategy for CPU auto pinning. The possible values arenone
(disables CPU auto pinning - also the new default) andauto
(enables CPU auto pinning).