summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorFabricio Voznika <fvoznika@google.com>2021-10-14 18:35:08 -0700
committergVisor bot <gvisor-bot@google.com>2021-10-14 18:42:07 -0700
commit33b41d8fe98e7820118e8d42b0cfbec4ca159d62 (patch)
tree84b6bbdb35f4044e7f0c8210f4f771cf15547d1a
parent1711fd9efe86948198d836faf060e7dea60bae8d (diff)
Report total memory based on limit or host
gVisor was previously reporting the lower of cgroup limit or 2GB as total memory. This may cause applications to make bad decisions based on amount of memory available to them when more than 2GB is required. This change makes the lower of cgroup limit or the host total memory to be reported inside the sandbox. This also is more inline with docker which always reports host total memory. Note that reporting cgroup limit is strictly better than host total memory when there is a limit set. Fixes #5608 PiperOrigin-RevId: 403241608
-rw-r--r--pkg/sentry/fsimpl/proc/tasks_files.go5
-rw-r--r--pkg/sentry/usage/memory.go6
-rw-r--r--runsc/boot/loader.go1
-rw-r--r--runsc/sandbox/BUILD10
-rw-r--r--runsc/sandbox/memory.go70
-rw-r--r--runsc/sandbox/memory_test.go91
-rw-r--r--runsc/sandbox/sandbox.go14
-rw-r--r--test/e2e/integration_test.go44
8 files changed, 225 insertions, 16 deletions
diff --git a/pkg/sentry/fsimpl/proc/tasks_files.go b/pkg/sentry/fsimpl/proc/tasks_files.go
index 4d3a2f7e6..faec36d8d 100644
--- a/pkg/sentry/fsimpl/proc/tasks_files.go
+++ b/pkg/sentry/fsimpl/proc/tasks_files.go
@@ -262,9 +262,8 @@ var _ dynamicInode = (*meminfoData)(nil)
// Generate implements vfs.DynamicBytesSource.Generate.
func (*meminfoData) Generate(ctx context.Context, buf *bytes.Buffer) error {
- k := kernel.KernelFromContext(ctx)
- mf := k.MemoryFile()
- mf.UpdateUsage()
+ mf := kernel.KernelFromContext(ctx).MemoryFile()
+ _ = mf.UpdateUsage() // Best effort
snapshot, totalUsage := usage.MemoryAccounting.Copy()
totalSize := usage.TotalMemory(mf.TotalSize(), totalUsage)
anon := snapshot.Anonymous + snapshot.Tmpfs
diff --git a/pkg/sentry/usage/memory.go b/pkg/sentry/usage/memory.go
index e7073ec87..d9df890c4 100644
--- a/pkg/sentry/usage/memory.go
+++ b/pkg/sentry/usage/memory.go
@@ -252,9 +252,9 @@ func (m *MemoryLocked) Copy() (MemoryStats, uint64) {
return ms, m.totalLocked()
}
-// These options control how much total memory the is reported to the application.
-// They may only be set before the application starts executing, and must not
-// be modified.
+// These options control how much total memory the is reported to the
+// application. They may only be set before the application starts executing,
+// and must not be modified.
var (
// MinimumTotalMemoryBytes is the minimum reported total system memory.
MinimumTotalMemoryBytes uint64 = 2 << 30 // 2 GB
diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go
index 2f2d4df5e..28dbbcf1f 100644
--- a/runsc/boot/loader.go
+++ b/runsc/boot/loader.go
@@ -340,6 +340,7 @@ func New(args Args) (*Loader, error) {
if args.TotalMem > 0 {
// Adjust the total memory returned by the Sentry so that applications that
// use /proc/meminfo can make allocations based on this limit.
+ usage.MinimumTotalMemoryBytes = args.TotalMem
usage.MaximumTotalMemoryBytes = args.TotalMem
log.Infof("Setting total memory to %.2f GB", float64(args.TotalMem)/(1<<30))
}
diff --git a/runsc/sandbox/BUILD b/runsc/sandbox/BUILD
index d625230dd..bca14c7b8 100644
--- a/runsc/sandbox/BUILD
+++ b/runsc/sandbox/BUILD
@@ -1,10 +1,11 @@
-load("//tools:defs.bzl", "go_library")
+load("//tools:defs.bzl", "go_library", "go_test")
package(licenses = ["notice"])
go_library(
name = "sandbox",
srcs = [
+ "memory.go",
"network.go",
"network_unsafe.go",
"sandbox.go",
@@ -39,3 +40,10 @@ go_library(
"@org_golang_x_sys//unix:go_default_library",
],
)
+
+go_test(
+ name = "sandbox_test",
+ size = "small",
+ srcs = ["memory_test.go"],
+ library = ":sandbox",
+)
diff --git a/runsc/sandbox/memory.go b/runsc/sandbox/memory.go
new file mode 100644
index 000000000..77b88eb89
--- /dev/null
+++ b/runsc/sandbox/memory.go
@@ -0,0 +1,70 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package sandbox
+
+import (
+ "bufio"
+ "fmt"
+ "io"
+ "os"
+ "strconv"
+ "strings"
+)
+
+// totalSystemMemory extracts "MemTotal" from "/proc/meminfo".
+func totalSystemMemory() (uint64, error) {
+ f, err := os.Open("/proc/meminfo")
+ if err != nil {
+ return 0, err
+ }
+ defer f.Close()
+ return parseTotalSystemMemory(f)
+}
+
+func parseTotalSystemMemory(r io.Reader) (uint64, error) {
+ for scanner := bufio.NewScanner(r); scanner.Scan(); {
+ line := scanner.Text()
+ totalStr := strings.TrimPrefix(line, "MemTotal:")
+ if len(totalStr) < len(line) {
+ fields := strings.Fields(totalStr)
+ if len(fields) == 0 || len(fields) > 2 {
+ return 0, fmt.Errorf(`malformed "MemTotal": %q`, line)
+ }
+ totalStr = fields[0]
+ unit := ""
+ if len(fields) == 2 {
+ unit = fields[1]
+ }
+ mem, err := strconv.ParseUint(totalStr, 10, 64)
+ if err != nil {
+ return 0, err
+ }
+ switch unit {
+ case "":
+ // do nothing.
+ case "kB":
+ memKb := mem
+ mem = memKb * 1024
+ if mem < memKb {
+ return 0, fmt.Errorf(`"MemTotal" too large: %d`, memKb)
+ }
+ default:
+ return 0, fmt.Errorf("unknown unit %q: %q", unit, line)
+ }
+ return mem, nil
+ }
+ }
+ return 0, fmt.Errorf(`malformed "/proc/meminfo": "MemTotal" not found`)
+}
diff --git a/runsc/sandbox/memory_test.go b/runsc/sandbox/memory_test.go
new file mode 100644
index 000000000..81dc67881
--- /dev/null
+++ b/runsc/sandbox/memory_test.go
@@ -0,0 +1,91 @@
+// Copyright 2021 The gVisor Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package sandbox
+
+import (
+ "bytes"
+ "fmt"
+ "math"
+ "strings"
+ "testing"
+)
+
+func TestTotalSystemMemory(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ content string
+ want uint64
+ err string
+ }{
+ {
+ name: "simple",
+ content: "MemTotal: 123\n",
+ want: 123,
+ },
+ {
+ name: "kb",
+ content: "MemTotal: 123 kB\n",
+ want: 123 * 1024,
+ },
+ {
+ name: "multi-line",
+ content: "Something: 123\nMemTotal: 456\nAnotherThing: 789\n",
+ want: 456,
+ },
+ {
+ name: "not-found",
+ content: "Something: 123 kB\nAnotherThing: 789 kB\n",
+ err: "not found",
+ },
+ {
+ name: "no-number",
+ content: "MemTotal: \n",
+ err: "malformed",
+ },
+ {
+ name: "only-unit",
+ content: "MemTotal: kB\n",
+ err: "invalid syntax",
+ },
+ {
+ name: "negative",
+ content: "MemTotal: -1\n",
+ err: "invalid syntax",
+ },
+ {
+ name: "overflow",
+ content: fmt.Sprintf("MemTotal: %d kB\n", uint64(math.MaxUint64)),
+ err: "too large",
+ },
+ {
+ name: "unkown-unit",
+ content: "MemTotal: 123 mB\n",
+ err: "unknown unit",
+ },
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ mem, err := parseTotalSystemMemory(bytes.NewReader([]byte(tc.content)))
+ if len(tc.err) > 0 {
+ if err == nil || !strings.Contains(err.Error(), tc.err) {
+ t.Errorf("parseTotalSystemMemory(%q) invalid error: %v, want: %v", tc.content, err, tc.err)
+ }
+ } else {
+ if tc.want != mem {
+ t.Errorf("parseTotalSystemMemory(%q) got: %v, want: %v", tc.content, mem, tc.want)
+ }
+ }
+ })
+ }
+}
diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go
index f4a37cedc..714919139 100644
--- a/runsc/sandbox/sandbox.go
+++ b/runsc/sandbox/sandbox.go
@@ -758,6 +758,11 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn
// shown as `exe`.
cmd.Args[0] = "runsc-sandbox"
+ mem, err := totalSystemMemory()
+ if err != nil {
+ return err
+ }
+
if s.Cgroup != nil {
cpuNum, err := s.Cgroup.NumCPU()
if err != nil {
@@ -785,16 +790,15 @@ func (s *Sandbox) createSandboxProcess(conf *config.Config, args *Args, startSyn
}
cmd.Args = append(cmd.Args, "--cpu-num", strconv.Itoa(cpuNum))
- mem, err := s.Cgroup.MemoryLimit()
+ memLimit, err := s.Cgroup.MemoryLimit()
if err != nil {
return fmt.Errorf("getting memory limit from cgroups: %v", err)
}
- // When memory limit is unset, a "large" number is returned. In that case,
- // just stick with the default.
- if mem < 0x7ffffffffffff000 {
- cmd.Args = append(cmd.Args, "--total-memory", strconv.FormatUint(mem, 10))
+ if memLimit < mem {
+ mem = memLimit
}
}
+ cmd.Args = append(cmd.Args, "--total-memory", strconv.FormatUint(mem, 10))
if args.UserLog != "" {
f, err := os.OpenFile(args.UserLog, os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0664)
diff --git a/test/e2e/integration_test.go b/test/e2e/integration_test.go
index d41139944..c5aa35623 100644
--- a/test/e2e/integration_test.go
+++ b/test/e2e/integration_test.go
@@ -29,6 +29,7 @@ import (
"net"
"net/http"
"os"
+ "os/exec"
"path/filepath"
"regexp"
"strconv"
@@ -41,8 +42,12 @@ import (
"gvisor.dev/gvisor/pkg/test/testutil"
)
-// defaultWait is the default wait time used for tests.
-const defaultWait = time.Minute
+const (
+ // defaultWait is the default wait time used for tests.
+ defaultWait = time.Minute
+
+ memInfoCmd = "cat /proc/meminfo | grep MemTotal: | awk '{print $2}'"
+)
func TestMain(m *testing.M) {
dockerutil.EnsureSupportedDockerVersion()
@@ -255,16 +260,47 @@ func TestConnectToSelf(t *testing.T) {
}
}
+func TestMemory(t *testing.T) {
+ // Find total amount of memory in the host.
+ host, err := exec.Command("sh", "-c", memInfoCmd).CombinedOutput()
+ if err != nil {
+ t.Fatal(err)
+ }
+ want, err := strconv.ParseUint(strings.TrimSpace(string(host)), 10, 64)
+ if err != nil {
+ t.Fatalf("failed to parse %q: %v", host, err)
+ }
+
+ ctx := context.Background()
+ d := dockerutil.MakeContainer(ctx, t)
+ defer d.CleanUp(ctx)
+
+ out, err := d.Run(ctx, dockerutil.RunOpts{Image: "basic/alpine"}, "sh", "-c", memInfoCmd)
+ if err != nil {
+ t.Fatalf("docker run failed: %v", err)
+ }
+
+ // Get memory from inside the container and ensure it matches the host.
+ got, err := strconv.ParseUint(strings.TrimSpace(out), 10, 64)
+ if err != nil {
+ t.Fatalf("failed to parse %q: %v", out, err)
+ }
+ if got != want {
+ t.Errorf("MemTotal got: %d, want: %d", got, want)
+ }
+}
+
func TestMemLimit(t *testing.T) {
ctx := context.Background()
d := dockerutil.MakeContainer(ctx, t)
defer d.CleanUp(ctx)
allocMemoryKb := 50 * 1024
- out, err := d.Run(ctx, dockerutil.RunOpts{
+ opts := dockerutil.RunOpts{
Image: "basic/alpine",
Memory: allocMemoryKb * 1024, // In bytes.
- }, "sh", "-c", "cat /proc/meminfo | grep MemTotal: | awk '{print $2}'")
+ }
+ out, err := d.Run(ctx, opts, "sh", "-c", memInfoCmd)
if err != nil {
t.Fatalf("docker run failed: %v", err)
}