From d34c05a252032aeca7a4d7c52d93e6f838f83340 Mon Sep 17 00:00:00 2001 From: Ismo Puustinen Date: Tue, 18 Apr 2023 13:22:13 +0300 Subject: [PATCH] Intel RDT: updates according to proposed spec changes. There are two proposed clarifications to the OCI spec. The first item specifies that the L3CacheSchema line filtering needs to happen only when both MemBw and L3Cache schemas are specified. The second item specifies that the subdirectory needs to be deleted. Runc already does that, but the clarification adds for directory removal only if the directory was created by us. Signed-off-by: Ismo Puustinen --- libcontainer/intelrdt/intelrdt.go | 54 +++++++++++++++++++++----- libcontainer/intelrdt/intelrdt_test.go | 40 +++++++++++++++++++ 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 0f5c457b099..1b9ee59e4b8 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -146,10 +146,11 @@ import ( */ type Manager struct { - mu sync.Mutex - config *configs.Config - id string - path string + mu sync.Mutex + config *configs.Config + id string + path string + directoryCreated bool } // NewManager returns a new instance of Manager, or nil if the Intel RDT @@ -170,9 +171,10 @@ func NewManager(config *configs.Config, id string, path string) *Manager { // is actually available. Used by unit tests that mock intelrdt paths. func newManager(config *configs.Config, id string, path string) *Manager { return &Manager{ - config: config, - id: id, - path: path, + config: config, + id: id, + path: path, + directoryCreated: false, } } @@ -466,6 +468,14 @@ func (m *Manager) Apply(pid int) (err error) { } } + // If the directory doesn't exist we need to create it -> it means we also need + // to clean it up afterwards. Make a note to the manager. + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + m.directoryCreated = true + } + } + if err := os.MkdirAll(path, 0o755); err != nil { return newLastCmdError(err) } @@ -482,8 +492,9 @@ func (m *Manager) Apply(pid int) (err error) { func (m *Manager) Destroy() error { // Don't remove resctrl group if closid has been explicitly specified. The // group is likely externally managed, i.e. by some other entity than us. - // There are probably other containers/tasks sharing the same group. - if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID == "" { + // There are probably other containers/tasks sharing the same group. Also + // only remove the directory if it was created by us. + if m.config.IntelRdt != nil && m.config.IntelRdt.ClosID == "" && m.directoryCreated { m.mu.Lock() defer m.mu.Unlock() if err := os.RemoveAll(m.GetPath()); err != nil { @@ -589,6 +600,28 @@ func (m *Manager) GetStats() (*Stats, error) { return stats, nil } +func combineSchemas(l3CacheSchema, memBwSchema string) string { + // If both l3CacheSchema and memBwSchema are set and + // l3CacheSchema contains a line beginning with "MB:", the + // value written to schemata file MUST be the non-"MB:" + // line(s) from l3CacheSchema and the line from memBWSchema. + + validLines := make([]string, 0) + + // Split the l3CacheSchema to lines. + lines := strings.Split(l3CacheSchema, "\n") + + // Remove the "MB:" lines. + for _, line := range lines { + if strings.HasPrefix(line, "MB:") { + continue + } + validLines = append(validLines, line) + } + + return strings.Join(validLines, "\n") + "\n" + memBwSchema +} + // Set Intel RDT "resource control" filesystem as configured. func (m *Manager) Set(container *configs.Config) error { // About L3 cache schema: @@ -649,7 +682,8 @@ func (m *Manager) Set(container *configs.Config) error { // Write a single joint schema string to schemata file if l3CacheSchema != "" && memBwSchema != "" { - if err := writeFile(path, "schemata", l3CacheSchema+"\n"+memBwSchema); err != nil { + schemata := combineSchemas(l3CacheSchema, memBwSchema) + if err := writeFile(path, "schemata", schemata); err != nil { return err } } diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index c127cd8f7c6..237182f52b9 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -67,6 +67,46 @@ func TestIntelRdtSetMemBwSchema(t *testing.T) { } } +func TestIntelRdtSetCombinedSchema(t *testing.T) { + helper := NewIntelRdtTestUtil(t) + + // Test filtering out the "MB:"" line in l3CacheSchema. + + const ( + schemaBefore = "MB:0=20;1=70" + memBwSchema = "MB:0=70;1=20" + l3CacheSchema = "MB:0=80;1=10\nL3:0=f0;1=f" + combinedSchemaAfter = "L3:0=f0;1=f\nMB:0=70;1=20" + ) + + helper.writeFileContents(map[string]string{ + "schemata": schemaBefore + "\n", + }) + + helper.config.IntelRdt.MemBwSchema = memBwSchema + helper.config.IntelRdt.L3CacheSchema = l3CacheSchema + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) + if err := intelrdt.Set(helper.config); err != nil { + t.Fatal(err) + } + + tmpStrings, err := getIntelRdtParamString(helper.IntelRdtPath, "schemata") + if err != nil { + t.Fatalf("Failed to parse file 'schemata' - %s", err) + } + + readValues := strings.Split(tmpStrings, "\n") + expectedValues := strings.Split(combinedSchemaAfter, "\n") + + if readValues[0] != expectedValues[0] { + t.Fatal("Got the wrong value for L3 cache, set 'schemata' failed.") + } + + if readValues[1] != expectedValues[1] { + t.Fatal("Got the wrong value for MemBW, set 'schemata' failed.") + } +} + func TestIntelRdtSetMemBwScSchema(t *testing.T) { helper := NewIntelRdtTestUtil(t)