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.

Sign up for GitHub

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

cgroupv1: refactor and optimize #3215

Merged
merged 8 commits into from
Sep 21, 2021
Merged

cgroupv1: refactor and optimize #3215

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dc907e8
libct/cg/sd/v*.go: nit
kolyshkin Aug 6, 2021
c7e0864
libct/cg/sd/v1: factor out initPaths
kolyshkin Aug 6, 2021
63c8491
libct/cg/sd/v1: optimize initPaths
kolyshkin Aug 6, 2021
eb09df7
libct/cg/sd/v1: initPaths: minor optimization
kolyshkin Aug 6, 2021
19b542a
libct/cg/fs: move internal code out of fs.go
kolyshkin Aug 10, 2021
5c7cb83
libct/cg/fs: micro optimization
kolyshkin Aug 10, 2021
7d1cb32
libct/cg/fs: rename join to apply
kolyshkin Aug 10, 2021
e692886
libct/cg/fs: refactor
kolyshkin Aug 10, 2021
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
12 changes: 4 additions & 8 deletions libcontainer/cgroups/systemd/v1.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,25 +120,21 @@ func (m *legacyManager) Apply(pid int) error {

properties = append(properties, systemdDbus.PropDescription("libcontainer container "+c.Name))

// if we create a slice, the parent is defined via a Wants=
if strings.HasSuffix(unitName, ".slice") {
// If we create a slice, the parent is defined via a Wants=.
Copy link
Member

Choose a reason for hiding this comment

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

I should probably add a nit here that If inside an if is describing the code 😇. Within an if/else branch it could be (e.g.);

Suggested change
// If we create a slice, the parent is defined via a Wants=.
// Create a slice; the parent is defined via a Wants=.

(no need to change)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're not creating a slice right here, I think the older version of the comment is more comprehensible. Or something like "For a slice, the parent is defined via Wants=", but it's good enough as it is for me.

properties = append(properties, systemdDbus.PropWants(slice))
} else {
// otherwise, we use Slice=
// Otherwise it's a scope, which we put into a Slice=.
properties = append(properties, systemdDbus.PropSlice(slice))
// Assume scopes always support delegation (supported since systemd v218).
properties = append(properties, newProp("Delegate", true))
}

// only add pid if its valid, -1 is used w/ general slice creation.
if pid != -1 {
properties = append(properties, newProp("PIDs", []uint32{uint32(pid)}))
}

// Check if we can delegate. This is only supported on systemd versions 218 and above.
if !strings.HasSuffix(unitName, ".slice") {
// Assume scopes always support delegation.
properties = append(properties, newProp("Delegate", true))
}

// Always enable accounting, this gets us the same behaviour as the fs implementation,
// plus the kernel has some problems with joining the memory cgroup at a later time.
properties = append(properties,
Expand Down
12 changes: 4 additions & 8 deletions libcontainer/cgroups/systemd/v2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,25 +242,21 @@ func (m *unifiedManager) Apply(pid int) error {

properties = append(properties, systemdDbus.PropDescription("libcontainer container "+c.Name))

// if we create a slice, the parent is defined via a Wants=
if strings.HasSuffix(unitName, ".slice") {
// If we create a slice, the parent is defined via a Wants=.
properties = append(properties, systemdDbus.PropWants(slice))
} else {
// otherwise, we use Slice=
// Otherwise it's a scope, which we put into a Slice=.
properties = append(properties, systemdDbus.PropSlice(slice))
// Assume scopes always support delegation (supported since systemd v218).
properties = append(properties, newProp("Delegate", true))
}

// only add pid if its valid, -1 is used w/ general slice creation.
if pid != -1 {
properties = append(properties, newProp("PIDs", []uint32{uint32(pid)}))
}

// Check if we can delegate. This is only supported on systemd versions 218 and above.
if !strings.HasSuffix(unitName, ".slice") {
// Assume scopes always support delegation.
properties = append(properties, newProp("Delegate", true))
}

// Always enable accounting, this gets us the same behaviour as the fs implementation,
// plus the kernel has some problems with joining the memory cgroup at a later time.
properties = append(properties,
Expand Down