libct/cgroups: rewrite getHugePageSizeFromFilenames

This is a function to convert huge page sizes (obtained by reading
/sys/kernel/mm/hugepages directory entries) to strings user for hugetlb
cgroup controller resource files. Those strings are when used to get the
hugetlb resource statistics.

This function used external library, floating point numbers, and can
(theoretically) produce invalid values, since the kernel only uses KB,
MB, and GB suffixes.

Rewrite it to produce the same strings as used in the kernel (see [1]).
As a result, it's also faster, more future-proof (entries that do not
start with "hugepages-" and/or incorrect suffix are skipped), and does
more input sanity checks. As a side effect, libcontainer no longer
depends on docker/go-units.

While at it, add more test cases.

Before:
	BenchmarkGetHugePageSize-8       	  187452	      6265 ns/op
	BenchmarkGetHugePageSizeImpl-8   	  396769	      2998 ns/op

After:
	BenchmarkGetHugePageSize-8       	  222898	      4554 ns/op
	BenchmarkGetHugePageSizeImpl-8   	 4738924	       241 ns/op

NOTE on removing HugePageSizeUnitList -- this was added by commit
6f77e35da and was used by kubernetes code in [2], which was later
superceded by [3], so there are (hopefully) no external users.
If there are any, they should not be doing that.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/hugetlb_cgroup.c?id=eff48ddeab782e35e58ccc8853f7386bbae9dec4#n574
[2] https://github.com/kubernetes/kubernetes/pull/78495
[3] https://github.com/kubernetes/kubernetes/pull/84154

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This commit is contained in:
Kir Kolyshkin
2020-09-22 15:15:02 -07:00
parent 8ceae9f766
commit 360981ae1d
2 changed files with 86 additions and 36 deletions
+29 -16
View File
@@ -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() {
@@ -299,15 +291,36 @@ func GetHugePageSize() ([]string, error) {
}
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
+57 -20
View File
@@ -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
@@ -429,36 +430,72 @@ func BenchmarkGetHugePageSizeImpl(b *testing.B) {
}
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)
}
}
}