Skip to content

Commit

Permalink
mount-utils: don't reread mountinfo on newer kernels
Browse files Browse the repository at this point in the history
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 <kolyshkin@gmail.com>

Kubernetes-commit: b690450e8462841a2b5ed6dc79a4cc23d548ecc2
  • Loading branch information
kolyshkin authored and k8s-publishing-bot committed May 9, 2023
1 parent 9181b6e commit 4ae857e
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 4 deletions.
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -16,7 +17,8 @@ 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
)

replace k8s.io/mount-utils => ../mount-utils
52 changes: 51 additions & 1 deletion mount_helper_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
3 changes: 1 addition & 2 deletions mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (

"k8s.io/klog/v2"
utilexec "k8s.io/utils/exec"
utilio "k8s.io/utils/io"
)

const (
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 4ae857e

Please sign in to comment.