Skip to content

Commit

Permalink
cgroupManager: add/use AddPid
Browse files Browse the repository at this point in the history
Commit df3fa11 adds a second call to cgroupManager.Apply() method in
order to add a second pid to the cgroup.

While I admit I don't understand the reason why it was added, it seems
that going through all the steps (configure cgroup parameters, figure
out paths to all controllers, create a systemd unit) twice is excessive
and should not be done.

Add and use AddPid method for adding the second PID to the cgroup.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Jun 1, 2020
1 parent dbe5aca commit 27b9830
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 1 deletion.
3 changes: 3 additions & 0 deletions libcontainer/cgroups/cgroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ type Manager interface {
// Applies cgroup configuration to the process with the specified pid
Apply(pid int) error

// AddPid adds another process to an already created and configured
AddPid(pid int) error

// Returns the PIDs inside the cgroup set
GetPids() ([]int, error)

Expand Down
7 changes: 7 additions & 0 deletions libcontainer/cgroups/fs/apply_raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ func (m *manager) Apply(pid int) (err error) {
return nil
}

func (m *manager) AddPid(pid int) error {
if m.paths == nil {
return errors.New("cgroup is not yet configured")
}
return cgroups.EnterPid(m.paths, pid)
}

func (m *manager) Destroy() error {
if m.cgroups == nil || m.cgroups.Paths != nil {
return nil
Expand Down
7 changes: 7 additions & 0 deletions libcontainer/cgroups/fs2/fs2.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,13 @@ func (m *manager) Apply(pid int) error {
return nil
}

func (m *manager) AddPid(pid int) error {
if m.dirPath != "" {
return errors.New("cgroup is not yet configured")
}
return cgroups.WriteCgroupProc(m.dirPath, pid)
}

func (m *manager) GetPids() ([]int, error) {
return cgroups.GetPids(m.dirPath)
}
Expand Down
7 changes: 7 additions & 0 deletions libcontainer/cgroups/systemd/v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,13 @@ func (m *legacyManager) Apply(pid int) error {
return nil
}

func (m *legacyManager) AddPid(pid int) error {
if m.paths == nil {
return errors.New("cgroup is not yet configured")
}
return cgroups.EnterPid(m.paths, pid)
}

func (m *legacyManager) Destroy() error {
if m.cgroups.Paths != nil {
return nil
Expand Down
7 changes: 7 additions & 0 deletions libcontainer/cgroups/systemd/v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,13 @@ func (m *unifiedManager) Apply(pid int) error {
return nil
}

func (m *unifiedManager) AddPid(pid int) error {
if m.path == "" {
return errors.New("cgroup is not yet configured")
}
return cgroups.WriteCgroupProc(m.path, pid)
}

func (m *unifiedManager) Destroy() error {
if m.cgroups.Paths != nil {
return nil
Expand Down
2 changes: 1 addition & 1 deletion libcontainer/process_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ func (p *initProcess) start() (retErr error) {
p.setExternalDescriptors(fds)
// Do this before syncing with child so that no children
// can escape the cgroup
if err := p.manager.Apply(childPid); err != nil {
if err := p.manager.AddPid(childPid); err != nil {
return newSystemErrorWithCause(err, "applying cgroup configuration for process")
}
if p.intelRdtManager != nil {
Expand Down

0 comments on commit 27b9830

Please sign in to comment.