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

runc incompatibility: cannot set swap limit without the memory limit #1001

Closed
AkihiroSuda opened this issue Sep 3, 2022 · 6 comments · Fixed by #1241
Closed

runc incompatibility: cannot set swap limit without the memory limit #1001

AkihiroSuda opened this issue Sep 3, 2022 · 6 comments · Fixed by #1241

Comments

@AkihiroSuda
Copy link

runc (v1.1.4) accepts the following .linux.resources configuration, but crun (v1.5) rejects with cannot set swap limit without the memory limit

    "resources": {
      "devices": [
        {
          "allow": false,
          "access": "rwm"
        }
      ],
      "memory": {
        "swap": 0
      }
    }
@kolyshkin
Copy link
Collaborator

runc does reject such configuration, too, but only for cgroup v2.

@AkihiroSuda
Copy link
Author

runc does reject such configuration, too, but only for cgroup v2.

Doesn't seem rejected (runc v1.1.4, Ubuntu 22.04, kernel 5.15, cgroup v2 unified)

giuseppe added a commit to giuseppe/crun that referenced this issue Sep 5, 2022
runc compatibility: now -v prints the version in the same mode as
--version.

Closes: containers#1001

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
Copy link
Member

giuseppe commented Sep 5, 2022

how should it be handled? Reading the memory limit?

@AkihiroSuda
Copy link
Author

flouthoc added a commit to flouthoc/crun that referenced this issue Jul 5, 2023
Runc accepts `0` value of `swap` from config as an exception case
* See: https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/fs2/memory.go#L36
* See: https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/utils.go#L431

As per the documentation here: https://facebookmicrosites.github.io/cgroup2/docs/memory-controller.html#using-swap
Setting `memory.swap.max` to `0` is treated as `swap` being disabled
hence allow `crun` to do the same.

Closes: containers#1001

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
Copy link
Collaborator

flouthoc commented Jul 5, 2023

While checking the runc source code I see it adds expection while validating when swap is set to 0 here

Once I remove these check runc fails same as crun, however while reading about the values https://facebookmicrosites.github.io/cgroup2/docs/memory-controller.html#using-swap I think setting memory.swap.max to 0 is still allowed as a way to disable the swap as compared to default value which is max.

flouthoc added a commit to flouthoc/crun that referenced this issue Jul 5, 2023
Runc accepts `0` value of `swap` from config as an exception case
* See: https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/fs2/memory.go#L36
* See: https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/utils.go#L431

As per the documentation here: https://facebookmicrosites.github.io/cgroup2/docs/memory-controller.html#using-swap
Setting `memory.swap.max` to `0` is treated as `swap` being disabled
hence allow `crun` to do the same.

Closes: containers#1001

Signed-off-by: Aditya R <arajan@redhat.com>
flouthoc added a commit to flouthoc/crun that referenced this issue Jul 5, 2023
Runc accepts `0` value of `swap` from config as an exception case
* See: https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/fs2/memory.go#L36
* See: https://github.com/opencontainers/runc/blob/main/libcontainer/cgroups/utils.go#L431

As per the documentation here: https://facebookmicrosites.github.io/cgroup2/docs/memory-controller.html#using-swap
Setting `memory.swap.max` to `0` is treated as `swap` being disabled
hence allow `crun` to do the same.

Closes: containers#1001

Signed-off-by: Aditya R <arajan@redhat.com>
@flouthoc
Copy link
Collaborator

flouthoc commented Jul 5, 2023

Okay I think if runc accepts 0 as a valid value of memory.swap.max from config then it never applies it, so there is a bug in runc as well afaics ( if 0 is a valid value for memory.swap.max as per [1]). I found where is it going wrong, but I want others to take a look at [1] before creating any changes there.

1: https://facebookmicrosites.github.io/cgroup2/docs/memory-controller.html#using-swap

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 a pull request may close this issue.

4 participants