imagecache: skip /proc/self/mountinfo scan via STATX_MNT_ID fast path by besher-massri · Pull Request #2963 · agent-substrate/substrate

M.Besher Massri (besher-massri) · 2026-10-20T15:00:18Z

Why is this needed?

When a worker pod hosts many concurrent sandboxes, each sandbox adds multiple mount points (rootfs overlays, image volumes, and shared directories) to the worker mount namespace, causing /proc/self/mountinfo to grow to thousands of lines (each containing long overlay lowerdir chains). Calling imagecache.UnmountAllUnder on every checkpoint and restore previously opened and scanned the entire /proc/self/mountinfo table, so unmount CPU cost scaled linearly with the total number of active mounts on the worker rather than with the single actor being torn down.

On Linux 5.8+, unix.Statx sets unix.STATX_ATTR_MOUNT_ROOT in sx.Attributes whenever a path sits at the root of a mount point without reading /proc/self/mountinfo. Checking each bundle directory entry with dirfd-relative statx identifies active mounts in O(bundle entries) syscalls while skipping recursion into overlay scratch directories (upper and work) and mounted rootfs or volume directories inside each container bundle.

Changes

Replace mountPointsUnder in internal/imagecache/bundle_linux.go with mountPointsUnderStatx, walkMountsUnderStatx, and statxAt using dirfd-relative unix.Statx(AT_SYMLINK_NOFOLLOW|AT_STATX_DONT_SYNC, STATX_TYPE) and STATX_ATTR_MOUNT_ROOT to discover mount points at or below dir.
Gate bundle-specific upper, work, rootfs, and volumes pruning on curDir containing OverlaySpecFileName or config.json so container directories under bundles/ and image volume entries under volumes/ named upper, work, rootfs, or volumes are always checked.
Update internal/imagecache/README.md teardown documentation.
Add TestMountPointsUnderStatx_NonExistentAndMultiBundle, root-gated TestSetupBundleRootfs_MultiBundleUnmount and TestMountPointsUnderStatx_RegularFileBindMount, and BenchmarkUnmountAllUnder in internal/imagecache/bundle_linux_test.go comparing mountPointsUnderStatx (statx=true) versus mountPointsIn (statx=false) on the same multi-container bundle tree and 5,000-line mountinfo table.

Benchmark Results

go test -run='^$' -bench='BenchmarkUnmountAllUnder' -benchmem -count=5 ./internal/imagecache/...

goos: linux
goarch: amd64
pkg: go.ate.dev/substrate/internal/imagecache
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
BenchmarkUnmountAllUnder/statx=true-8         	    4857	    234390 ns/op	    1809 B/op	      57 allocs/op
BenchmarkUnmountAllUnder/statx=true-8         	    6357	    200626 ns/op	    1810 B/op	      57 allocs/op
BenchmarkUnmountAllUnder/statx=true-8         	    7375	    182364 ns/op	    1807 B/op	      57 allocs/op
BenchmarkUnmountAllUnder/statx=true-8         	    5949	    170113 ns/op	    1810 B/op	      57 allocs/op
BenchmarkUnmountAllUnder/statx=true-8         	    7312	    176650 ns/op	    1813 B/op	      57 allocs/op
BenchmarkUnmountAllUnder/statx=false-8        	     121	  10070272 ns/op	    4264 B/op	       4 allocs/op
BenchmarkUnmountAllUnder/statx=false-8        	     100	  10469967 ns/op	    4264 B/op	       4 allocs/op
BenchmarkUnmountAllUnder/statx=false-8        	      93	  14161822 ns/op	    4264 B/op	       4 allocs/op
BenchmarkUnmountAllUnder/statx=false-8        	      86	  13043353 ns/op	    4264 B/op	       4 allocs/op
BenchmarkUnmountAllUnder/statx=false-8        	      86	  12464647 ns/op	    4264 B/op	       4 allocs/op
PASS
ok  	go.ate.dev/substrate/internal/imagecache	13.257s

UnmountAllUnder Discovery Mode (Multi-Container Bundle, 5,000 Mounts on Worker)	Median Latency (5 runs)	Memory Allocated	Allocations	Improvement
statx=false (legacy /proc/self/mountinfo scan)	12,464,647 ns/op (12.46 ms)	4,264 B/op (4.26 KB)	4 allocs/op	1.0x baseline
statx=true (STATX_ATTR_MOUNT_ROOT fast path)	182,364 ns/op (182.4 us)	1,810 B/op (1.81 KB)	57 allocs/op	68.4x faster, 57.6% less memory

AI use: AI-assisted, I reviewed and tested the changes.

Tests pass
Appropriate changes to documentation are included in the PR

Parnika Mandewalkar (pmandewalkar) · 2026-10-20T18:07:48Z

+1, wanted to link the issue I opened which has my test data #2950

Benjamin Elder (BenTheElder) · 2026-10-20T18:56:06Z

+		if name == "upper" || name == "work" {
+			continue
+		}
+		sx, err := statxAt(dirfd, name)
+		if err != nil {
+			if errors.Is(err, os.ErrNotExist) || errors.Is(err, unix.ENOENT) {
+				continue
+			}
+			_ = f.Close()
+			return err
+		}
+		if sx.Mode&unix.S_IFMT == unix.S_IFLNK {
+			continue
+		}
+		if isMountRoot(sx, dirMntID) {
+			*points = append(*points, filepath.Join(curDir, name))
+			// SetupBundleRootfs mounts <bundle>/rootfs and
+			// <bundle>/volumes/<name>; neither contains nested submounts,
+			// so never descend into a mounted directory.
+			continue
+		}
+		// Never descend into an unmounted rootfs or volume directory.
+		isDir := sx.Mode&unix.S_IFMT == unix.S_IFDIR
+		if !isDir || name == "rootfs" || inVolumesDir {
+			continue
+		}
+		subdirs = append(subdirs, subdir{
+			path:  filepath.Join(curDir, name),
+			mntID: sx.Mnt_id,
+			inVol: name == "volumes",
+		})


🤖 should-fix 🟡 – These name checks run at every directory level rather than only inside a container bundle directory. Calling UnmountAllUnder(actorDirs.GetOciBundleDir()) starts at <actor>/bundles, where subdirectories are container names (k8s-short-name), and image volumes live at <bundle>/volumes/<name>. A container named upper, work, rootfs, or volumes, or an image volume named upper or work, will be skipped before its mounts are reached, leaving mounts attached in the worker namespace.

Gate the upper/work skip, the unmounted rootfs skip, and inVol: name == "volumes" on curDir being a bundle directory (for example when names contains OverlaySpecFileName or "config.json"), so container directories under bundles/ and volume entries under volumes/ are always checked with statxAt and isMountRoot.

Benjamin Elder (BenTheElder) · 2026-10-20T18:56:06Z

+var (
+	statxUnmount  = true
+	mountinfoPath = "/proc/self/mountinfo"
+)


🤖 nit 🟢 – AGENTS.md asks not to add package globals just so tests can swap behavior. TestMountPointsUnderStatx_NonExistentAndMultiBundle already exercises the fallback against /proc/self/mountinfo on Linux without overriding mountinfoPath, and BenchmarkUnmountAllUnder (which has no active mounts to detach) can call mountPointsUnderStatx and mountPointsIn directly without statxUnmount or mountinfoPath.

Benjamin Elder (BenTheElder)

so some of the comments in imagecache about kernel versions are wrong ... filing a fix for that to avoid wasting time next time. couple vetted comments.

Benjamin Elder (BenTheElder) · 2026-10-21T11:56:30Z

-	points, err := mountPointsUnder(dir)
+	points, err := mountPointsUnderStatx(dir)
+	if err != nil {
+		points, err = mountPointsUnder(dir)


🤖 should-fix 🟡 – Every kernel that can mount the rootfs here reports both statx fields (see the thread below), so this fallback only runs when the walk hits an error. It does that silently, pays the mountinfo scan this PR is removing, and loses the statx error if mountinfo also fails.

I think we should just return the walk error, then mountPointsUnder and the fallback test can go. If we keep the fallback, it should at least log the statx error.

Removed the fallback

Benjamin Elder (BenTheElder) · 2026-10-21T11:56:30Z

+		return sx, err
+	}
+	if sx.Mask&unix.STATX_MNT_ID == 0 {
+		return sx, unix.ENOSYS


🤖 should-fix 🟡 – statx has set STATX_MNT_ID and STATX_ATTR_MOUNT_ROOT since Linux 5.8:
https://github.com/torvalds/linux/blob/v5.8/fs/stat.c#L207-L211

The overlay mount already needs a newer kernel than that, since it appends each layer with lowerdir+:

substrate/internal/imagecache/bundle_linux.go

Line 175 in 4d98981

if err := set("lowerdir+", lower); err != nil {

which landed in 6.7, and in 6.6.23 on the 6.6 stable branch:
https://github.com/torvalds/linux/blob/v6.7/fs/overlayfs/params.c#L148
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/overlayfs/params.c?h=v6.6.23#n148

So this ENOSYS check, the Mnt_id comparison in isMountRoot and the parent statxAt in mountPointsUnderStatx can't be reached. Let's drop them and check STATX_ATTR_MOUNT_ROOT directly, then the mntID plumbing through the walk can go too.

Benjamin Elder (BenTheElder) · 2026-10-21T11:56:30Z

+	}
+}
+
+func TestUnmountAllUnder_DetachesRegularFileBindMount(t *testing.T) {


🤖 nit 🟢 – This mostly duplicates TestResetRunscStateAndPidFileDirs_DetachesMounts, which already detaches a null-netns file bind through UnmountAllUnder:

substrate/cmd/ateworker-gvisor/actorpath_test.go

Lines 47 to 71 in 1872468

func TestResetRunscStateAndPidFileDirs_DetachesMounts(t *testing.T) {

roottest.Require(t, "mount/unmount")

actorDirs := &ateworkerpb.ActorDirs{RootDir: t.TempDir()}

if err := os.MkdirAll(runscStateDir(actorDirs), 0o700); err != nil {

t.Fatal(err)

}

src := filepath.Join(t.TempDir(), "netns")

target := filepath.Join(runscStateDir(actorDirs), "null-netns")

for _, f := range []string{src, target} {

if err := os.WriteFile(f, nil, 0o600); err != nil {

t.Fatal(err)

}

}

if err := unix.Mount(src, target, "", unix.MS_BIND, ""); err != nil {

t.Fatalf("bind mount: %v", err)

}

t.Cleanup(func() { _ = unix.Unmount(target, unix.MNT_DETACH) })

if err := resetRunscStateAndPidFileDirs(actorDirs); err != nil {

t.Fatalf("resetRunscStateAndPidFileDirs() = %v", err)

}

if entries, err := os.ReadDir(runscStateDir(actorDirs)); err != nil || len(entries) != 0 {

t.Errorf("ReadDir(%q) = %v, %v; want an empty directory", runscStateDir(actorDirs), entries, err)

}

}

Can we drop it, or keep only the mountPointsUnderStatx(target) case?

Done. Went with trimming it to only mountPointsUnderStatx

…gent-substrate#3128) follow-up to agent-substrate#2963 (review) The imagecache comment and README say the minimum kernel is 6.5, but overlayfs's `lowerdir+` landed in 6.7 (24e16e385f22, "ovl: add support for appending lowerdirs one by one"), and was backported to 6.6.y in v6.6.23: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/overlayfs/params.c?h=v6.7#n148 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/fs/overlayfs/params.c?h=v6.6.23#n148 6.5 had a different append syntax, `lowerdir=:dir`, but it was disabled before 6.6 shipped (beae836e9c61, "ovl: temporarily disable appending lowedirs") and we don't use it: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/overlayfs/params.c?h=v6.6#n375 AI use: AI-assisted, I reviewed it. - [x] Tests pass - [x] Docs updated

Discover mount points in UnmountAllUnder using dirfd-relative statx with AT_SYMLINK_NOFOLLOW, AT_STATX_DONT_SYNC, and STATX_ATTR_MOUNT_ROOT instead of scanning /proc/self/mountinfo. Gate bundle-specific upper, work, rootfs, and volumes pruning on curDir containing OverlaySpecFileName or config.json so container directories and image volumes with those names are still checked.

Vicente Ferrara (vicentefb) · 2026-11-12T19:10:38Z

is this still active ?

I ran into this at fleet scale, and I have before/after numbers in case they help with prioritizing it.

Setup

351 GKE nodes: 300 × n2-standard-16 and 51 × c3-standard-88
ateworker-gvisor v1.0.0
221K suspended sandboxes across 36,840 images, running wake + suspend cycles

One difference from #2950: on my nodes, the mounts don't come from the sandboxes. The worker pod mounts /var/lib/ate with HostToContainer propagation, so its mount namespace has one mount per cached image layer. That's 1,000–3,300 mounts per node, even on an idle node.

Reading mountinfo gets slow as mounts grow. Time for one read of /proc/<worker>/mountinfo on an idle node:

Mounts	Read time
~1,000	17–23 ms
1,533	108 ms
2,169	305 ms
2,815	331 ms

The worker reads this file 5 times per wake + suspend: 2 times before restore and 3 times during teardown.

The bigger problem is queueing. Teardown runs while the checkpoint is holding one of the 2 checkpointWriteSlots. On a loaded node, a 1.5 s teardown turned the checkpoint step into 5–6 s of waiting for a slot.

Results with a fix. I tested a local change that uses the same approach as this PR (a statx mount-root walk). It isn't the PR itself. Two neighbouring nodes in the same run, v1.0.0 vs. patched, p50:

	v1.0.0	Patched
Restore call	2.19 s	0.24 s
Checkpoint call	9.16 s	0.14 s

Across the fleet, the best 5 minutes went from 2,775 to 3,681 starts/s.

Parnika Mandewalkar (pmandewalkar) · 2026-11-12T19:25:36Z

+1, would love for this to be active again as the numbers in #2950 are still relevant for this

Benjamin Elder (BenTheElder) self-requested a review October 20, 2026 15:14

M.Besher Massri (besher-massri) mentioned this pull request Oct 20, 2026

ateworker-microvm: avoid /proc and mountinfo scans in sandbox cleanup #2965

Open

2 tasks done

Parnika Mandewalkar (pmandewalkar) mentioned this pull request Oct 20, 2026

UnmountAllUnder reads all of /proc/self/mountinfo on every call, which contends with mounts on dense gVisor nodes #2950

Open

Benjamin Elder (BenTheElder) reviewed Oct 21, 2026

View reviewed changes

M.Besher Massri (besher-massri) force-pushed the prod/slim-u3a-imagecache-statx-unmount branch 2 times, most recently from 9e18f99 to 1872468 Compare October 21, 2026 01:34

Benjamin Elder (BenTheElder) reviewed Oct 22, 2026

View reviewed changes

Benjamin Elder (BenTheElder) mentioned this pull request Oct 22, 2026

correct the minimum kernel for the image cache: Linux 6.7, or 6.6.23 #3128

Merged

2 tasks done

M.Besher Massri (besher-massri) force-pushed the prod/slim-u3a-imagecache-statx-unmount branch from 1872468 to c12486d Compare October 23, 2026 18:07

Bowei Du (bowei) added area/node area/storage kind/feature An enhancement / feature request or implementation labels Oct 27, 2026

Max Smythe (maxsmythe) added this to the v1.1 milestone Nov 11, 2026

	func TestResetRunscStateAndPidFileDirs_DetachesMounts(t *testing.T) {
	roottest.Require(t, "mount/unmount")
	actorDirs := &ateworkerpb.ActorDirs{RootDir: t.TempDir()}
	if err := os.MkdirAll(runscStateDir(actorDirs), 0o700); err != nil {
	t.Fatal(err)
	}
	src := filepath.Join(t.TempDir(), "netns")
	target := filepath.Join(runscStateDir(actorDirs), "null-netns")
	for _, f := range []string{src, target} {
	if err := os.WriteFile(f, nil, 0o600); err != nil {
	t.Fatal(err)
	}
	}
	if err := unix.Mount(src, target, "", unix.MS_BIND, ""); err != nil {
	t.Fatalf("bind mount: %v", err)
	}
	t.Cleanup(func() { _ = unix.Unmount(target, unix.MNT_DETACH) })

	if err := resetRunscStateAndPidFileDirs(actorDirs); err != nil {
	t.Fatalf("resetRunscStateAndPidFileDirs() = %v", err)
	}
	if entries, err := os.ReadDir(runscStateDir(actorDirs)); err != nil \|\| len(entries) != 0 {
	t.Errorf("ReadDir(%q) = %v, %v; want an empty directory", runscStateDir(actorDirs), entries, err)
	}
	}

Conversation

M.Besher Massri (besher-massri) commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why is this needed?

Changes

Benchmark Results

Uh oh!

Parnika Mandewalkar (pmandewalkar) commented Oct 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vicente Ferrara (vicentefb) commented Nov 12, 2026

Uh oh!

Parnika Mandewalkar (pmandewalkar) commented Nov 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

M.Besher Massri (besher-massri) commented Oct 20, 2026 •

edited

Loading

Parnika Mandewalkar (pmandewalkar) commented Nov 12, 2026 •

edited

Loading