Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
cgroupv1: check cpu shares in place
Commit 4e65e0e added a check for cpu shares. Apparently, the
kernel allows to set a value higher than max or lower than min without
an error, but the value read back is always within the limits.

The check (which was later moved out to a separate CheckCpushares()
function) is always performed after setting the cpu shares, so let's
move it to the very place where it is set.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed May 29, 2020
1 parent 774a9e7 commit 3249e23
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 39 deletions.
33 changes: 0 additions & 33 deletions libcontainer/cgroups/fs/apply_raw.go
Expand Up @@ -4,7 +4,6 @@ package fs

import (
"fmt"
"io"
"os"
"path/filepath"
"sync"
Expand Down Expand Up @@ -259,11 +258,6 @@ func (m *manager) Set(container *configs.Config) error {
}
}

if m.paths["cpu"] != "" {
if err := CheckCpushares(m.paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil {
return err
}
}
return nil
}

Expand Down Expand Up @@ -377,33 +371,6 @@ func removePath(p string, err error) error {
return nil
}

func CheckCpushares(path string, c uint64) error {
var cpuShares uint64

if c == 0 {
return nil
}

fd, err := os.Open(filepath.Join(path, "cpu.shares"))
if err != nil {
return err
}
defer fd.Close()

_, err = fmt.Fscanf(fd, "%d", &cpuShares)
if err != nil && err != io.EOF {
return err
}

if c > cpuShares {
return fmt.Errorf("The maximum allowed cpu-shares is %d", cpuShares)
} else if c < cpuShares {
return fmt.Errorf("The minimum allowed cpu-shares is %d", cpuShares)
}

return nil
}

func (m *manager) GetPaths() map[string]string {
m.mu.Lock()
defer m.mu.Unlock()
Expand Down
15 changes: 14 additions & 1 deletion libcontainer/cgroups/fs/cpu.go
Expand Up @@ -4,6 +4,7 @@ package fs

import (
"bufio"
"fmt"
"os"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -66,9 +67,21 @@ func (s *CpuGroup) SetRtSched(path string, cgroup *configs.Cgroup) error {

func (s *CpuGroup) Set(path string, cgroup *configs.Cgroup) error {
if cgroup.Resources.CpuShares != 0 {
if err := fscommon.WriteFile(path, "cpu.shares", strconv.FormatUint(cgroup.Resources.CpuShares, 10)); err != nil {
shares := cgroup.Resources.CpuShares
if err := fscommon.WriteFile(path, "cpu.shares", strconv.FormatUint(shares, 10)); err != nil {
return err
}
// read it back
sharesRead, err := fscommon.GetCgroupParamUint(path, "cpu.shares")
if err != nil {
return err
}
// ... and check
if shares > sharesRead {
return fmt.Errorf("the maximum allowed cpu-shares is %d", sharesRead)
} else if shares < sharesRead {
return fmt.Errorf("the minimum allowed cpu-shares is %d", sharesRead)
}
}
if cgroup.Resources.CpuPeriod != 0 {
if err := fscommon.WriteFile(path, "cpu.cfs_period_us", strconv.FormatUint(cgroup.Resources.CpuPeriod, 10)); err != nil {
Expand Down
5 changes: 0 additions & 5 deletions libcontainer/cgroups/systemd/v1.go
Expand Up @@ -435,11 +435,6 @@ func (m *legacyManager) Set(container *configs.Config) error {
}
}

if m.paths["cpu"] != "" {
if err := fs.CheckCpushares(m.paths["cpu"], container.Cgroups.Resources.CpuShares); err != nil {
return err
}
}
return nil
}

Expand Down

0 comments on commit 3249e23

Please sign in to comment.