From 5a98426296e4234cb5dd8b15ce6c597811bb2332 Mon Sep 17 00:00:00 2001 From: Odin Ugedal Date: Sun, 23 Jun 2019 20:30:34 +0200 Subject: [PATCH 1/2] Fix cgroup hugetlb size prefix for kB The hugetlb cgroup control files (introduced here in 2012: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=abb8206cb0773) use "KB" and not "kB" (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/hugetlb_cgroup.c\?h\=v5.0\#n349\). The behavior in the kernel has not changed since the introduction, and the current code using "kB" will therefore fail on devices with small amounts of ram (see kubernetes/kubernetes#77169) running a kernel with config flag CONFIG_HUGETLBFS=y As seen from the code in "mem_fmt" inside hugetlb_cgroup.c, only "KB", "MB" and "GB" are used, so the others may be removed as well. Here is a real world example of the files inside the "/sys/kernel/mm/hugepages/" directory: - "hugepages-64kB" - "hugepages-2048kB" - "hugepages-32768kB" - "hugepages-1048576kB" And the corresponding cgroup files: - "hugetlb.64KB._____" - "hugetlb.2MB._____" - "hugetlb.32MB._____" - "hugetlb.1GB._____" Signed-off-by: Odin Ugedal --- cgroups/cgroups_v1.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/cgroups/cgroups_v1.go b/cgroups/cgroups_v1.go index 63a878e1..87d3dfb2 100644 --- a/cgroups/cgroups_v1.go +++ b/cgroups/cgroups_v1.go @@ -18,6 +18,11 @@ type CgroupV1 struct { MountPath string } +// HugePageSizeUnitList is a list of the units used by the linux kernel when +// naming the HugePage control files. +// https://www.kernel.org/doc/Documentation/cgroup-v1/hugetlb.txt +var HugePageSizeUnitList = []string{"B", "KB", "MB", "GB", "TB", "PB"} + func getDeviceID(id string) (int64, int64, error) { elem := strings.Split(id, ":") major, err := strconv.ParseInt(elem[0], 10, 64) @@ -393,9 +398,11 @@ func inBytes(size string) (int64, error) { return int64(byteSize), nil } -func getHugePageSize() ([]string, error) { +// GetHugePageSize returns a list of the supported Hugepage sizes on the same format as +// opencontainers/runtime-spec (see pageSize): https://github.com/opencontainers/runtime-spec/blob/master/config-linux.md#huge-page-limits +// eg. ["64KB", "2MB", "1GB"] +func GetHugePageSize() ([]string, error) { var pageSizes []string - sizeList := []string{"B", "kB", "MB", "GB", "TB", "PB"} files, err := ioutil.ReadDir("/sys/kernel/mm/hugepages") if err != nil { return pageSizes, err @@ -409,12 +416,12 @@ func getHugePageSize() ([]string, error) { size := float64(pageSize) base := float64(1024.0) i := 0 - unitsLimit := len(sizeList) - 1 + unitsLimit := len(HugePageSizeUnitList) - 1 for size >= base && i < unitsLimit { size = size / base i++ } - sizeString := fmt.Sprintf("%g%s", size, sizeList[i]) + sizeString := fmt.Sprintf("%g%s", size, HugePageSizeUnitList[i]) pageSizes = append(pageSizes, sizeString) } @@ -433,7 +440,7 @@ func (cg *CgroupV1) GetHugepageLimitData(pid int, cgPath string) ([]rspec.LinuxH } } lh := []rspec.LinuxHugepageLimit{} - pageSizes, err := getHugePageSize() + pageSizes, err := GetHugePageSize() if err != nil { return lh, err } From be9f6f1dd1c2de7ce0059226513fa73a4ea98ee0 Mon Sep 17 00:00:00 2001 From: Odin Ugedal Date: Sun, 23 Jun 2019 20:31:36 +0200 Subject: [PATCH 2/2] Update hugetlb tests to be more portable Not all arches/setups supports all page sizes. Should only use the ones supported on the current platform Signed-off-by: Odin Ugedal --- .../linux_cgroups_hugetlb.go | 33 +++++---- .../linux_cgroups_relative_hugetlb.go | 70 +++++++++++-------- 2 files changed, 56 insertions(+), 47 deletions(-) diff --git a/validation/linux_cgroups_hugetlb/linux_cgroups_hugetlb.go b/validation/linux_cgroups_hugetlb/linux_cgroups_hugetlb.go index 4927ab92..d71dfb9e 100644 --- a/validation/linux_cgroups_hugetlb/linux_cgroups_hugetlb.go +++ b/validation/linux_cgroups_hugetlb/linux_cgroups_hugetlb.go @@ -15,26 +15,24 @@ func testHugetlbCgroups() error { t.Header(0) defer t.AutoPlan() - // limit =~ 100 * page size - // NOTE: on some systems, pagesize "1GB" doesn't seem to work. - // Ideally we should auto-detect the value. - cases := []struct { - page string - limit uint64 - }{ - {"2MB", 100 * 2 * 1024 * 1024}, - {"1GB", 100 * 1024 * 1024 * 1024}, - {"2MB", 100 * 2 * 1024 * 1024}, - {"1GB", 100 * 1024 * 1024 * 1024}, + pageSizes, err := cgroups.GetHugePageSize() + + if err != nil { + t.Fail(fmt.Sprintf("error when getting hugepage sizes: %+v", err)) } - for _, c := range cases { + // When setting the limit just for checking if writing works, the amount of memory + // requested does not matter, as all insigned integers will be accepted. + // Use 2GiB as an example + const limit = 2 * (1 << 30) + + for _, pageSize := range pageSizes { g, err := util.GetDefaultGenerator() if err != nil { return err } g.SetLinuxCgroupsPath(cgroups.AbsCgroupPath) - g.AddLinuxResourcesHugepageLimit(c.page, c.limit) + g.AddLinuxResourcesHugepageLimit(pageSize, limit) err = util.RuntimeOutsideValidate(g, t, func(config *rspec.Spec, t *tap.T, state *rspec.State) error { cg, err := cgroups.FindCgroup() if err != nil { @@ -45,11 +43,11 @@ func testHugetlbCgroups() error { return err } for _, lhl := range lhd { - if lhl.Pagesize != c.page { + if lhl.Pagesize != pageSize { continue } - t.Ok(lhl.Limit == c.limit, "hugepage limit is set correctly") - t.Diagnosticf("expect: %d, actual: %d", c.limit, lhl.Limit) + t.Ok(lhl.Limit == limit, fmt.Sprintf("hugepage limit is set correctly for size: %s", pageSize)) + t.Diagnosticf("expect: %d, actual: %d", limit, lhl.Limit) } return nil }) @@ -63,7 +61,8 @@ func testHugetlbCgroups() error { func testWrongHugetlb() error { // We deliberately set the page size to a wrong value, "3MB", to see - // if the container really returns an error. + // if the container really returns an error. Page sizes will always be a + // on the format 2^(integer) page := "3MB" var limit uint64 = 100 * 3 * 1024 * 1024 diff --git a/validation/linux_cgroups_relative_hugetlb/linux_cgroups_relative_hugetlb.go b/validation/linux_cgroups_relative_hugetlb/linux_cgroups_relative_hugetlb.go index 54ce8d80..b6d7ae81 100644 --- a/validation/linux_cgroups_relative_hugetlb/linux_cgroups_relative_hugetlb.go +++ b/validation/linux_cgroups_relative_hugetlb/linux_cgroups_relative_hugetlb.go @@ -1,6 +1,7 @@ package main import ( + "fmt" "github.com/mndrix/tap-go" rspec "github.com/opencontainers/runtime-spec/specs-go" "github.com/opencontainers/runtime-tools/cgroups" @@ -8,48 +9,57 @@ import ( ) func main() { - page := "1GB" - var limit uint64 = 56892210544640 - t := tap.New() t.Header(0) defer t.AutoPlan() - g, err := util.GetDefaultGenerator() + pageSizes, err := cgroups.GetHugePageSize() + if err != nil { - util.Fatal(err) + t.Fail(fmt.Sprintf("error when getting hugepage sizes: %+v", err)) } - g.SetLinuxCgroupsPath(cgroups.RelCgroupPath) - g.AddLinuxResourcesHugepageLimit(page, limit) - err = util.RuntimeOutsideValidate(g, t, func(config *rspec.Spec, t *tap.T, state *rspec.State) error { - cg, err := cgroups.FindCgroup() - t.Ok((err == nil), "find hugetlb cgroup") - if err != nil { - t.Diagnostic(err.Error()) - return nil - } + // When setting the limit just for checking if writing works, the amount of memory + // requested does not matter, as all insigned integers will be accepted. + // Use 2GiB as an example + const limit = 2 * (1 << 30) - lhd, err := cg.GetHugepageLimitData(state.Pid, config.Linux.CgroupsPath) - t.Ok((err == nil), "get hugetlb cgroup data") + for _, pageSize := range pageSizes { + g, err := util.GetDefaultGenerator() if err != nil { - t.Diagnostic(err.Error()) - return nil + util.Fatal(err) } + g.SetLinuxCgroupsPath(cgroups.RelCgroupPath) + g.AddLinuxResourcesHugepageLimit(pageSize, limit) + err = util.RuntimeOutsideValidate(g, t, func(config *rspec.Spec, t *tap.T, state *rspec.State) error { + cg, err := cgroups.FindCgroup() + t.Ok((err == nil), "find hugetlb cgroup") + if err != nil { + t.Diagnostic(err.Error()) + return nil + } - found := false - for _, lhl := range lhd { - if lhl.Pagesize == page { - found = true - t.Ok(lhl.Limit == limit, "hugepage limit is set correctly") - t.Diagnosticf("expect: %d, actual: %d", limit, lhl.Limit) + lhd, err := cg.GetHugepageLimitData(state.Pid, config.Linux.CgroupsPath) + t.Ok((err == nil), "get hugetlb cgroup data") + if err != nil { + t.Diagnostic(err.Error()) + return nil } - } - t.Ok(found, "hugepage limit found") - return nil - }) + found := false + for _, lhl := range lhd { + if lhl.Pagesize == pageSize { + found = true + t.Ok(lhl.Limit == limit, fmt.Sprintf("hugepage limit is set correctly for size: %s", pageSize)) + t.Diagnosticf("expect: %d, actual: %d", limit, lhl.Limit) + } + } + t.Ok(found, "hugepage limit found") - if err != nil { - t.Fail(err.Error()) + return nil + }) + + if err != nil { + t.Fail(err.Error()) + } } }