Skip to content
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

cgroupv2: set mem+swap to max if memory is set to max #2285

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions libcontainer/cgroups/fs2/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,21 @@ func numToStr(value int64) (ret string) {
}

func setMemory(dirPath string, cgroup *configs.Cgroup) error {
if val := numToStr(cgroup.Resources.MemorySwap); val != "" {
if err := fscommon.WriteFile(dirPath, "memory.swap.max", val); err != nil {
mem := numToStr(cgroup.Resources.Memory)
memSwap := numToStr(cgroup.Resources.MemorySwap)
if mem == "max" {
// If the memory is set to max, set the mem+swap to max, too
memSwap = "max"
}

if memSwap != "" {
if err := fscommon.WriteFile(dirPath, "memory.swap.max", memSwap); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to keep the same cgroup v1 behaviour (as the config.json file is configured for), we need to set the memory swap to requested_memory_limit - requested_swap_limit. Since on cgroup v1 the swap limit accounts also for the memory

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, in this case maybe we don't need to touch swap at all, and this PR should be closed, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes setting the memory.max should be enough on cgroup v2 as the swap max should not have any effect on it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but since the PR is open, I think we could use it for setting the swap limit as it is expected to work on cgroup v1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #2288

return err
}
}
if val := numToStr(cgroup.Resources.Memory); val != "" {
if err := fscommon.WriteFile(dirPath, "memory.max", val); err != nil {

if mem != "" {
if err := fscommon.WriteFile(dirPath, "memory.max", mem); err != nil {
return err
}
}
Expand Down