From b690450e8462841a2b5ed6dc79a4cc23d548ecc2 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 9 May 2023 15:52:07 -0700 Subject: [PATCH] mount-utils: don't reread mountinfo on newer kernels 1. Background. Since the dawn of times mount-utils package tries to work around the bug in the Linux kernel, which results in occasional incomplete read of mountinfo entries (from either /proc/mounts or /proc/PID/mountinfo). The workaround used is to read the whole file twice and compare the two blobs. If they differ, try again. The kernel bug is manifesting when mountinfo read is performed concurrently with an unmount, and can easily be reproduced by running lots of mounts and unmounts in parallel with the code reading mountinfo. For one such reproducer, see https://github.com/kolyshkin/procfs-test. On a Kubernetes node with lots of short-lived containers, mounts and unmounts are quite frequent. This leads to the occasional bug, and surely results in much more re-reads of mountinfo, because the workaround assumes its content is more-or-less static. The good news is, this bug was finally fixed by kernel commit 9f6c61f96f2d97, which made its way into Linux 5.8. 2. The issue. The code still read every file at least twice, and up to 10 times. The chance of re-reading is higher if there is a mount or unmount going on at the same time. The result is higher system and kernel load, and degraded performance. 3. The fix. As the re-reading is not necessary for newer kernels, let's check the kernel version and skip the workaround if running Linux >= 5.8. Signed-off-by: Kir Kolyshkin --- staging/src/k8s.io/mount-utils/go.mod | 2 +- .../k8s.io/mount-utils/mount_helper_unix.go | 52 ++++++++++++++++++- staging/src/k8s.io/mount-utils/mount_linux.go | 3 +- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/staging/src/k8s.io/mount-utils/go.mod b/staging/src/k8s.io/mount-utils/go.mod index e2dec9a41ff..f3fe8a08c4c 100644 --- a/staging/src/k8s.io/mount-utils/go.mod +++ b/staging/src/k8s.io/mount-utils/go.mod @@ -7,6 +7,7 @@ go 1.20 require ( github.com/moby/sys/mountinfo v0.6.2 github.com/stretchr/testify v1.8.2 + golang.org/x/sys v0.8.0 k8s.io/klog/v2 v2.100.1 k8s.io/utils v0.0.0-20230406110748-d93618cff8a2 ) @@ -16,7 +17,6 @@ require ( github.com/go-logr/logr v1.2.4 // indirect github.com/kr/pretty v0.3.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.8.0 // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/staging/src/k8s.io/mount-utils/mount_helper_unix.go b/staging/src/k8s.io/mount-utils/mount_helper_unix.go index 7b1eb6af1b7..82210aaa23a 100644 --- a/staging/src/k8s.io/mount-utils/mount_helper_unix.go +++ b/staging/src/k8s.io/mount-utils/mount_helper_unix.go @@ -20,14 +20,17 @@ limitations under the License. package mount import ( + "bytes" "errors" "fmt" "io/fs" "os" "strconv" "strings" + "sync" "syscall" + "golang.org/x/sys/unix" "k8s.io/klog/v2" utilio "k8s.io/utils/io" ) @@ -91,7 +94,7 @@ type MountInfo struct { // nolint: golint // ParseMountInfo parses /proc/xxx/mountinfo. func ParseMountInfo(filename string) ([]MountInfo, error) { - content, err := utilio.ConsistentRead(filename, maxListTries) + content, err := readMountInfo(filename) if err != nil { return []MountInfo{}, err } @@ -198,3 +201,50 @@ func PathExists(path string) (bool, error) { } return false, err } + +// These variables are used solely by kernelHasMountinfoBug. +var ( + hasMountinfoBug bool + checkMountinfoBugOnce sync.Once +) + +// kernelHasMountinfoBug checks if the kernel bug that can lead to incomplete +// mountinfo being read is fixed. It does so by checking the kernel version. +// +// The bug was fixed by the kernel commit 9f6c61f96f2d97 (since Linux 5.8). +// Alas, there is no better way to check if the bug is fixed other than to +// rely on the kernel version returned by uname. +func kernelHasMountinfoBug() bool { + checkMountinfoBugOnce.Do(func() { + // Assume old kernel. + hasMountinfoBug = true + + uname := unix.Utsname{} + err := unix.Uname(&uname) + if err != nil { + return + } + + end := bytes.IndexByte(uname.Release[:], 0) + v := bytes.SplitN(uname.Release[:end], []byte{'.'}, 3) + if len(v) != 3 { + return + } + major, _ := strconv.Atoi(string(v[0])) + minor, _ := strconv.Atoi(string(v[1])) + + if major > 5 || (major == 5 && minor >= 8) { + hasMountinfoBug = false + } + }) + + return hasMountinfoBug +} + +func readMountInfo(path string) ([]byte, error) { + if kernelHasMountinfoBug() { + return utilio.ConsistentRead(path, maxListTries) + } + + return os.ReadFile(path) +} diff --git a/staging/src/k8s.io/mount-utils/mount_linux.go b/staging/src/k8s.io/mount-utils/mount_linux.go index 005f447d9d8..4ce73b6c598 100644 --- a/staging/src/k8s.io/mount-utils/mount_linux.go +++ b/staging/src/k8s.io/mount-utils/mount_linux.go @@ -37,7 +37,6 @@ import ( "k8s.io/klog/v2" utilexec "k8s.io/utils/exec" - utilio "k8s.io/utils/io" ) const ( @@ -633,7 +632,7 @@ func (mounter *SafeFormatAndMount) GetDiskFormat(disk string) (string, error) { // ListProcMounts is shared with NsEnterMounter func ListProcMounts(mountFilePath string) ([]MountPoint, error) { - content, err := utilio.ConsistentRead(mountFilePath, maxListTries) + content, err := readMountInfo(mountFilePath) if err != nil { return nil, err }