github/containerd
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

core/mount: use ptrace instead of go:linkname

Browse Source 
The Go runtime has started to [lock down future uses of linkname][1] since
go1.23. In the go source code, containerd project has been marked in the
comment, [hall of shame][2]. Well, the go:linkname is used to fork no-op
subprocess efficiently. However, since that comment, I would like to use
ptrace and remove go:linkname in the whole repository.

With go1.22 `go:linkname`:

```bash
$ go test -bench=.  -benchmem ./ -exec sudo
goos: linux
goarch: amd64
pkg: github.com/containerd/containerd/v2/core/mount
cpu: AMD Ryzen 7 5800H with Radeon Graphics
BenchmarkBatchRunGetUsernsFD_Concurrent1-16                 2440            533320 ns/op            1145 B/op         43 allocs/op
BenchmarkBatchRunGetUsernsFD_Concurrent10-16                 342           3661616 ns/op           11562 B/op        421 allocs/op
PASS
ok      github.com/containerd/containerd/v2/core/mount  2.983s
```

With go1.22 `ptrace`:

```bash
$ go test -bench=.  -benchmem ./ -exec sudo
goos: linux
goarch: amd64
pkg: github.com/containerd/containerd/v2/core/mount
cpu: AMD Ryzen 7 5800H with Radeon Graphics
BenchmarkBatchRunGetUsernsFD_Concurrent1-16                 1785            739557 ns/op            3948 B/op         68 allocs/op
BenchmarkBatchRunGetUsernsFD_Concurrent10-16                 328           4024300 ns/op           39601 B/op        671 allocs/op
PASS
ok      github.com/containerd/containerd/v2/core/mount  3.104s
```

With go1.23 `ptrace`:

```bash
$ go test -bench=.  -benchmem ./ -exec sudo
goos: linux
goarch: amd64
pkg: github.com/containerd/containerd/v2/core/mount
cpu: AMD Ryzen 7 5800H with Radeon Graphics
BenchmarkBatchRunGetUsernsFD_Concurrent1-16                 1815            723252 ns/op            4220 B/op         69 allocs/op
BenchmarkBatchRunGetUsernsFD_Concurrent10-16                 319           3957157 ns/op           42351 B/op        682 allocs/op
PASS
ok      github.com/containerd/containerd/v2/core/mount  3.051s
```

Diff:

The `ptrace` is slower than `go:linkname` mode. However, it's accepctable.

```
goos: linux
goarch: amd64
pkg: github.com/containerd/containerd/v2/core/mount
cpu: AMD Ryzen 7 5800H with Radeon Graphics
                                    │ go122-golinkname │             go122-ptrace              │             go123-ptrace              │
                                    │      sec/op      │    sec/op     vs base                 │    sec/op     vs base                 │
BatchRunGetUsernsFD_Concurrent1-16        533.3µ ± ∞ ¹   739.6µ ± ∞ ¹        ~ (p=1.000 n=1) ²   723.3µ ± ∞ ¹        ~ (p=1.000 n=1) ²
BatchRunGetUsernsFD_Concurrent10-16       3.662m ± ∞ ¹   4.024m ± ∞ ¹        ~ (p=1.000 n=1) ²   3.957m ± ∞ ¹        ~ (p=1.000 n=1) ²
geomean                                   1.397m         1.725m        +23.45%                   1.692m        +21.06%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                                    │ go122-golinkname │              go122-ptrace               │              go123-ptrace               │
                                    │       B/op       │     B/op       vs base                  │     B/op       vs base                  │
BatchRunGetUsernsFD_Concurrent1-16       1.118Ki ± ∞ ¹   3.855Ki ± ∞ ¹         ~ (p=1.000 n=1) ²   4.121Ki ± ∞ ¹         ~ (p=1.000 n=1) ²
BatchRunGetUsernsFD_Concurrent10-16      11.29Ki ± ∞ ¹   38.67Ki ± ∞ ¹         ~ (p=1.000 n=1) ²   41.36Ki ± ∞ ¹         ~ (p=1.000 n=1) ²
geomean                                  3.553Ki         12.21Ki        +243.65%                   13.06Ki        +267.43%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

                                    │ go122-golinkname │             go122-ptrace             │             go123-ptrace             │
                                    │    allocs/op     │  allocs/op   vs base                 │  allocs/op   vs base                 │
BatchRunGetUsernsFD_Concurrent1-16         43.00 ± ∞ ¹   68.00 ± ∞ ¹        ~ (p=1.000 n=1) ²   69.00 ± ∞ ¹        ~ (p=1.000 n=1) ²
BatchRunGetUsernsFD_Concurrent10-16        421.0 ± ∞ ¹   671.0 ± ∞ ¹        ~ (p=1.000 n=1) ²   682.0 ± ∞ ¹        ~ (p=1.000 n=1) ²
geomean                                    134.5         213.6        +58.76%                   216.9        +61.23%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05
```

[1]: <https://github.com/golang/go/issues/67401>
[2]: <https://github.com/golang/go/blob/release-branch.go1.23/src/runtime/proc.go#L4820>

Signed-off-by: Wei Fu <fuweid89@gmail.com>
This commit is contained in:
Wei Fu
2024-08-19 19:05:24 +08:00
parent bcdf507363
commit 3cd8f9734d
7 changed files with 280 additions and 332 deletions
Download Patch File Download Diff File Expand all files Collapse all files

125
core/mount/mount_idmapped_linux_test.go
View File

@@ -27,7 +27,6 @@ import (
kernel "github.com/containerd/containerd/v2/pkg/kernelversion"
"github.com/containerd/continuity/testutil"
"github.com/stretchr/testify/require"
"golang.org/x/sys/unix"
)
func BenchmarkBatchRunGetUsernsFD_Concurrent1(b *testing.B) {
@@ -72,7 +71,7 @@ var (
}
)
func TestGetUsernsFD(t *testing.T) {
func TestIdmappedMount(t *testing.T) {
testutil.RequiresRoot(t)
k512 := kernel.KernelVersion{Kernel: 5, Major: 12}
@@ -82,15 +81,12 @@ func TestGetUsernsFD(t *testing.T) {
t.Skip("GetUsernsFD requires kernel >= 5.12")
}
t.Run("basic", testGetUsernsFDBasic)
t.Run("when kill child process before write u[g]id maps", testGetUsernsFDKillChildWhenWriteUGIDMaps)
t.Run("when kill child process after open u[g]id_map file", testGetUsernsFDKillChildAfterOpenUGIDMapFiles)
t.Run("GetUsernsFD", testGetUsernsFD)
t.Run("IDMapMount", testIDMapMount)
}
func testGetUsernsFDBasic(t *testing.T) {
func testGetUsernsFD(t *testing.T) {
for idx, tc := range []struct {
uidMaps string
gidMaps string
@@ -128,108 +124,21 @@ func testGetUsernsFDBasic(t *testing.T) {
}
}
func testGetUsernsFDKillChildWhenWriteUGIDMaps(t *testing.T) {
hookFunc := func(reap bool) func(uintptr, *os.File) {
return func(_pid uintptr, pidFD *os.File) {
err := unix.PidfdSendSignal(int(pidFD.Fd()), unix.SIGKILL, nil, 0)
require.NoError(t, err)
func testIDMapMount(t *testing.T) {
usernsFD, err := getUsernsFD(testUIDMaps, testGIDMaps)
require.NoError(t, err)
defer usernsFD.Close()
if reap {
pidfdWaitid(pidFD)
}
}
}
srcDir, checkFunc := initIDMappedChecker(t, testUIDMaps, testGIDMaps)
destDir := t.TempDir()
defer func() {
require.NoError(t, UnmountAll(destDir, 0))
}()
for _, tcReap := range []bool{true, false} {
t.Run(fmt.Sprintf("#reap=%v", tcReap), func(t *testing.T) {
updateTestHookKillForGetUsernsFD(t, nil, hookFunc(tcReap))
usernsFD, err := getUsernsFD(testUIDMaps, testGIDMaps)
require.NoError(t, err)
defer usernsFD.Close()
srcDir, checkFunc := initIDMappedChecker(t, testUIDMaps, testGIDMaps)
destDir := t.TempDir()
defer func() {
require.NoError(t, UnmountAll(destDir, 0))
}()
err = IDMapMount(srcDir, destDir, int(usernsFD.Fd()))
usernsFD.Close()
require.NoError(t, err)
checkFunc(destDir)
})
}
}
func testGetUsernsFDKillChildAfterOpenUGIDMapFiles(t *testing.T) {
hookFunc := func(reap bool) func(uintptr, *os.File) {
return func(_pid uintptr, pidFD *os.File) {
err := unix.PidfdSendSignal(int(pidFD.Fd()), unix.SIGKILL, nil, 0)
require.NoError(t, err)
if reap {
pidfdWaitid(pidFD)
}
}
}
for _, tc := range []struct {
reap bool
expected error
}{
{
reap: false,
expected: nil,
},
{
reap: true,
expected: syscall.ESRCH,
},
} {
t.Run(fmt.Sprintf("#reap=%v", tc.reap), func(t *testing.T) {
updateTestHookKillForGetUsernsFD(t, hookFunc(tc.reap), nil)
usernsFD, err := getUsernsFD(testUIDMaps, testGIDMaps)
if tc.expected != nil {
require.Error(t, tc.expected, err)
return
}
require.NoError(t, err)
defer usernsFD.Close()
srcDir, checkFunc := initIDMappedChecker(t, testUIDMaps, testGIDMaps)
destDir := t.TempDir()
defer func() {
require.NoError(t, UnmountAll(destDir, 0))
}()
err = IDMapMount(srcDir, destDir, int(usernsFD.Fd()))
usernsFD.Close()
require.NoError(t, err)
checkFunc(destDir)
})
}
}
func updateTestHookKillForGetUsernsFD(t *testing.T, newBeforeFunc, newAfterFunc func(uintptr, *os.File)) {
testHookLock.Lock()
oldBefore := testHookKillChildBeforePidfdSendSignal
oldAfter := testHookKillChildAfterPidfdSendSignal
t.Cleanup(func() {
testHookKillChildBeforePidfdSendSignal = oldBefore
testHookKillChildAfterPidfdSendSignal = oldAfter
testHookLock.Unlock()
})
if newBeforeFunc != nil {
testHookKillChildBeforePidfdSendSignal = newBeforeFunc
}
if newAfterFunc != nil {
testHookKillChildAfterPidfdSendSignal = newAfterFunc
}
err = IDMapMount(srcDir, destDir, int(usernsFD.Fd()))
usernsFD.Close()
require.NoError(t, err)
checkFunc(destDir)
}
func initIDMappedChecker(t *testing.T, uidMaps, gidMaps []syscall.SysProcIDMap) (_srcDir string, _verifyFunc func(destDir string)) {