From 7a7fcf2dbaa7bdcdb9b523358de91c71d5cb05d8 Mon Sep 17 00:00:00 2001 From: Rahat Mahmood Date: Mon, 5 Apr 2021 19:44:12 -0700 Subject: Report task CPU usage through the cpuacct cgroup controller. PiperOrigin-RevId: 366923274 --- pkg/sentry/fsimpl/cgroupfs/base.go | 8 ++-- pkg/sentry/fsimpl/cgroupfs/cgroupfs.go | 2 +- pkg/sentry/fsimpl/cgroupfs/cpuacct.go | 79 +++++++++++++++++++++++++++++++++- test/syscalls/linux/cgroup.cc | 51 ++++++++++++++++++++++ 4 files changed, 133 insertions(+), 7 deletions(-) diff --git a/pkg/sentry/fsimpl/cgroupfs/base.go b/pkg/sentry/fsimpl/cgroupfs/base.go index 360bbb17d..39c1013e1 100644 --- a/pkg/sentry/fsimpl/cgroupfs/base.go +++ b/pkg/sentry/fsimpl/cgroupfs/base.go @@ -167,8 +167,8 @@ func (d *cgroupProcsData) Generate(ctx context.Context, buf *bytes.Buffer) error pgids := make(map[kernel.ThreadID]struct{}) - d.fs.tasksMu.Lock() - defer d.fs.tasksMu.Unlock() + d.fs.tasksMu.RLock() + defer d.fs.tasksMu.RUnlock() for task := range d.ts { // Map dedups pgid, since iterating over all tasks produces multiple @@ -209,8 +209,8 @@ func (d *tasksData) Generate(ctx context.Context, buf *bytes.Buffer) error { var pids []kernel.ThreadID - d.fs.tasksMu.Lock() - defer d.fs.tasksMu.Unlock() + d.fs.tasksMu.RLock() + defer d.fs.tasksMu.RUnlock() for task := range d.ts { if pid := currPidns.IDOfTask(task); pid != 0 { diff --git a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go index 6061bace2..ca8caee5f 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go +++ b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go @@ -129,7 +129,7 @@ type filesystem struct { // tasksMu serializes task membership changes across all cgroups within a // filesystem. - tasksMu sync.Mutex `state:"nosave"` + tasksMu sync.RWMutex `state:"nosave"` } // Name implements vfs.FilesystemType.Name. diff --git a/pkg/sentry/fsimpl/cgroupfs/cpuacct.go b/pkg/sentry/fsimpl/cgroupfs/cpuacct.go index 0bb7f5c76..d4104a00e 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cpuacct.go +++ b/pkg/sentry/fsimpl/cgroupfs/cpuacct.go @@ -15,9 +15,14 @@ package cgroupfs import ( + "bytes" + "fmt" + + "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" + "gvisor.dev/gvisor/pkg/sentry/usage" ) // +stateify savable @@ -34,6 +39,76 @@ func newCPUAcctController(fs *filesystem) *cpuacctController { } // AddControlFiles implements controller.AddControlFiles. -func (c *cpuacctController) AddControlFiles(ctx context.Context, creds *auth.Credentials, _ *cgroupInode, contents map[string]kernfs.Inode) { - // This controller is currently intentionally empty. +func (c *cpuacctController) AddControlFiles(ctx context.Context, creds *auth.Credentials, cg *cgroupInode, contents map[string]kernfs.Inode) { + cpuacctCG := &cpuacctCgroup{cg} + contents["cpuacct.stat"] = c.fs.newControllerFile(ctx, creds, &cpuacctStatData{cpuacctCG}) + contents["cpuacct.usage"] = c.fs.newControllerFile(ctx, creds, &cpuacctUsageData{cpuacctCG}) + contents["cpuacct.usage_user"] = c.fs.newControllerFile(ctx, creds, &cpuacctUsageUserData{cpuacctCG}) + contents["cpuacct.usage_sys"] = c.fs.newControllerFile(ctx, creds, &cpuacctUsageSysData{cpuacctCG}) +} + +// +stateify savable +type cpuacctCgroup struct { + *cgroupInode +} + +func (c *cpuacctCgroup) collectCPUStats() usage.CPUStats { + var cs usage.CPUStats + c.fs.tasksMu.RLock() + // Note: This isn't very accurate, since the tasks are potentially + // still running as we accumulate their stats. + for t := range c.ts { + cs.Accumulate(t.CPUStats()) + } + c.fs.tasksMu.RUnlock() + return cs +} + +// +stateify savable +type cpuacctStatData struct { + *cpuacctCgroup +} + +// Generate implements vfs.DynamicBytesSource.Generate. +func (d *cpuacctStatData) Generate(ctx context.Context, buf *bytes.Buffer) error { + cs := d.collectCPUStats() + fmt.Fprintf(buf, "user %d\n", linux.ClockTFromDuration(cs.UserTime)) + fmt.Fprintf(buf, "system %d\n", linux.ClockTFromDuration(cs.SysTime)) + return nil +} + +// +stateify savable +type cpuacctUsageData struct { + *cpuacctCgroup +} + +// Generate implements vfs.DynamicBytesSource.Generate. +func (d *cpuacctUsageData) Generate(ctx context.Context, buf *bytes.Buffer) error { + cs := d.collectCPUStats() + fmt.Fprintf(buf, "%d\n", cs.UserTime.Nanoseconds()+cs.SysTime.Nanoseconds()) + return nil +} + +// +stateify savable +type cpuacctUsageUserData struct { + *cpuacctCgroup +} + +// Generate implements vfs.DynamicBytesSource.Generate. +func (d *cpuacctUsageUserData) Generate(ctx context.Context, buf *bytes.Buffer) error { + cs := d.collectCPUStats() + fmt.Fprintf(buf, "%d\n", cs.UserTime.Nanoseconds()) + return nil +} + +// +stateify savable +type cpuacctUsageSysData struct { + *cpuacctCgroup +} + +// Generate implements vfs.DynamicBytesSource.Generate. +func (d *cpuacctUsageSysData) Generate(ctx context.Context, buf *bytes.Buffer) error { + cs := d.collectCPUStats() + fmt.Fprintf(buf, "%d\n", cs.SysTime.Nanoseconds()) + return nil } diff --git a/test/syscalls/linux/cgroup.cc b/test/syscalls/linux/cgroup.cc index 3a51348ba..a1006a978 100644 --- a/test/syscalls/linux/cgroup.cc +++ b/test/syscalls/linux/cgroup.cc @@ -21,6 +21,7 @@ #include "gtest/gtest.h" #include "absl/container/flat_hash_map.h" #include "absl/container/flat_hash_set.h" +#include "absl/strings/str_split.h" #include "test/util/capability_util.h" #include "test/util/cgroup_util.h" #include "test/util/temp_path.h" @@ -31,6 +32,7 @@ namespace testing { namespace { using ::testing::_; +using ::testing::Ge; using ::testing::Gt; std::vector known_controllers = {"cpu", "cpuset", "cpuacct", @@ -206,6 +208,55 @@ TEST(CPUCgroup, ControlFilesHaveDefaultValues) { IsPosixErrorOkAndHolds(1024)); } +TEST(CPUAcctCgroup, CPUAcctUsage) { + SKIP_IF(!CgroupsAvailable()); + + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + Cgroup c = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("cpuacct")); + + const int64_t usage = + ASSERT_NO_ERRNO_AND_VALUE(c.ReadIntegerControlFile("cpuacct.usage")); + const int64_t usage_user = + ASSERT_NO_ERRNO_AND_VALUE(c.ReadIntegerControlFile("cpuacct.usage_user")); + const int64_t usage_sys = + ASSERT_NO_ERRNO_AND_VALUE(c.ReadIntegerControlFile("cpuacct.usage_sys")); + + EXPECT_GE(usage, 0); + EXPECT_GE(usage_user, 0); + EXPECT_GE(usage_sys, 0); + + EXPECT_GE(usage_user + usage_sys, usage); +} + +TEST(CPUAcctCgroup, CPUAcctStat) { + SKIP_IF(!CgroupsAvailable()); + + Mounter m(ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir())); + Cgroup c = ASSERT_NO_ERRNO_AND_VALUE(m.MountCgroupfs("cpuacct")); + + std::string stat = + ASSERT_NO_ERRNO_AND_VALUE(c.ReadControlFile("cpuacct.stat")); + + // We're expecting the contents of "cpuacct.stat" to look similar to this: + // + // user 377986 + // system 220662 + + std::vector lines = + absl::StrSplit(stat, '\n', absl::SkipEmpty()); + ASSERT_EQ(lines.size(), 2); + + std::vector user_tokens = + StrSplit(lines[0], absl::ByChar(' ')); + EXPECT_EQ(user_tokens[0], "user"); + EXPECT_THAT(Atoi(user_tokens[1]), IsPosixErrorOkAndHolds(Ge(0))); + + std::vector sys_tokens = + StrSplit(lines[1], absl::ByChar(' ')); + EXPECT_EQ(sys_tokens[0], "system"); + EXPECT_THAT(Atoi(sys_tokens[1]), IsPosixErrorOkAndHolds(Ge(0))); +} + TEST(ProcCgroups, Empty) { SKIP_IF(!CgroupsAvailable()); -- cgit v1.2.3