diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 58cc4e19c..bc98375ea 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -15,7 +15,6 @@ import ( "sync" "time" - units "github.com/docker/go-units" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -30,13 +29,6 @@ var ( isUnified bool ) -// 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 -// TODO Since the kernel only use KB, MB and GB; TB and PB should be removed, -// depends on https://github.com/docker/go-units/commit/a09cd47f892041a4fac473133d181f5aea6fa393 -var HugePageSizeUnitList = []string{"B", "KB", "MB", "GB", "TB", "PB"} - // IsCgroup2UnifiedMode returns whether we are running in cgroup v2 unified mode. func IsCgroup2UnifiedMode() bool { isUnifiedOnce.Do(func() { @@ -285,27 +277,50 @@ func RemovePaths(paths map[string]string) (err error) { } func GetHugePageSize() ([]string, error) { - files, err := ioutil.ReadDir("/sys/kernel/mm/hugepages") + dir, err := os.OpenFile("/sys/kernel/mm/hugepages", unix.O_DIRECTORY|unix.O_RDONLY, 0) if err != nil { - return []string{}, err + return nil, err } - var fileNames []string - for _, st := range files { - fileNames = append(fileNames, st.Name()) + files, err := dir.Readdirnames(0) + dir.Close() + if err != nil { + return nil, err } - return getHugePageSizeFromFilenames(fileNames) + + return getHugePageSizeFromFilenames(files) } func getHugePageSizeFromFilenames(fileNames []string) ([]string, error) { - var pageSizes []string - for _, fileName := range fileNames { - nameArray := strings.Split(fileName, "-") - pageSize, err := units.RAMInBytes(nameArray[1]) - if err != nil { - return []string{}, err + pageSizes := make([]string, 0, len(fileNames)) + + for _, file := range fileNames { + // example: hugepages-1048576kB + val := strings.TrimPrefix(file, "hugepages-") + if len(val) == len(file) { + // unexpected file name: no prefix found + continue } - sizeString := units.CustomSize("%g%s", float64(pageSize), 1024.0, HugePageSizeUnitList) - pageSizes = append(pageSizes, sizeString) + // The suffix is always "kB" (as of Linux 5.9) + eLen := len(val) - 2 + val = strings.TrimSuffix(val, "kB") + if len(val) != eLen { + logrus.Warnf("GetHugePageSize: %s: invalid filename suffix (expected \"kB\")", file) + continue + } + size, err := strconv.Atoi(val) + if err != nil { + return nil, err + } + // Model after https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/hugetlb_cgroup.c?id=eff48ddeab782e35e58ccc8853f7386bbae9dec4#n574 + // but in our case the size is in KB already. + if size >= (1 << 20) { + val = strconv.Itoa(size>>20) + "GB" + } else if size >= (1 << 10) { + val = strconv.Itoa(size>>10) + "MB" + } else { + val += "KB" + } + pageSizes = append(pageSizes, val) } return pageSizes, nil diff --git a/libcontainer/cgroups/utils_test.go b/libcontainer/cgroups/utils_test.go index ebc38f253..4a5d95eb6 100644 --- a/libcontainer/cgroups/utils_test.go +++ b/libcontainer/cgroups/utils_test.go @@ -4,11 +4,12 @@ package cgroups import ( "bytes" - "errors" "fmt" "reflect" "strings" "testing" + + "github.com/sirupsen/logrus" ) const fedoraMountinfo = `15 35 0:3 / /proc rw,nosuid,nodev,noexec,relatime shared:5 - proc proc rw @@ -401,37 +402,100 @@ func TestFindCgroupMountpointAndRoot(t *testing.T) { } } -func TestGetHugePageSizeImpl(t *testing.T) { +func BenchmarkGetHugePageSize(b *testing.B) { + var ( + output []string + err error + ) + for i := 0; i < b.N; i++ { + output, err = GetHugePageSize() + } + if err != nil || len(output) == 0 { + b.Fatal("unexpected results") + } +} +func BenchmarkGetHugePageSizeImpl(b *testing.B) { + var ( + input = []string{"hugepages-1048576kB", "hugepages-2048kB", "hugepages-32768kB", "hugepages-64kB"} + output []string + err error + ) + for i := 0; i < b.N; i++ { + output, err = getHugePageSizeFromFilenames(input) + } + if err != nil || len(output) != len(input) { + b.Fatal("unexpected results") + } +} + +func TestGetHugePageSizeImpl(t *testing.T) { testCases := []struct { - inputFiles []string - outputPageSizes []string - err error + input []string + output []string + isErr bool + isWarn bool }{ { - inputFiles: []string{"hugepages-1048576kB", "hugepages-2048kB", "hugepages-32768kB", "hugepages-64kB"}, - outputPageSizes: []string{"1GB", "2MB", "32MB", "64KB"}, - err: nil, + input: []string{"hugepages-1048576kB", "hugepages-2048kB", "hugepages-32768kB", "hugepages-64kB"}, + output: []string{"1GB", "2MB", "32MB", "64KB"}, }, { - inputFiles: []string{}, - outputPageSizes: []string{}, - err: nil, + input: []string{}, + output: []string{}, }, - { - inputFiles: []string{"hugepages-a"}, - outputPageSizes: []string{}, - err: errors.New("invalid size: 'a'"), + { // not a number + input: []string{"hugepages-akB"}, + isErr: true, + }, + { // no prefix (silently skipped) + input: []string{"1024kB"}, + }, + { // invalid prefix (silently skipped) + input: []string{"whatever-1024kB"}, + }, + { // invalid suffix (skipped with a warning) + input: []string{"hugepages-1024gB"}, + isWarn: true, + }, + { // no suffix (skipped with a warning) + input: []string{"hugepages-1024"}, + isWarn: true, }, } + // Need to catch warnings. + savedOut := logrus.StandardLogger().Out + defer logrus.SetOutput(savedOut) + warns := new(bytes.Buffer) + logrus.SetOutput(warns) + for _, c := range testCases { - pageSizes, err := getHugePageSizeFromFilenames(c.inputFiles) - if len(pageSizes) != 0 && len(c.outputPageSizes) != 0 && !reflect.DeepEqual(pageSizes, c.outputPageSizes) { - t.Errorf("expected %s, got %s", c.outputPageSizes, pageSizes) + warns.Reset() + output, err := getHugePageSizeFromFilenames(c.input) + if err != nil { + t.Logf("(input %v, error %v)", c.input, err) + if !c.isErr { + t.Error("unexpected error ^^^^") + } + // no more checks + continue } - if err != nil && err.Error() != c.err.Error() { - t.Errorf("expected error %s, got %s", c.err, err) + if c.isErr { + t.Errorf("input %v, expected error, got error: nil, output: %v", c.input, output) + // no more checks + continue + } + // check for warnings + if c.isWarn && warns.Len() == 0 { + t.Errorf("input %v, expected a warning, got none", c.input) + } + if !c.isWarn && warns.Len() > 0 { + t.Errorf("input %v, unexpected warning: %s", c.input, warns.String()) + } + // check output + if len(output) != len(c.output) || (len(output) > 0 && !reflect.DeepEqual(output, c.output)) { + t.Errorf("input %v, expected %v, got %v", c.input, c.output, output) } } }