Skip to content

Commit

Permalink
[1.1] libct: rm intelrtd.Manager interface, NewIntelRdtManager
Browse files Browse the repository at this point in the history
[This is a manual port of commit dbd990d to release-1.1
branch. Original description follows.]

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 <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Aug 9, 2023
1 parent b4f7bf2 commit ea2adae
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 93 deletions.
2 changes: 1 addition & 1 deletion libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type linuxContainer struct {
root string
config *configs.Config
cgroupManager cgroups.Manager
intelRdtManager intelrdt.Manager
intelRdtManager *intelrdt.Manager
initPath string
initArgs []string
initProcess parentProcess
Expand Down
43 changes: 11 additions & 32 deletions libcontainer/factory_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,6 @@ func InitArgs(args ...string) func(*LinuxFactory) error {
}
}

// 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)
Expand Down Expand Up @@ -136,9 +122,6 @@ type LinuxFactory struct {

// Validator provides validation to container configurations.
Validator validate.Validator

// 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) {
Expand Down Expand Up @@ -208,18 +191,16 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err
return nil, err
}
c := &linuxContainer{
id: id,
root: containerRoot,
config: config,
initPath: l.InitPath,
initArgs: l.InitArgs,
criuPath: l.CriuPath,
newuidmapPath: l.NewuidmapPath,
newgidmapPath: l.NewgidmapPath,
cgroupManager: cm,
}
if l.NewIntelRdtManager != nil {
c.intelRdtManager = l.NewIntelRdtManager(config, id, "")
id: id,
root: containerRoot,
config: config,
initPath: l.InitPath,
initArgs: l.InitArgs,
criuPath: l.CriuPath,
newuidmapPath: l.NewuidmapPath,
newgidmapPath: l.NewgidmapPath,
cgroupManager: cm,
intelRdtManager: intelrdt.NewManager(config, id, ""),
}
c.state = &stoppedState{c: c}
return c, nil
Expand Down Expand Up @@ -261,12 +242,10 @@ func (l *LinuxFactory) Load(id string) (Container, error) {
newuidmapPath: l.NewuidmapPath,
newgidmapPath: l.NewgidmapPath,
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
Expand Down
24 changes: 1 addition & 23 deletions libcontainer/factory_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
49 changes: 21 additions & 28 deletions libcontainer/intelrdt/intelrdt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
// 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.
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{
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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -574,15 +567,15 @@ 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()
}
return m.path
}

// 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
Expand Down Expand Up @@ -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).
Expand Down
10 changes: 5 additions & 5 deletions libcontainer/intelrdt/intelrdt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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")
}
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/process_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ type initProcess struct {
logFilePair filePair
config *initConfig
manager cgroups.Manager
intelRdtManager intelrdt.Manager
intelRdtManager *intelrdt.Manager
container *linuxContainer
fds []string
process *Process
Expand Down
4 changes: 1 addition & 3 deletions utils_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
return nil, err
}

intelRdtManager := libcontainer.IntelRdtFs

// We resolve the paths for {newuidmap,newgidmap} from the context of runc,
// to avoid doing a path lookup in the nsexec context. TODO: The binary
// names are not currently configurable.
Expand All @@ -46,7 +44,7 @@ func loadFactory(context *cli.Context) (libcontainer.Factory, error) {
newgidmap = ""
}

return libcontainer.New(abs, intelRdtManager,
return libcontainer.New(abs,
libcontainer.CriuPath(context.GlobalString("criu")),
libcontainer.NewuidmapPath(newuidmap),
libcontainer.NewgidmapPath(newgidmap))
Expand Down

0 comments on commit ea2adae

Please sign in to comment.