...by checking the default path first.
Quick benchmark shows it's about 5x faster on an idle system, and the
gain should be much more on a system doing mounts etc.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In case cgroupPath is under the default cgroup prefix, let's try to
guess the mount point by adding the subsystem name to the default
prefix, and resolving the resulting path in case it's a symlink.
In most cases, given the default cgroup setup, this trick
should result in returning the same result faster, and avoiding
/proc/self/mountinfo parsing which is relatively slow and problematic.
Be very careful with the default path, checking it is
- a directory;
- a mount point;
- has cgroup fstype.
If something is not right, fall back to parsing mountinfo.
While at it, remove the obsoleted comment about mountinfo parsing. The
comment belongs to findCgroupMountpointAndRootFromReader(), but rather
than moving it there, let's just remove it, since it does not add any
value in understanding the current code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In manager.Apply() method, a path to each subsystem is obtained by
calling d.path(sys.Name()), and the sys.Apply() is called that does
the same call to d.path() again.
d.path() is an expensive call, so rather than to call it twice, let's
reuse the result.
This results the number of times we parse mountinfo during container
start from 62 to 34 on my setup.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of iterating over m.paths, iterate over subsystems and look up
the path for each. This is faster since a map lookup is faster than
iterating over the names in Get. A quick benchmark shows that the new
way is 2.5x faster than the old one.
Note though that this is not done to make things faster, as savings are
negligible, but to make things simpler by removing some code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Half of controllers' GetStats just return nil, and most of the others
ignore ENOENT on files, so it will be cheaper to not check that the
path exists in the main GetStats method, offloading that to the
controllers.
Drop PathExists check from GetStats, add it to those controllers'
GetStats where it was missing.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
some hierarchies were created directly by .Apply() on top of systemd
managed cgroups. systemd doesn't manage these and as a result we leak
these cgroups.
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
The result of cgroupv1.FindCgroupMountpoint() call (which is relatively
expensive) is only used in case raw.innerPath is absolute, so it only
makes sense to call it in that case.
This drastically reduces the number of calls to FindCgroupMountpoint
during container start (from 116 to 62 in my setup).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In here, defer looks like an overkill, since the code is very simple and
we already have an error path.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Iterating over the list of subsystems and comparing their names to get an
instance of fs.cgroupFreezer is useless and a waste of time, since it is
a shallow type (i.e. does not have any data/state) and we can create an
instance in place.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The kubelet uses libct/cgroups code to set up cgroups. It creates a
parent cgroup (kubepods) to put the containers into.
The problem (for cgroupv2 that uses eBPF for device configuration) is
the hard requirement to have devices cgroup configured results in
leaking an eBPF program upon every kubelet restart. program. If kubelet
is restarted 64+ times, the cgroup can't be configured anymore.
Work around this by adding a SkipDevices flag to Resources.
A check was added so that if SkipDevices is set, such a "container"
can't be started (to make sure it is only used for non-containers).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
there have been cases observed where instead of `v$VER.0-$OS` the systemdVersion returned is just `$VER`, or `$VER-1`.
handle these cases
Signed-off-by: Peter Hunt <pehunt@redhat.com>
Not sure why but the errors from scanner were ignored. Such errors
can happen if open(2) has succeeded but the subsequent read(2) fails.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
For some reason, runc systemd drivers (both v1 and v2) never set
systemd unit property named `CPUQuotaPeriod` (known as
`CPUQuotaPeriodUSec` on dbus and in `systemctl show` output).
Set it, and add a check to all the integration tests. The check is less
than trivial because, when not set, the value is shown as "infinity" but
when set to the same (default) value, shown as "100ms", so in case we
expect 100ms (period = 100000 us), we have to _also_ check for
"infinity".
[v2: add systemd version checks since CPUQuotaPeriod requires v242+]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When testing GetCgroupMounts, the map data is supposed to be obtained
from /proc/self/cgroup, but since we're mocking things, we provide
our own map.
Unfortunately, not all controllers existing in mountinfos were listed.
Also, "name=systemd" needs special handling, so add it.
The controllers added were:
* for fedoraMountinfo case: name=systemd
* for systemdMountinfo case: name=systemd, net_prio
* for bedrockMountinfo case: name=systemd, net_prio, pids
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
In most project, "utils" is a big mess, and this is not an exception.
Try to clean it up a bit by moving cgroup v1 specific code to a separate
source file.
There are no code changes in this commit, just moving it from one file
to another.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This function is cgroupv1-specific, is only used once, and its name
is very close to the name of another function, FindCgroupMountpoint.
Inline it into the (only) caller.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This function is only called from cgroupv1 code, so there is no need
for it to implement cgroupv2 stuff.
Make it v1-specific, and panic if it is called from v2 code (since this
is an internal function, the panic would mean incorrect runc code).
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It's bad and wrong to use these functions for any cgroupv2 code,
and there are no existing users (in runc, at least).
Make them return an error in such case.
Also, remove the cgroupv2-specific handling from
findCgroupMountpointAndRootFromReader().
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This function should not really be used for cgroupv2 code.
Currently it is used in kubernetes code, so we can't remove
the v2 case yet.
Add a TODO item to remove v2 code once kubernetes is converted
to not use it, and separate out v1 code.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This function is not used and were never used in any cgroupv2 code.
To have it stay that way, let it return error in case it's called
for v2.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This fixes a few cases of accessing m.paths map directly without holding
the mutex lock.
Fixes: 9087f2e82
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Since commit 714c91e9f7, method GetPaths() should only be used
for saving container state. For other uses, we have a new method,
Path(), which is cleaner.
Fix GetPaths() usage introduced by recent commits 859a780d6f and 9087f2e82.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
systemd drivers ignore --cpu-quota during update if the CPU
period was not set earlier.
Fixed by adding the default for the period.
The test will be added by the following commit.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
This (and the converting function) is only used by one of the four
cgroup drivers. The other three do some checking and conversion in
place, so let the fs2 do the same.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The code that adds CpuQuotaPerSecUSec is the same in v1 and v2
systemd cgroup driver. Move it to common.
No functional change.
Note that the comment telling that we always set this property
contradicts with the current code, and therefore it is removed.
[v2: drop cgroupv1-specific comment]
[v3: drop returning error as it's not used]
[v4: remove an obsoleted comment]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Use r instead of c.Resources for readability. No functional change.
This commit has been brought to you by '<,'>s/c\.Resources\./r./g
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When we use cgroup with systemd driver, the cgroup path will be auto removed
by systemd when all processes exited. So we should check cgroup path exists
when we access the cgroup path, for example in `kill/ps`, or else we will
got an error.
Signed-off-by: lifubang <lifubang@acmcoder.com>
This is a regression from commit 1d4ccc8e0. We only need to enable
kernel memory accounting once, from the (*legacyManager*).Apply(),
and there is no need to do it in (*legacyManager*).Set().
While at it, rename the method to better reflect what it's doing.
This saves 1 call to mountinfo parser.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Commit 4e65e0e90a 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>
1. In cases there are no sub-cgroups, a single rmdir should be faster
than iterating through the list of files.
2. Use unix.Rmdir() to save one more syscall since os.Remove() tries
unlink(2) first which fails on a directory, and only then tries
rmdir(2).
3. Re-use rmdir.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Akihiro Suda
Mrunal Patel
Kir Kolyshkin
Aleksa Sarai
Giuseppe Scrivano
Peter Hunt
Katarzyna Kujawa
lifubang