systemd: Write rounded CPU quota to cgroupfs#4
Conversation
kolyshkin
left a comment
There was a problem hiding this comment.
- Why don't we just do
r = adjustResources(r)at the very beginning of Set methods? This way we won't have to call convertToCpuQuotaPerSecUsec twice.
- We don't have CI in this repo yet, so this have to wait (until at least #3 is merged).
|
I would also add a comment to |
When CPU quota is updated, the value is converted to CPUQuotaPerSecUSec property for passing to systemd. The value can be rounded in the following cases: - The value is rounded up to the nearest 10ms. - Depending on CPU period, the value may be rounded during division. Because of this rounding, systemd and systemd driver may write different values to cgroupfs. In order to avoid this inconsistency, this fix makes systemd driver write the same rounded value to cgroupfs. Even if systemd writes CPU quota and CPU period to cgroupfs, systemd driver still needs to write to cgroupfs for a case where CPU period is updated to less than the minimum value. In this case, systemd accepts this value by adjusting CPU period while direct update by systemd driver fails. In order to keep this case invalid, systemd driver still needs to update cgroupfs. Signed-off-by: Hironori Shiina <shiina.hironori@gmail.com>
|
@hshiina IIUC this didn't address the first comment here. Can you please address it? Or am I missing something? |
At the first revision, I introduced |
I removed |
|
Cool, sorry I missed it, I have not been following this PR. @kolyshkin PTAL :) |
|
@AkihiroSuda friendly ping? This fixes a bug people is hitting in Kubernetes |
|
Two LGTMs, but the merge button seems unclickable w/o checking "bypass rules" 🤔 |
|
Let me uncheck "Require branches to be up to date before merging" in https://github.com/opencontainers/cgroups/settings/branch_protection_rules/60411775 |
|
Do we need a release of this to include it in runc 1.3? |
|
I can make a release but I'd like other PRs to be merged first. |
|
Sure, this can be part of a patch release in 1.3 if we can't release this before final 1.3.0 :) |
|
I think all code-related PRs are merged (open PRs are more about CI tweaks). Do I smell a release soon? :-D |
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2 Fix integration tests according to changes in [1] (now the CPU quota value set is rounded the same way systemd does it). [1]: opencontainers/cgroups#4 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2 Fix integration tests according to changes in [1] (now the CPU quota value set is rounded the same way systemd does it). [1]: opencontainers/cgroups#4 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2 Fix integration tests according to changes in [1] (now the CPU quota value set is rounded the same way systemd does it). [1]: opencontainers/cgroups#4 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2 Fix integration tests according to changes in [1] (now the CPU quota value set is rounded the same way systemd does it). [1]: opencontainers/cgroups#4 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2 Fix integration tests according to changes in [1] (now the CPU quota value set is rounded the same way systemd does it). [1]: opencontainers/cgroups#4 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For changes, see https://github.com/opencontainers/cgroups/releases/tag/v0.0.2 Fix integration tests according to changes in [1] (now the CPU quota value set is rounded the same way systemd does it). [1]: opencontainers/cgroups#4 Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This is a carry of opencontainers/runc#4639.
When CPU quota is updated, the value is converted to
CPUQuotaPerSecUSecproperty for passing to systemd. The value can be rounded in the following cases:Because of this rounding, systemd and systemd driver may write different values to cgroupfs. In order to avoid this inconsistency, this fix makes systemd driver write the same rounded value to cgroupfs by calculating the value from
CPUQuotaPerSecUSec.Even if systemd writes CPU quota and CPU period to cgroupfs, systemd driver still needs to write to cgroupfs for a case where CPU period is updated to less than the minimum value. In this case, systemd accepts this value by adjusting CPU period while direct update by systemd driver fails. In order to keep this case invalid, systemd driver still needs to update cgroupfs.
Fixes opencontainers/runc#4622