From f8be7009064e7493eef097ae5f5bb3a252624100 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 20 Oct 2023 11:27:42 -0700 Subject: [PATCH 1/2] [1.1] tests/int/helpers: add get_cgroup_path Separate it out of get_cgroup_value. Needed for the next commit. This function was initially introduced in main branch commit d4582ae2f. Signed-off-by: Kir Kolyshkin --- tests/integration/helpers.bash | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash index 23e32e978..12650593d 100644 --- a/tests/integration/helpers.bash +++ b/tests/integration/helpers.bash @@ -226,19 +226,27 @@ function set_cgroups_path() { update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"' } +# Get a path to cgroup directory, based on controller name. +# Parameters: +# $1: controller name (like "pids") or a file name (like "pids.max"). +function get_cgroup_path() { + if [ "$CGROUP_UNIFIED" = "yes" ]; then + echo "$CGROUP_PATH" + return + fi + + local var cgroup + var=${1%%.*} # controller name (e.g. memory) + var=CGROUP_${var^^}_BASE_PATH # variable name (e.g. CGROUP_MEMORY_BASE_PATH) + eval cgroup=\$"${var}${REL_CGROUPS_PATH}" + echo "$cgroup" +} + # Get a value from a cgroup file. function get_cgroup_value() { - local source=$1 - local cgroup var current - - if [ "$CGROUP_UNIFIED" = "yes" ]; then - cgroup=$CGROUP_PATH - else - var=${source%%.*} # controller name (e.g. memory) - var=CGROUP_${var^^}_BASE_PATH # variable name (e.g. CGROUP_MEMORY_BASE_PATH) - eval cgroup=\$"${var}${REL_CGROUPS_PATH}" - fi - cat "$cgroup/$source" + local cgroup + cgroup="$(get_cgroup_path "$1")" + cat "$cgroup/$1" } # Helper to check a if value in a cgroup file matches the expected one. From 8214e6347475c549540102344a4042a048bf55bb Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 21 Mar 2023 13:03:52 -0700 Subject: [PATCH 2/2] libct/cg: support hugetlb rsvd This adds support for hugetlb..rsvd limiting and accounting. The previous non-rsvd max/limit_in_bytes does not account for reserved huge page memory, making it possible for a processes to reserve all the huge page memory, without being able to allocate it (due to cgroup restrictions). In practice this makes it possible to successfully mmap more huge page memory than allowed via the cgroup settings, but when using the memory the process will get a SIGBUS and crash. This is bad for applications trying to mmap at startup (and it succeeds), but the program crashes when starting to use the memory. eg. postgres is doing this by default. This also keeps writing to the old max/limit_in_bytes, for backward compatibility. More info can be found here: https://lkml.org/lkml/2020/2/3/1153 (commit message mostly written by Odin Ugedal) [1.1 backport: check for CGROUP_UNIFIED in integration test] Co-authored-by: Odin Ugedal (cherry picked from commit 4a7d3ae5cd7ebb8b0fc7b0a74ba94cdf739361f2) Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/hugetlb.go | 36 +++++++++++--- libcontainer/cgroups/fs/hugetlb_test.go | 43 ++++++++++++++--- libcontainer/cgroups/fs2/hugetlb.go | 30 ++++++++++-- tests/integration/cgroups.bats | 64 +++++++++++++++++++++++++ 4 files changed, 155 insertions(+), 18 deletions(-) diff --git a/libcontainer/cgroups/fs/hugetlb.go b/libcontainer/cgroups/fs/hugetlb.go index 8ddd6fdd8..50f8f30cd 100644 --- a/libcontainer/cgroups/fs/hugetlb.go +++ b/libcontainer/cgroups/fs/hugetlb.go @@ -1,6 +1,8 @@ package fs import ( + "errors" + "os" "strconv" "github.com/opencontainers/runc/libcontainer/cgroups" @@ -19,8 +21,23 @@ func (s *HugetlbGroup) Apply(path string, _ *configs.Resources, pid int) error { } func (s *HugetlbGroup) Set(path string, r *configs.Resources) error { + const suffix = ".limit_in_bytes" + skipRsvd := false + for _, hugetlb := range r.HugetlbLimit { - if err := cgroups.WriteFile(path, "hugetlb."+hugetlb.Pagesize+".limit_in_bytes", strconv.FormatUint(hugetlb.Limit, 10)); err != nil { + prefix := "hugetlb." + hugetlb.Pagesize + val := strconv.FormatUint(hugetlb.Limit, 10) + if err := cgroups.WriteFile(path, prefix+suffix, val); err != nil { + return err + } + if skipRsvd { + continue + } + if err := cgroups.WriteFile(path, prefix+".rsvd"+suffix, val); err != nil { + if errors.Is(err, os.ErrNotExist) { + skipRsvd = true + continue + } return err } } @@ -32,24 +49,29 @@ func (s *HugetlbGroup) GetStats(path string, stats *cgroups.Stats) error { if !cgroups.PathExists(path) { return nil } + rsvd := ".rsvd" hugetlbStats := cgroups.HugetlbStats{} for _, pageSize := range cgroups.HugePageSizes() { - usage := "hugetlb." + pageSize + ".usage_in_bytes" - value, err := fscommon.GetCgroupParamUint(path, usage) + again: + prefix := "hugetlb." + pageSize + rsvd + + value, err := fscommon.GetCgroupParamUint(path, prefix+".usage_in_bytes") if err != nil { + if rsvd != "" && errors.Is(err, os.ErrNotExist) { + rsvd = "" + goto again + } return err } hugetlbStats.Usage = value - maxUsage := "hugetlb." + pageSize + ".max_usage_in_bytes" - value, err = fscommon.GetCgroupParamUint(path, maxUsage) + value, err = fscommon.GetCgroupParamUint(path, prefix+".max_usage_in_bytes") if err != nil { return err } hugetlbStats.MaxUsage = value - failcnt := "hugetlb." + pageSize + ".failcnt" - value, err = fscommon.GetCgroupParamUint(path, failcnt) + value, err = fscommon.GetCgroupParamUint(path, prefix+".failcnt") if err != nil { return err } diff --git a/libcontainer/cgroups/fs/hugetlb_test.go b/libcontainer/cgroups/fs/hugetlb_test.go index f4aea7eb5..17b4945a1 100644 --- a/libcontainer/cgroups/fs/hugetlb_test.go +++ b/libcontainer/cgroups/fs/hugetlb_test.go @@ -21,6 +21,11 @@ const ( limit = "hugetlb.%s.limit_in_bytes" maxUsage = "hugetlb.%s.max_usage_in_bytes" failcnt = "hugetlb.%s.failcnt" + + rsvdUsage = "hugetlb.%s.rsvd.usage_in_bytes" + rsvdLimit = "hugetlb.%s.rsvd.limit_in_bytes" + rsvdMaxUsage = "hugetlb.%s.rsvd.max_usage_in_bytes" + rsvdFailcnt = "hugetlb.%s.rsvd.failcnt" ) func TestHugetlbSetHugetlb(t *testing.T) { @@ -52,13 +57,15 @@ func TestHugetlbSetHugetlb(t *testing.T) { } for _, pageSize := range cgroups.HugePageSizes() { - limit := fmt.Sprintf(limit, pageSize) - value, err := fscommon.GetCgroupParamUint(path, limit) - if err != nil { - t.Fatal(err) - } - if value != hugetlbAfter { - t.Fatalf("Set hugetlb.limit_in_bytes failed. Expected: %v, Got: %v", hugetlbAfter, value) + for _, f := range []string{limit, rsvdLimit} { + limit := fmt.Sprintf(f, pageSize) + value, err := fscommon.GetCgroupParamUint(path, limit) + if err != nil { + t.Fatal(err) + } + if value != hugetlbAfter { + t.Fatalf("Set %s failed. Expected: %v, Got: %v", limit, hugetlbAfter, value) + } } } } @@ -85,6 +92,28 @@ func TestHugetlbStats(t *testing.T) { } } +func TestHugetlbRStatsRsvd(t *testing.T) { + path := tempDir(t, "hugetlb") + for _, pageSize := range cgroups.HugePageSizes() { + writeFileContents(t, path, map[string]string{ + fmt.Sprintf(rsvdUsage, pageSize): hugetlbUsageContents, + fmt.Sprintf(rsvdMaxUsage, pageSize): hugetlbMaxUsageContents, + fmt.Sprintf(rsvdFailcnt, pageSize): hugetlbFailcnt, + }) + } + + hugetlb := &HugetlbGroup{} + actualStats := *cgroups.NewStats() + err := hugetlb.GetStats(path, &actualStats) + if err != nil { + t.Fatal(err) + } + expectedStats := cgroups.HugetlbStats{Usage: 128, MaxUsage: 256, Failcnt: 100} + for _, pageSize := range cgroups.HugePageSizes() { + expectHugetlbStatEquals(t, expectedStats, actualStats.HugetlbStats[pageSize]) + } +} + func TestHugetlbStatsNoUsageFile(t *testing.T) { path := tempDir(t, "hugetlb") writeFileContents(t, path, map[string]string{ diff --git a/libcontainer/cgroups/fs2/hugetlb.go b/libcontainer/cgroups/fs2/hugetlb.go index c92a7e64a..2ce2631e1 100644 --- a/libcontainer/cgroups/fs2/hugetlb.go +++ b/libcontainer/cgroups/fs2/hugetlb.go @@ -1,6 +1,8 @@ package fs2 import ( + "errors" + "os" "strconv" "github.com/opencontainers/runc/libcontainer/cgroups" @@ -16,8 +18,22 @@ func setHugeTlb(dirPath string, r *configs.Resources) error { if !isHugeTlbSet(r) { return nil } + const suffix = ".max" + skipRsvd := false for _, hugetlb := range r.HugetlbLimit { - if err := cgroups.WriteFile(dirPath, "hugetlb."+hugetlb.Pagesize+".max", strconv.FormatUint(hugetlb.Limit, 10)); err != nil { + prefix := "hugetlb." + hugetlb.Pagesize + val := strconv.FormatUint(hugetlb.Limit, 10) + if err := cgroups.WriteFile(dirPath, prefix+suffix, val); err != nil { + return err + } + if skipRsvd { + continue + } + if err := cgroups.WriteFile(dirPath, prefix+".rsvd"+suffix, val); err != nil { + if errors.Is(err, os.ErrNotExist) { + skipRsvd = true + continue + } return err } } @@ -27,15 +43,21 @@ func setHugeTlb(dirPath string, r *configs.Resources) error { func statHugeTlb(dirPath string, stats *cgroups.Stats) error { hugetlbStats := cgroups.HugetlbStats{} + rsvd := ".rsvd" for _, pagesize := range cgroups.HugePageSizes() { - value, err := fscommon.GetCgroupParamUint(dirPath, "hugetlb."+pagesize+".current") + again: + prefix := "hugetlb." + pagesize + rsvd + value, err := fscommon.GetCgroupParamUint(dirPath, prefix+".current") if err != nil { + if rsvd != "" && errors.Is(err, os.ErrNotExist) { + rsvd = "" + goto again + } return err } hugetlbStats.Usage = value - fileName := "hugetlb." + pagesize + ".events" - value, err = fscommon.GetValueByKey(dirPath, fileName, "max") + value, err = fscommon.GetValueByKey(dirPath, prefix+".events", "max") if err != nil { return err } diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index e6e136e7f..810eaf8a0 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -187,6 +187,70 @@ function setup() { [[ "$weights" == *"$major:$minor 444"* ]] } +# Convert size in KB to hugetlb size suffix. +convert_hugetlb_size() { + local size=$1 + local units=("KB" "MB" "GB") + local idx=0 + + while ((size >= 1024)); do + ((size /= 1024)) + ((idx++)) + done + + echo "$size${units[$idx]}" +} + +@test "runc run (hugetlb limits)" { + requires cgroups_hugetlb + [ $EUID -ne 0 ] && requires rootless_cgroup + # shellcheck disable=SC2012 # ls is fine here. + mapfile -t sizes_kb < <(ls /sys/kernel/mm/hugepages/ | sed -e 's/.*hugepages-//' -e 's/kB$//') # + if [ "${#sizes_kb[@]}" -lt 1 ]; then + skip "requires hugetlb" + fi + + # Create two arrays: + # - sizes: hugetlb cgroup file suffixes; + # - limits: limits for each size. + for size in "${sizes_kb[@]}"; do + sizes+=("$(convert_hugetlb_size "$size")") + # Limit to 1 page. + limits+=("$((size * 1024))") + done + + # Set per-size limits. + for ((i = 0; i < ${#sizes[@]}; i++)); do + size="${sizes[$i]}" + limit="${limits[$i]}" + update_config '.linux.resources.hugepageLimits += [{ pagesize: "'"$size"'", limit: '"$limit"' }]' + done + + set_cgroups_path + runc run -d --console-socket "$CONSOLE_SOCKET" test_hugetlb + [ "$status" -eq 0 ] + + lim="max" + [ "$CGROUP_UNIFIED" = "no" ] && lim="limit_in_bytes" + + optional=("") + # Add rsvd, if available. + if test -f "$(get_cgroup_path hugetlb)/hugetlb.${sizes[0]}.rsvd.$lim"; then + optional+=(".rsvd") + fi + + # Check if the limits are as expected. + for ((i = 0; i < ${#sizes[@]}; i++)); do + size="${sizes[$i]}" + limit="${limits[$i]}" + for rsvd in "${optional[@]}"; do + param="hugetlb.${size}${rsvd}.$lim" + echo "checking $param" + check_cgroup_value "$param" "$limit" + done + done +} + @test "runc run (cgroup v2 resources.unified only)" { requires root cgroups_v2