From ab9d8455d4c8f03e1fb96b4c9917572827dafe87 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Tue, 10 Dec 2024 17:08:20 -0800 Subject: [PATCH] Re-use the same device file inode in tmpfs for overlay whiteout files. When using overlayfs with tmpfs as the upper layer (common case), depending on the application, a lot of whiteouts can be created. This leads to a lot of memory allocation because new dentry and inode structs need to be allocated for each whiteout. With this change, we at least avoid the inode allocations. This is analogous with what Linux does. See fs/overlayfs/ovl_entry.h:ovl_fs's field `whiteout` with comment "Shared whiteout cache". PiperOrigin-RevId: 704897805 --- pkg/abi/linux/fs.go | 6 +++++ pkg/sentry/fsimpl/overlay/directory.go | 2 +- pkg/sentry/fsimpl/overlay/filesystem.go | 6 +++-- pkg/sentry/fsimpl/tmpfs/device_file.go | 33 +++++++++++++++++++------ pkg/sentry/fsimpl/tmpfs/filesystem.go | 6 ++--- pkg/sentry/fsimpl/tmpfs/tmpfs.go | 8 ++++-- 6 files changed, 44 insertions(+), 17 deletions(-) diff --git a/pkg/abi/linux/fs.go b/pkg/abi/linux/fs.go index 00524d965..e1c4a13bc 100644 --- a/pkg/abi/linux/fs.go +++ b/pkg/abi/linux/fs.go @@ -121,3 +121,9 @@ const ( RENAME_EXCHANGE = (1 << 1) // Exchange src and dst. RENAME_WHITEOUT = (1 << 2) // Whiteout src. ) + +// Overlayfs constants from include/linux/fs.h. +const ( + WHITEOUT_MODE = 0 + WHITEOUT_DEV = 0 +) diff --git a/pkg/sentry/fsimpl/overlay/directory.go b/pkg/sentry/fsimpl/overlay/directory.go index 1119f1af6..c46f8f51b 100644 --- a/pkg/sentry/fsimpl/overlay/directory.go +++ b/pkg/sentry/fsimpl/overlay/directory.go @@ -83,7 +83,7 @@ func (d *dentry) collectWhiteoutsForRmdirLocked(ctx context.Context) (map[string readdirErr = err return false } - if stat.RdevMajor != 0 || stat.RdevMinor != 0 { + if linux.MakeDeviceID(uint16(stat.RdevMajor), stat.RdevMinor) != linux.WHITEOUT_DEV { // This file is a real character device, not a whiteout. readdirErr = linuxerr.ENOTEMPTY return false diff --git a/pkg/sentry/fsimpl/overlay/filesystem.go b/pkg/sentry/fsimpl/overlay/filesystem.go index 91a17a470..1d8d12ebc 100644 --- a/pkg/sentry/fsimpl/overlay/filesystem.go +++ b/pkg/sentry/fsimpl/overlay/filesystem.go @@ -574,9 +574,11 @@ func (fs *filesystem) doCreateAt(ctx context.Context, rp *vfs.ResolvingPath, ct // // Preconditions: pop's parent directory has been copied up. func CreateWhiteout(ctx context.Context, vfsObj *vfs.VirtualFilesystem, creds *auth.Credentials, pop *vfs.PathOperation) error { + major, minor := linux.DecodeDeviceID(linux.WHITEOUT_DEV) return vfsObj.MknodAt(ctx, creds, pop, &vfs.MknodOptions{ - Mode: linux.S_IFCHR, // permissions == include/linux/fs.h:WHITEOUT_MODE == 0 - // DevMajor == DevMinor == 0, from include/linux/fs.h:WHITEOUT_DEV + Mode: linux.S_IFCHR | linux.WHITEOUT_MODE, + DevMajor: uint32(major), + DevMinor: minor, }) } diff --git a/pkg/sentry/fsimpl/tmpfs/device_file.go b/pkg/sentry/fsimpl/tmpfs/device_file.go index 617bcc0ff..fd0b26ede 100644 --- a/pkg/sentry/fsimpl/tmpfs/device_file.go +++ b/pkg/sentry/fsimpl/tmpfs/device_file.go @@ -31,21 +31,38 @@ type deviceFile struct { minor uint32 } -func (fs *filesystem) newDeviceFile(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, kind vfs.DeviceKind, major, minor uint32, parentDir *directory) *inode { +func isOvlWhiteoutDev(mode linux.FileMode, major, minor uint32) bool { + return mode.FileType() == linux.S_IFCHR && + mode.Permissions() == linux.WHITEOUT_MODE && + linux.MakeDeviceID(uint16(major), minor) == linux.WHITEOUT_DEV +} + +// Precondition: fs.mu must be locked for writing. +func (fs *filesystem) newDeviceFileLocked(kuid auth.KUID, kgid auth.KGID, mode linux.FileMode, major, minor uint32, parentDir *directory) *inode { + ovlWhiteout := isOvlWhiteoutDev(mode, major, minor) + if ovlWhiteout && fs.ovlWhiteout != nil { + // If reusing the same inode, acts like a hard link. + fs.ovlWhiteout.inode.incLinksLocked() + return &fs.ovlWhiteout.inode + } file := &deviceFile{ - kind: kind, major: major, minor: minor, } - switch kind { - case vfs.BlockDevice: - mode |= linux.S_IFBLK - case vfs.CharDevice: - mode |= linux.S_IFCHR + switch mode.FileType() { + case linux.S_IFBLK: + file.kind = vfs.BlockDevice + case linux.S_IFCHR: + file.kind = vfs.CharDevice default: - panic(fmt.Sprintf("invalid DeviceKind: %v", kind)) + panic(fmt.Sprintf("invalid file type for device file: %s", mode)) } file.inode.init(file, fs, kuid, kgid, mode, parentDir) file.inode.nlink = atomicbitops.FromUint32(1) // from parent directory + if ovlWhiteout { + fs.ovlWhiteout = file + // An extra link is held by fs, so nlink doesn't fall to 0. + file.inode.incLinksLocked() + } return &file.inode } diff --git a/pkg/sentry/fsimpl/tmpfs/filesystem.go b/pkg/sentry/fsimpl/tmpfs/filesystem.go index 4d33f2ec9..a1641c265 100644 --- a/pkg/sentry/fsimpl/tmpfs/filesystem.go +++ b/pkg/sentry/fsimpl/tmpfs/filesystem.go @@ -320,10 +320,8 @@ func (fs *filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v childInode = fs.newRegularFile(creds.EffectiveKUID, creds.EffectiveKGID, opts.Mode, parentDir) case linux.S_IFIFO: childInode = fs.newNamedPipe(creds.EffectiveKUID, creds.EffectiveKGID, opts.Mode, parentDir) - case linux.S_IFBLK: - childInode = fs.newDeviceFile(creds.EffectiveKUID, creds.EffectiveKGID, opts.Mode, vfs.BlockDevice, opts.DevMajor, opts.DevMinor, parentDir) - case linux.S_IFCHR: - childInode = fs.newDeviceFile(creds.EffectiveKUID, creds.EffectiveKGID, opts.Mode, vfs.CharDevice, opts.DevMajor, opts.DevMinor, parentDir) + case linux.S_IFBLK, linux.S_IFCHR: + childInode = fs.newDeviceFileLocked(creds.EffectiveKUID, creds.EffectiveKGID, opts.Mode, opts.DevMajor, opts.DevMinor, parentDir) case linux.S_IFSOCK: childInode = fs.newSocketFile(creds.EffectiveKUID, creds.EffectiveKGID, opts.Mode, opts.Endpoint, parentDir) default: diff --git a/pkg/sentry/fsimpl/tmpfs/tmpfs.go b/pkg/sentry/fsimpl/tmpfs/tmpfs.go index 23131c983..bbe1d8709 100644 --- a/pkg/sentry/fsimpl/tmpfs/tmpfs.go +++ b/pkg/sentry/fsimpl/tmpfs/tmpfs.go @@ -105,6 +105,9 @@ type filesystem struct { // allowXattrPrefix is a set of xattr namespace prefixes that this // tmpfs mount will allow. It is immutable. allowXattrPrefix map[string]struct{} + + // ovlWhiteout is the shared overlay whiteout device. It is protected by mu. + ovlWhiteout *deviceFile } // Name implements vfs.FilesystemType.Name. @@ -328,6 +331,9 @@ func (fs *filesystem) Release(ctx context.Context) { if fs.root.inode.isDir() { fs.root.releaseChildrenLocked(ctx) } + if fs.ovlWhiteout != nil { + fs.ovlWhiteout.inode.decLinksLocked(ctx) + } fs.mu.Unlock() if fs.mf.RestoreID() != "" { // If RestoreID is set, then this is a private MemoryFile which needs to be @@ -533,7 +539,6 @@ func (i *inode) init(impl any, fs *filesystem, kuid auth.KUID, kgid auth.KGID, m // // Preconditions: // - filesystem.mu must be locked for writing. -// - i.mu must be lcoked. // - i.nlink != 0. // - i.nlink < maxLinks. func (i *inode) incLinksLocked() { @@ -551,7 +556,6 @@ func (i *inode) incLinksLocked() { // // Preconditions: // - filesystem.mu must be locked for writing. -// - i.mu must be lcoked. // - i.nlink != 0. func (i *inode) decLinksLocked(ctx context.Context) { if i.nlink.RacyLoad() == 0 {