From cd479f9d146e305088772b5fdc8e39bb6bbba8cf Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 3 Jun 2020 09:07:38 -0700 Subject: [PATCH 1/2] cgroupv1/freezer: don't use subsystemSet.Get() 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 --- libcontainer/cgroups/fs/fs.go | 12 ++++-------- libcontainer/cgroups/systemd/v1.go | 12 +++--------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go index 245ce84f..ae7ada6b 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -312,10 +312,7 @@ func (m *manager) Freeze(state configs.FreezerState) (Err error) { } }() - freezer, err := m.getSubsystems().Get("freezer") - if err != nil { - return err - } + freezer := &FreezerGroup{} if err := freezer.Set(path, m.cgroups); err != nil { return err } @@ -418,13 +415,12 @@ func (m *manager) GetCgroups() (*configs.Cgroup, error) { func (m *manager) GetFreezerState() (configs.FreezerState, error) { dir := m.Path("freezer") - freezer, err := m.getSubsystems().Get("freezer") - // If the container doesn't have the freezer cgroup, say it's undefined. - if err != nil || dir == "" { + if dir == "" { return configs.Undefined, nil } - return freezer.(*FreezerGroup).GetState(dir) + freezer := &FreezerGroup{} + return freezer.GetState(dir) } func (m *manager) Exists() bool { diff --git a/libcontainer/cgroups/systemd/v1.go b/libcontainer/cgroups/systemd/v1.go index 9e8e2fdd..a832fd4e 100644 --- a/libcontainer/cgroups/systemd/v1.go +++ b/libcontainer/cgroups/systemd/v1.go @@ -319,10 +319,7 @@ func (m *legacyManager) Freeze(state configs.FreezerState) error { } prevState := m.cgroups.Resources.Freezer m.cgroups.Resources.Freezer = state - freezer, err := legacySubsystems.Get("freezer") - if err != nil { - return err - } + freezer := &fs.FreezerGroup{} err = freezer.Set(path, m.cgroups) if err != nil { m.cgroups.Resources.Freezer = prevState @@ -458,11 +455,8 @@ func (m *legacyManager) GetFreezerState() (configs.FreezerState, error) { if err != nil && !cgroups.IsNotFound(err) { return configs.Undefined, err } - freezer, err := legacySubsystems.Get("freezer") - if err != nil { - return configs.Undefined, err - } - return freezer.(*fs.FreezerGroup).GetState(path) + freezer := &fs.FreezerGroup{} + return freezer.GetState(path) } func (m *legacyManager) Exists() bool { From 46b26bc05d4e5db6785b0dee4ea86544cbc4eb85 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 3 Jun 2020 12:01:14 -0700 Subject: [PATCH 2/2] cgroups/fs/Freeze: simplify 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 --- libcontainer/cgroups/fs/fs.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go index ae7ada6b..fdae531f 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -298,7 +298,7 @@ func (m *manager) Set(container *configs.Config) error { // Freeze toggles the container's freezer cgroup depending on the state // provided -func (m *manager) Freeze(state configs.FreezerState) (Err error) { +func (m *manager) Freeze(state configs.FreezerState) error { path := m.Path("freezer") if m.cgroups == nil || path == "" { return errors.New("cannot toggle freezer: cgroups not configured for container") @@ -306,14 +306,9 @@ func (m *manager) Freeze(state configs.FreezerState) (Err error) { prevState := m.cgroups.Resources.Freezer m.cgroups.Resources.Freezer = state - defer func() { - if Err != nil { - m.cgroups.Resources.Freezer = prevState - } - }() - freezer := &FreezerGroup{} if err := freezer.Set(path, m.cgroups); err != nil { + m.cgroups.Resources.Freezer = prevState return err } return nil