From f817847704130df53b1fc6d7b62b9c529766d678 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 27 Jan 2022 10:58:18 -0800 Subject: [PATCH] libct: rm intelrtd.Manager interface, NewIntelRdtManager Remove intelrtd.Manager interface, since we only have a single implementation, and do not expect another one. Rename intelRdtManager to Manager, and modify its users accordingly. Remove NewIntelRdtManager from factory. Remove IntelRdtfs. Instead, make intelrdt.NewManager return nil if the feature is not available. Remove TestFactoryNewIntelRdt as it is now identical to TestFactoryNew. Add internal function newManager to be used for tests (to make sure some testing is done even when the feature is not available in kernel/hardware). Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux.go | 2 +- libcontainer/factory_linux.go | 33 ++++------------- libcontainer/factory_linux_test.go | 24 +------------ libcontainer/intelrdt/intelrdt.go | 49 +++++++++++--------------- libcontainer/intelrdt/intelrdt_test.go | 10 +++--- libcontainer/process_linux.go | 2 +- utils_linux.go | 4 +-- 7 files changed, 36 insertions(+), 88 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index e56bd742b5e..293f1fa2fd7 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -41,7 +41,7 @@ type linuxContainer struct { root string config *configs.Config cgroupManager cgroups.Manager - intelRdtManager intelrdt.Manager + intelRdtManager *intelrdt.Manager initProcess parentProcess initProcessStartTime uint64 m sync.Mutex diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index afb0a43fa83..a0ea4fe3f2b 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -28,20 +28,6 @@ const ( var idRegex = regexp.MustCompile(`^[\w+-\.]+$`) -// IntelRdtfs is an options func to configure a LinuxFactory to return -// containers that use the Intel RDT "resource control" filesystem to -// create and manage Intel RDT resources (e.g., L3 cache, memory bandwidth). -func IntelRdtFs(l *LinuxFactory) error { - if !intelrdt.IsCATEnabled() && !intelrdt.IsMBAEnabled() { - l.NewIntelRdtManager = nil - } else { - l.NewIntelRdtManager = func(config *configs.Config, id string, path string) intelrdt.Manager { - return intelrdt.NewManager(config, id, path) - } - } - return nil -} - // TmpfsRoot is an option func to mount LinuxFactory.Root to tmpfs. func TmpfsRoot(l *LinuxFactory) error { mounted, err := mountinfo.Mounted(l.Root) @@ -83,9 +69,6 @@ func New(root string, options ...func(*LinuxFactory) error) (Factory, error) { type LinuxFactory struct { // Root directory for the factory to store state. Root string - - // NewIntelRdtManager returns an initialized Intel RDT manager for a single container. - NewIntelRdtManager func(config *configs.Config, id string, path string) intelrdt.Manager } func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, error) { @@ -146,13 +129,11 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err return nil, err } c := &linuxContainer{ - id: id, - root: containerRoot, - config: config, - cgroupManager: cm, - } - if l.NewIntelRdtManager != nil { - c.intelRdtManager = l.NewIntelRdtManager(config, id, "") + id: id, + root: containerRoot, + config: config, + cgroupManager: cm, + intelRdtManager: intelrdt.NewManager(config, id, ""), } c.state = &stoppedState{c: c} return c, nil @@ -189,12 +170,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) { id: id, config: &state.Config, cgroupManager: cm, + intelRdtManager: intelrdt.NewManager(&state.Config, id, state.IntelRdtPath), root: containerRoot, created: state.Created, } - if l.NewIntelRdtManager != nil { - c.intelRdtManager = l.NewIntelRdtManager(&state.Config, id, state.IntelRdtPath) - } c.state = &loadedState{c: c} if err := c.refreshState(); err != nil { return nil, err diff --git a/libcontainer/factory_linux_test.go b/libcontainer/factory_linux_test.go index d29c32e9dbf..47f3069953b 100644 --- a/libcontainer/factory_linux_test.go +++ b/libcontainer/factory_linux_test.go @@ -37,28 +37,6 @@ func TestFactoryNew(t *testing.T) { } } -func TestFactoryNewIntelRdt(t *testing.T) { - root := t.TempDir() - factory, err := New(root, IntelRdtFs) - if err != nil { - t.Fatal(err) - } - if factory == nil { - t.Fatal("factory should not be nil") - } - lfactory, ok := factory.(*LinuxFactory) - if !ok { - t.Fatal("expected linux factory returned on linux based systems") - } - if lfactory.Root != root { - t.Fatalf("expected factory root to be %q but received %q", root, lfactory.Root) - } - - if factory.Type() != "libcontainer" { - t.Fatalf("unexpected factory type: %q, expected %q", factory.Type(), "libcontainer") - } -} - func TestFactoryNewTmpfs(t *testing.T) { root := t.TempDir() factory, err := New(root, TmpfsRoot) @@ -157,7 +135,7 @@ func TestFactoryLoadContainer(t *testing.T) { if err := marshal(filepath.Join(root, id, stateFilename), expectedState); err != nil { t.Fatal(err) } - factory, err := New(root, IntelRdtFs) + factory, err := New(root) if err != nil { t.Fatal(err) } diff --git a/libcontainer/intelrdt/intelrdt.go b/libcontainer/intelrdt/intelrdt.go index 1fe1ec3e86c..3953f930d25 100644 --- a/libcontainer/intelrdt/intelrdt.go +++ b/libcontainer/intelrdt/intelrdt.go @@ -146,34 +146,27 @@ import ( * } */ -type Manager interface { - // Applies Intel RDT configuration to the process with the specified pid - Apply(pid int) error - - // Returns statistics for Intel RDT - GetStats() (*Stats, error) - - // Destroys the Intel RDT container-specific 'container_id' group - Destroy() error - - // Returns Intel RDT path to save in a state file and to be able to - // restore the object later - GetPath() string - - // Set Intel RDT "resource control" filesystem as configured. - Set(container *configs.Config) error -} - -// This implements interface Manager -type intelRdtManager struct { +type Manager struct { mu sync.Mutex config *configs.Config id string path string } -func NewManager(config *configs.Config, id string, path string) Manager { - return &intelRdtManager{ +// NewManager returns a new instance of Manager, or nil, if the Intel RDT +// functionality is not available from hardware or not enabled in the kernel. +func NewManager(config *configs.Config, id string, path string) *Manager { + if _, err := Root(); err != nil { + // Intel RDT is not available. + return nil + } + return newManager(config, id, path) +} + +// newManager is the same as NewManager, except it does not check if the feature +// 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, @@ -507,7 +500,7 @@ func IsMBAScEnabled() bool { } // Get the path of the clos group in "resource control" filesystem that the container belongs to -func (m *intelRdtManager) getIntelRdtPath() (string, error) { +func (m *Manager) getIntelRdtPath() (string, error) { rootPath, err := Root() if err != nil { return "", err @@ -522,7 +515,7 @@ func (m *intelRdtManager) getIntelRdtPath() (string, error) { } // Applies Intel RDT configuration to the process with the specified pid -func (m *intelRdtManager) Apply(pid int) (err error) { +func (m *Manager) Apply(pid int) (err error) { // If intelRdt is not specified in config, we do nothing if m.config.IntelRdt == nil { return nil @@ -557,7 +550,7 @@ func (m *intelRdtManager) Apply(pid int) (err error) { } // Destroys the Intel RDT container-specific 'container_id' group -func (m *intelRdtManager) Destroy() 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. @@ -574,7 +567,7 @@ func (m *intelRdtManager) Destroy() error { // Returns Intel RDT path to save in a state file and to be able to // restore the object later -func (m *intelRdtManager) GetPath() string { +func (m *Manager) GetPath() string { if m.path == "" { m.path, _ = m.getIntelRdtPath() } @@ -582,7 +575,7 @@ func (m *intelRdtManager) GetPath() string { } // Returns statistics for Intel RDT -func (m *intelRdtManager) GetStats() (*Stats, error) { +func (m *Manager) GetStats() (*Stats, error) { // If intelRdt is not specified in config if m.config.IntelRdt == nil { return nil, nil @@ -668,7 +661,7 @@ func (m *intelRdtManager) GetStats() (*Stats, error) { } // Set Intel RDT "resource control" filesystem as configured. -func (m *intelRdtManager) Set(container *configs.Config) error { +func (m *Manager) Set(container *configs.Config) error { // About L3 cache schema: // It has allocation bitmasks/values for L3 cache on each socket, // which contains L3 cache id and capacity bitmask (CBM). diff --git a/libcontainer/intelrdt/intelrdt_test.go b/libcontainer/intelrdt/intelrdt_test.go index 2184a1468df..c127cd8f7c6 100644 --- a/libcontainer/intelrdt/intelrdt_test.go +++ b/libcontainer/intelrdt/intelrdt_test.go @@ -20,7 +20,7 @@ func TestIntelRdtSetL3CacheSchema(t *testing.T) { }) helper.config.IntelRdt.L3CacheSchema = l3CacheSchemeAfter - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Set(helper.config); err != nil { t.Fatal(err) } @@ -50,7 +50,7 @@ func TestIntelRdtSetMemBwSchema(t *testing.T) { }) helper.config.IntelRdt.MemBwSchema = memBwSchemeAfter - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Set(helper.config); err != nil { t.Fatal(err) } @@ -80,7 +80,7 @@ func TestIntelRdtSetMemBwScSchema(t *testing.T) { }) helper.config.IntelRdt.MemBwSchema = memBwScSchemeAfter - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Set(helper.config); err != nil { t.Fatal(err) } @@ -103,7 +103,7 @@ func TestApply(t *testing.T) { const closID = "test-clos" helper.config.IntelRdt.ClosID = closID - intelrdt := NewManager(helper.config, "", helper.IntelRdtPath) + intelrdt := newManager(helper.config, "", helper.IntelRdtPath) if err := intelrdt.Apply(1234); err == nil { t.Fatal("unexpected success when applying pid") } @@ -112,7 +112,7 @@ func TestApply(t *testing.T) { } // Dir should be created if some schema has been specified - intelrdt.(*intelRdtManager).config.IntelRdt.L3CacheSchema = "L3:0=f" + intelrdt.config.IntelRdt.L3CacheSchema = "L3:0=f" if err := intelrdt.Apply(1235); err != nil { t.Fatalf("Apply() failed: %v", err) } diff --git a/libcontainer/process_linux.go b/libcontainer/process_linux.go index 97f28a1ab30..ae91d5b900f 100644 --- a/libcontainer/process_linux.go +++ b/libcontainer/process_linux.go @@ -303,7 +303,7 @@ type initProcess struct { logFilePair filePair config *initConfig manager cgroups.Manager - intelRdtManager intelrdt.Manager + intelRdtManager *intelrdt.Manager container *linuxContainer fds []string process *Process diff --git a/utils_linux.go b/utils_linux.go index d49b024ec48..c2214a23377 100644 --- a/utils_linux.go +++ b/utils_linux.go @@ -31,9 +31,7 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) { return nil, err } - intelRdtManager := libcontainer.IntelRdtFs - - return libcontainer.New(abs, intelRdtManager) + return libcontainer.New(abs) } // getContainer returns the specified container instance by loading it from state