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

Use update time to detect if kmem limits have been set #935

Merged
merged 2 commits into from
Jul 11, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
62 changes: 36 additions & 26 deletions libcontainer/cgroups/fs/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,14 @@ package fs
import (
"bufio"
"fmt"
"math"
"os"
"path/filepath"
"strconv"
"strings"
"time"

"github.com/opencontainers/runc/libcontainer/cgroups"
"github.com/opencontainers/runc/libcontainer/configs"
"github.com/opencontainers/runc/libcontainer/system"
)

type MemoryGroup struct {
Expand All @@ -28,53 +27,65 @@ func (s *MemoryGroup) Apply(d *cgroupData) (err error) {
if err != nil && !cgroups.IsNotFound(err) {
return err
}
if memoryAssigned(d.config) {
if path != "" {
if err := os.MkdirAll(path, 0755); err != nil {
return err
}
}
// We have to set kernel memory here, as we can't change it once
// processes have been attached to the cgroup.
if err := s.SetKernelMemory(path, d.config); err != nil {
return err
}
// reset error.
err = nil
if path == "" {
// Invalid input.
return fmt.Errorf("invalid path for memory cgroups: %+v", d)
}

defer func() {
if err != nil {
os.RemoveAll(path)
}
}()

if !cgroups.PathExists(path) {
if err = os.MkdirAll(path, 0755); err != nil {
return err
}
}
if memoryAssigned(d.config) {
// We have to set kernel memory here, as we can't change it once
// processes have been attached to the cgroup.
if err = s.SetKernelMemory(path, d.config); err != nil {
return err
}
}
// We need to join memory cgroup after set memory limits, because
// kmem.limit_in_bytes can only be set when the cgroup is empty.
_, err = d.join("memory")
if err != nil && !cgroups.IsNotFound(err) {
if _, jerr := d.join("memory"); jerr != nil && !cgroups.IsNotFound(jerr) {
err = jerr
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

could just return jerr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

err needs to be set for the defer function to execute.

}
return nil
}

func getModifyTime(path string) (time.Time, error) {
stat, err := os.Stat(path)
if err != nil {
return time.Time{}, fmt.Errorf("failed to get memory cgroups creation time: %v", err)
}
return stat.ModTime(), nil
}

func (s *MemoryGroup) SetKernelMemory(path string, cgroup *configs.Cgroup) error {
// This has to be done separately because it has special
// constraints (it can only be initialized before setting up a
// hierarchy or adding a task to the cgroups. However, if
// sucessfully initialized, it can be updated anytime afterwards)
if cgroup.Resources.KernelMemory != 0 {
kmemInitialized := false
// Is kmem.limit_in_bytes already set?
kmemValue, err := getCgroupParamUint(path, "memory.kmem.limit_in_bytes")
// memory.kmem.max_usage_in_bytes is a read-only file. Use it to get cgroups creation time.
kmemCreationTime, err := getModifyTime(filepath.Join(path, "memory.kmem.max_usage_in_bytes"))
if err != nil {
return err
}
switch system.GetLongBit() {
case 32:
kmemInitialized = uint32(kmemValue) != uint32(math.MaxUint32)
case 64:
kmemInitialized = kmemValue != uint64(math.MaxUint64)
kmemLimitsUpdateTime, err := getModifyTime(filepath.Join(path, "memory.kmem.limit_in_bytes"))
if err != nil {
return err
}

// kmem.limit_in_bytes has already been set if its update time is after that of creation time.
// We use `!=` op instead of `>` because updates are losing precision compared to creation.
kmemInitialized := !kmemLimitsUpdateTime.Equal(kmemCreationTime)
if !kmemInitialized {
// If there's already tasks in the cgroup, we can't change the limit either
tasks, err := getCgroupParamString(path, "tasks")
Expand All @@ -85,7 +96,6 @@ func (s *MemoryGroup) SetKernelMemory(path string, cgroup *configs.Cgroup) error
return fmt.Errorf("cannot set kmem.limit_in_bytes after task have joined this cgroup")
}
}

if err := writeFile(path, "memory.kmem.limit_in_bytes", strconv.FormatInt(cgroup.Resources.KernelMemory, 10)); err != nil {
return err
}
Expand Down
6 changes: 6 additions & 0 deletions libcontainer/cgroups/fs/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,12 @@ func TestMemorySetKernelMemory(t *testing.T) {
helper.writeFileContents(map[string]string{
"memory.kmem.limit_in_bytes": strconv.Itoa(kernelMemoryBefore),
})
helper.writeFileContents(map[string]string{
"memory.kmem.max_usage_in_bytes": strconv.Itoa(kernelMemoryBefore),
})
helper.writeFileContents(map[string]string{
"tasks": "",
})

helper.CgroupData.config.Resources.KernelMemory = kernelMemoryAfter
memory := &MemoryGroup{}
Expand Down
19 changes: 0 additions & 19 deletions libcontainer/system/sysconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,9 @@ package system

/*
#include <unistd.h>
#include <limits.h>

int GetLongBit() {
#ifdef _SC_LONG_BIT
int longbits;

longbits = sysconf(_SC_LONG_BIT);
if (longbits < 0) {
longbits = (CHAR_BIT * sizeof(long));
}
return longbits;
#else
return (CHAR_BIT * sizeof(long));
#endif
}
*/
import "C"

func GetClockTicks() int {
return int(C.sysconf(C._SC_CLK_TCK))
}

func GetLongBit() int {
return int(C.GetLongBit())
}