Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the cgroups v2 hybrid check from init() #3432

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 28 additions & 26 deletions libcontainer/cgroups/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,6 @@ import (
"github.com/opencontainers/runc/libcontainer/configs"
)

var subsystems = []subsystem{
&CpusetGroup{},
&DevicesGroup{},
&MemoryGroup{},
&CpuGroup{},
&CpuacctGroup{},
&PidsGroup{},
&BlkioGroup{},
&HugetlbGroup{},
&NetClsGroup{},
&NetPrioGroup{},
&PerfEventGroup{},
&FreezerGroup{},
&RdmaGroup{},
&NameGroup{GroupName: "name=systemd", Join: true},
}

var errSubsystemDoesNotExist = errors.New("cgroup: subsystem does not exist")

type subsystem interface {
Expand All @@ -46,9 +29,10 @@ type subsystem interface {
}

type manager struct {
mu sync.Mutex
cgroups *configs.Cgroup
paths map[string]string
mu sync.Mutex
cgroups *configs.Cgroup
paths map[string]string
subsystems []subsystem
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not make sense to have a per-manager subsystem list.

Instead, I think, we should keep subsystems global, and wrap their initialization into once.Do.

}

func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, error) {
Expand All @@ -61,6 +45,23 @@ func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, e
return nil, cgroups.ErrV1NoUnified
}

subsystems := []subsystem{
&CpusetGroup{},
&DevicesGroup{},
&MemoryGroup{},
&CpuGroup{},
&CpuacctGroup{},
&PidsGroup{},
&BlkioGroup{},
&HugetlbGroup{},
&NetClsGroup{},
&NetPrioGroup{},
&PerfEventGroup{},
&FreezerGroup{},
&RdmaGroup{},
&NameGroup{GroupName: "name=systemd", Join: true},
}

// If using cgroups-hybrid mode then add a "" controller indicating
// it should join the cgroups v2.
if cgroups.IsCgroup2HybridMode() {
Expand All @@ -69,15 +70,16 @@ func NewManager(cg *configs.Cgroup, paths map[string]string) (cgroups.Manager, e

if paths == nil {
var err error
paths, err = initPaths(cg)
paths, err = initPaths(cg, subsystems)
if err != nil {
return nil, err
}
}

return &manager{
cgroups: cg,
paths: paths,
cgroups: cg,
paths: paths,
subsystems: subsystems,
}, nil
}

Expand Down Expand Up @@ -108,7 +110,7 @@ func (m *manager) Apply(pid int) (err error) {

c := m.cgroups

for _, sys := range subsystems {
for _, sys := range m.subsystems {
name := sys.Name()
p, ok := m.paths[name]
if !ok {
Expand Down Expand Up @@ -152,7 +154,7 @@ func (m *manager) GetStats() (*cgroups.Stats, error) {
m.mu.Lock()
defer m.mu.Unlock()
stats := cgroups.NewStats()
for _, sys := range subsystems {
for _, sys := range m.subsystems {
path := m.paths[sys.Name()]
if path == "" {
continue
Expand All @@ -175,7 +177,7 @@ func (m *manager) Set(r *configs.Resources) error {

m.mu.Lock()
defer m.mu.Unlock()
for _, sys := range subsystems {
for _, sys := range m.subsystems {
path := m.paths[sys.Name()]
if err := sys.Set(path, r); err != nil {
// When rootless is true, errors from the device subsystem
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/cgroups/fs/paths.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var (

const defaultCgroupRoot = "/sys/fs/cgroup"

func initPaths(cg *configs.Cgroup) (map[string]string, error) {
func initPaths(cg *configs.Cgroup, subsystems []subsystem) (map[string]string, error) {
root, err := rootPath()
if err != nil {
return nil, err
Expand Down