cgroupfs: drop registry entries on filesystem release#13215
Open
ibondarenko1 wants to merge 1 commit into
Open
cgroupfs: drop registry entries on filesystem release#13215ibondarenko1 wants to merge 1 commit into
ibondarenko1 wants to merge 1 commit into
Conversation
Commit 26ef517 added CgroupRegistry.RemoveCgroup and called it from dir.RmDir so that destroyed cgroup directories do not stay in kernel.CgroupRegistry.cgroups across save/restore. That fix covers rmdir but two release paths still leak: 1. GetFilesystem error window. newCgroupInode at base.go:190 calls r.AddCgroup for the root cgroup before prepareInitialCgroup and r.Register run. On either failure the code calls rootD.DecRef and fs.VFSFilesystem().DecRef. Release then runs but currently skips ReleaseCgroupHierarchy and Unregister because fs.hierarchyID is still InvalidCgroupHierarchyID. The root cgroup id stays in the registry forever. Repeated mount-failures accumulate unbounded entries. 2. prepareInitialCgroup creates intermediate cgroup directories via newDirWithOwner, each of which calls AddCgroup. If prepareInitialCgroup fails partway, the same residue applies. Walk fs.root in Release and call RemoveCgroup for each cgroupInode in the subtree. RemoveCgroup is documented as a no-op for ids not in the map (cgroup.go:556-558), so cgroups that RmDir already removed cost nothing and the rmdir path stays the primary owner. Signed-off-by: Ievgen Bondarenko <sactransport2000@gmail.com>
Collaborator
|
@xiangbin-hu could you help review this, as this is supposed to be a follow-up from your commit in #12909 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to commit
26ef5174081be0b4b1f750a97e75ee6bad5d5a53(cgroupfs: remove cgroup from CgroupRegistry on rmdir). Walk the cgroupfs dentry subtree in(*filesystem).Releaseand callCgroupRegistry.RemoveCgroupfor everycgroupInode.idin the tree.Why
The rmdir fix covers cgroups removed via
rmdir(2). Two release paths still leak entries:FilesystemType.GetFilesystemerror window atcgroupfs.go:376-398.newCgroupInodeatbase.go:190callsr.AddCgroupfor the root cgroup beforeprepareInitialCgroupandr.Registerrun. On either failure the code doesrootD.DecRef(ctx); fs.VFSFilesystem().DecRef(ctx).Releasethen runs but currently skipsReleaseCgroupHierarchyandUnregisterbecausefs.hierarchyIDis stillInvalidCgroupHierarchyID. The root cgroup id stays inCgroupRegistry.cgroupsforever. Repeated mount-failures accumulate unbounded entries that get serialized on every checkpoint, same class as the rmdir leak.prepareInitialCgroupatcgroupfs.go:440-447creates intermediate cgroup directories vianewDirWithOwner, each of which callsAddCgroup. If the path walk later fails, the partially created children leak.Approach
Add a helper
removeCgroupsFromRegistryLockedthat walksfs.rootvia the existingdir.forEachChildDirand callsRemoveCgroupfor eachcgroupInode.id.RemoveCgroupis documented as a no-op for ids that are not present (cgroup.go:556-558), so cgroups already removed byRmDircost nothing. RmDir stays the primary owner of the live-cgroup case; this walk is the safety net for the release-time residue.Test
bazel test //pkg/sentry/fsimpl/cgroupfs/... //test/syscalls:cgroup_testCLA
Signed via individual Google CLA on
sactransport2000@gmail.com.