From 2374178c9b6b8d596e4cd054892d5760d1a3aaff Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 7 Dec 2020 11:37:29 +0100 Subject: [PATCH] pkg/cri/server: optimizations in unmountRecursive() Use a PrefixFilter() to get only the mounts we're interested in, which removes the need to manually filter mounts from the mountinfo results. Additional optimizations can be made, as: > ... there's a little known fact that `umount(MNT_DETACH)` is actually > recursive in Linux, IOW this function can be replaced with > `unix.Umount(target, unix.MNT_DETACH)` (or `mount.UnmountAll(target, unix.MNT_DETACH)` > (provided that target itself is a mount point). https://github.com/containerd/containerd/pull/4578/commits/e8fb2c392f52ca8084fc88181599bd6c30650bba#r535450446 Signed-off-by: Sebastiaan van Stijn --- pkg/cri/server/helpers_linux.go | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/pkg/cri/server/helpers_linux.go b/pkg/cri/server/helpers_linux.go index 4b70aa7b8..c5ce1164f 100644 --- a/pkg/cri/server/helpers_linux.go +++ b/pkg/cri/server/helpers_linux.go @@ -166,34 +166,23 @@ func openLogFile(path string) (*os.File, error) { // unmountRecursive unmounts the target and all mounts underneath, starting with // the deepest mount first. func unmountRecursive(ctx context.Context, target string) error { - mounts, err := mountinfo.GetMounts(nil) + toUnmount, err := mountinfo.GetMounts(mountinfo.PrefixFilter(target)) if err != nil { return err } - var toUnmount []string - for _, m := range mounts { - p, err := filepath.Rel(target, m.Mountpoint) - if err != nil { - return err - } - if !strings.HasPrefix(p, "..") { - toUnmount = append(toUnmount, m.Mountpoint) - } - } - // Make the deepest mount be first sort.Slice(toUnmount, func(i, j int) bool { - return len(toUnmount[i]) > len(toUnmount[j]) + return len(toUnmount[i].Mountpoint) > len(toUnmount[j].Mountpoint) }) - for i, mountPath := range toUnmount { - if err := mount.UnmountAll(mountPath, unix.MNT_DETACH); err != nil { + for i, m := range toUnmount { + if err := mount.UnmountAll(m.Mountpoint, unix.MNT_DETACH); err != nil { if i == len(toUnmount)-1 { // last mount return err } // This is some submount, we can ignore this error for now, the final unmount will fail if this is a real problem - log.G(ctx).WithError(err).Debugf("failed to unmount submount %s", mountPath) + log.G(ctx).WithError(err).Debugf("failed to unmount submount %s", m.Mountpoint) } } return nil