Repository navigation
Conversation
kolyshkin
force-pushed
the
systemd-cpu-quota-overflow
branch
2 times, most recently
from
October 2, 2026 06:43
6cbfe93 to
fa7c9e6
Compare
kolyshkin
changed the title
kolyshkin
force-pushed
the
systemd-cpu-quota-overflow
branch
from
October 2, 2026 17:35
1b9eeab to
8e69244
Compare
addCPUQuota converts CFS quota to CPUQuotaPerSecUSec, and, if the value is rounded up, converts it back to quota. Both conversions are done using int64 multiplication, which overflows for a large quota. As a result, a large quota is converted to a tiny (or negative) value, which is either rejected by the kernel or means "unlimited" (e.g. quota of 9223372036854700 becomes 290, and 10000000000001 becomes -8446744072709). Fix this by: - doing the calculations in uint64; - rearranging the write-back calculation to avoid an overflow; - treating a quota above maxCPUQuota (the kernel limit of 2^44-1 us, which is low enough for quota * 1000000 to fit into uint64), either requested or resulting from the round-up, as unlimited, for both systemd and cgroupfs. Such quota is rejected by the kernel anyway, and it is effectively unlimited. Fixes opencontainers#73. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
systemd-cpu-quota-overflow
branch
from
October 2, 2026 17:38
8e69244 to
7b24f04
Compare
thaJeztah
reviewed
Oct 2, 2026
| name: "Quota too large", | ||
| quota: maxCPUQuota + 1, | ||
| expectedCPUQuotaPerSecUSec: math.MaxUint64, | ||
| expectedQuota: -1, |
Member
There was a problem hiding this comment.
Perhaps silly but should we define a const for the magic -1 so that it's clear we're talking about "unlimited" (not an accidental negative)
Contributor
Author
There was a problem hiding this comment.
I'd keep this as is -- in cgroupfs, -1 always means "unlimited" and the value is used in this repo quite a lot of times. If we want to change it, let's change it everywhere (and not in this PR).
thaJeztah
reviewed
Oct 2, 2026
| }, | ||
| { | ||
| name: "cpu.max", | ||
| minVer: 242, // For CPUQuotaPeriodUSec. |
Member
There was a problem hiding this comment.
The other PR added consts for this I think?
Contributor
Author
There was a problem hiding this comment.
Right! Thanks, fixed.
Commit 83f2519 made the systemd driver write the rounded CPU quota to cgroupfs, to keep it consistent with what systemd writes. This was not done for the quota set via Unified["cpu.max"], so the rounded value is passed to systemd, while the original value is written to cgroupfs. Similarly, a quota too large for the kernel is now passed to systemd as unlimited, but is written to cgroupfs as is, resulting in EINVAL. Fix this by updating res["cpu.max"] in unifiedResToSystemdProps if addCPUQuota modified the quota. To avoid modifying the caller's map, clone r.Unified in UnifiedManager.Set (where r is already copied for the same reason). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin
force-pushed
the
systemd-cpu-quota-overflow
branch
from
October 2, 2026 20:12
7b24f04 to
0a10b5b
Compare
Contributor
Author
|
@thaJeztah ptal |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
systemd: fix CPU quota overflow in addCPUQuota
addCPUQuota converts CFS quota to CPUQuotaPerSecUSec, and, if the value
is rounded up, converts it back to quota. Both conversions are done
using int64 multiplication, which overflows for a large quota. As a
result, a large quota is converted to a tiny (or negative) value, which
is either rejected by the kernel or means "unlimited" (e.g. quota of
9223372036854700 becomes 290, and 10000000000001 becomes -8446744072709).
Fix this by:
which is low enough for quota * 1000000 to fit into uint64), either
requested or resulting from the round-up, as unlimited, for both
systemd and cgroupfs. Such quota is rejected by the kernel anyway,
and it is effectively unlimited.
Fixes #73.
systemd: write rounded cpu.max quota to cgroupfs
Commit 83f2519 made the systemd driver write the rounded CPU quota to
cgroupfs, to keep it consistent with what systemd writes. This was not
done for the quota set via Unified["cpu.max"], so the rounded value
is passed to systemd, while the original value is written to cgroupfs.
Similarly, a quota too large for the kernel is now passed to systemd
as unlimited, but is written to cgroupfs as is, resulting in EINVAL.
Fix this by updating res["cpu.max"] in unifiedResToSystemdProps if
addCPUQuota modified the quota. To avoid modifying the caller's map,
clone r.Unified in UnifiedManager.Set (where r is already copied for
the same reason).