From 0ac92aab3f619d38d9162a904aec2cd704f47829 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 28 May 2020 07:49:44 -0700 Subject: [PATCH 1/2] cgroups/fs2: make removeCgroupPath faster 1. In cases there are no sub-cgroups, a single rmdir should be faster than iterating through the list of files. 2. Use unix.Rmdir() to save one more syscall since os.Remove() tries unlink(2) first which fails on a directory, and only then tries rmdir(2). 3. Re-use rmdir. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs2/fs2.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/libcontainer/cgroups/fs2/fs2.go b/libcontainer/cgroups/fs2/fs2.go index 55668470377..69b9948a4da 100644 --- a/libcontainer/cgroups/fs2/fs2.go +++ b/libcontainer/cgroups/fs2/fs2.go @@ -11,6 +11,7 @@ import ( "github.com/opencontainers/runc/libcontainer/cgroups" "github.com/opencontainers/runc/libcontainer/configs" "github.com/pkg/errors" + "golang.org/x/sys/unix" ) type manager struct { @@ -156,9 +157,22 @@ func (m *manager) Freeze(state configs.FreezerState) error { return nil } +func rmdir(path string) error { + err := unix.Rmdir(path) + if err == nil || err == unix.ENOENT { + return nil + } + return &os.PathError{Op: "rmdir", Path: path, Err: err} +} + // removeCgroupPath aims to remove cgroup path recursively // Because there may be subcgroups in it. func removeCgroupPath(path string) error { + // try the fast path first + if err := rmdir(path); err == nil { + return nil + } + infos, err := ioutil.ReadDir(path) if err != nil { if os.IsNotExist(err) { @@ -175,10 +189,7 @@ func removeCgroupPath(path string) error { } } if err == nil { - err = os.Remove(path) - if os.IsNotExist(err) { - err = nil - } + err = rmdir(path) } return err } From a78e21b5001dbe0d2cc6de630fc3e0d62b023ca5 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 28 May 2020 11:45:14 -0700 Subject: [PATCH 2/2] tests/int/delete.bats: fixups 1. __runc does not set $status, so the check is misleading. 2. Add set +eux to the nest.sh script so we can error out early, and see what is going on. 3. Doing "echo +io" > cgroup.controllers is giving an error on my machine ("sh: write error: Operation not supported"). It is probably fine to just enable pids controller. 4. Add status check for runc exec nest.sh 5. Remove the second check for cgroup.threads contents -- it was already checked earlier (the output of nest.sh script). Signed-off-by: Kir Kolyshkin --- tests/integration/delete.bats | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats index 44a76fc9635..9c80909dc1e 100644 --- a/tests/integration/delete.bats +++ b/tests/integration/delete.bats @@ -71,7 +71,6 @@ function teardown() { # create a sub process __runc exec -d test_busybox sleep 1d - [ "$status" -eq 0 ] # find the pid of sleep pid=$(__runc exec test_busybox ps -a | grep 1d | awk '{print $1}') @@ -79,8 +78,9 @@ function teardown() { # create subcgroups cat < nest.sh + set -e -u -x cd /sys/fs/cgroup - for f in \$(cat cgroup.controllers); do echo +\$f > cgroup.subtree_control; done + echo +pids > cgroup.subtree_control mkdir foo cd foo echo threaded > cgroup.type @@ -88,15 +88,12 @@ function teardown() { cat cgroup.threads EOF cat nest.sh | runc exec test_busybox sh - [[ ${output} =~ [0-9]+ ]] + [ "$status" -eq 0 ] + [[ "$output" =~ [0-9]+ ]] # check create subcgroups success [ -d $CGROUP_PATH/foo ] - # check cgroup.threads' value - runc exec test_busybox cat /sys/fs/cgroup/foo/cgroup.threads - [[ ${output} =~ [0-9]+ ]] - # force delete test_busybox runc delete --force test_busybox