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: use "max" for negative values #2272

Merged
merged 1 commit into from
Mar 26, 2020

Conversation

kolyshkin
Copy link
Contributor

Cgroup v1 kernel doc [1] says:

We can write "-1" to reset the *.limit_in_bytes(unlimited).

and cgroup v2 kernel documentation [2] says:

  • If a controller implements an absolute resource guarantee and/or
    limit, the interface files should be named "min" and "max"
    respectively. If a controller implements best effort resource
    guarantee and/or limit, the interface files should be named "low"
    and "high" respectively.

In the above four control files, the special token "max" should be
used to represent upward infinity for both reading and writing.

Allow -1 value to still be used for v2, converting it to "max"
where it makes sense to do so.

This fixes the following issue:

runc update test_update --memory-swap -1:
error while setting cgroup v2: [write /sys/fs/cgroup/machine.slice/runc-cgroups-integration-test.scope/memory.swap.max: invalid argument
failed to write "-1" to "/sys/fs/cgroup/machine.slice/runc-cgroups-integration-test.scope/memory.swap.max"
github.com/opencontainers/runc/libcontainer/cgroups/fscommon.WriteFile
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fscommon/fscommon.go:21
github.com/opencontainers/runc/libcontainer/cgroups/fs2.setMemory
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fs2/memory.go:20
github.com/opencontainers/runc/libcontainer/cgroups/fs2.(*manager).Set
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fs2/fs2.go:175
github.com/opencontainers/runc/libcontainer/cgroups/systemd.(*UnifiedManager).Set
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/systemd/unified_hierarchy.go:290
github.com/opencontainers/runc/libcontainer.(*linuxContainer).Set
/home/kir/go/src/github.com/opencontainers/runc/libcontainer/container_linux.go:211

[1] linux/Documentation/admin-guide/cgroup-v1/memory.rst
[2] linux/Documentation/admin-guide/cgroup-v2.rst

@AkihiroSuda @mrunalp PTAL

Cgroup v1 kernel doc [1] says:

> We can write "-1" to reset the ``*.limit_in_bytes(unlimited)``.

and cgroup v2 kernel documentation [2] says:

> - If a controller implements an absolute resource guarantee and/or
>  limit, the interface files should be named "min" and "max"
>  respectively.  If a controller implements best effort resource
>  guarantee and/or limit, the interface files should be named "low"
>  and "high" respectively.
>
>  In the above four control files, the special token "max" should be
>  used to represent upward infinity for both reading and writing.

Allow -1 value to still be used for v2, converting it to "max"
where it makes sense to do so.

This fixes the following issue:

> runc update test_update --memory-swap -1:
> error while setting cgroup v2: [write /sys/fs/cgroup/machine.slice/runc-cgroups-integration-test.scope/memory.swap.max: invalid argument
> failed to write "-1" to "/sys/fs/cgroup/machine.slice/runc-cgroups-integration-test.scope/memory.swap.max"
> github.com/opencontainers/runc/libcontainer/cgroups/fscommon.WriteFile
> 	/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fscommon/fscommon.go:21
> github.com/opencontainers/runc/libcontainer/cgroups/fs2.setMemory
> 	/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fs2/memory.go:20
> github.com/opencontainers/runc/libcontainer/cgroups/fs2.(*manager).Set
> 	/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/fs2/fs2.go:175
> github.com/opencontainers/runc/libcontainer/cgroups/systemd.(*UnifiedManager).Set
> 	/home/kir/go/src/github.com/opencontainers/runc/libcontainer/cgroups/systemd/unified_hierarchy.go:290
> github.com/opencontainers/runc/libcontainer.(*linuxContainer).Set
> 	/home/kir/go/src/github.com/opencontainers/runc/libcontainer/container_linux.go:211

[1] linux/Documentation/admin-guide/cgroup-v1/memory.rst
[2] linux/Documentation/admin-guide/cgroup-v2.rst

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@crosbymichael
Copy link
Member

crosbymichael commented Mar 26, 2020

LGTM

Approved with PullApprove

1 similar comment
@mrunalp
Copy link
Contributor

mrunalp commented Mar 26, 2020

LGTM

Approved with PullApprove

@mrunalp mrunalp merged commit cebef0e into opencontainers:master Mar 26, 2020
@kolyshkin kolyshkin deleted the cgroupv2-max branch March 30, 2020 15:59
@kolyshkin
Copy link
Contributor Author

kolyshkin commented Apr 8, 2020

This patch is wrong: any negative value other than -1 should be treated as an error. Will fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants