From 05605ec6b4290a596bff2e157870b24c9f3e515c Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Mon, 30 Jan 2023 21:01:32 -0800 Subject: [PATCH] Decommit entire MemoryFile upon destruction. MemoryFile.findReclaimable() immediately exits when MemoryFile.Destroy() is called, even if MemoryFile.reclaim set is not empty. If the MemoryFile is backed by a host filesystem based file, then just closing MemoryFile.FD() does not release resources held by the MemoryFile. To release the disk space held by the host file, it needs to be decommitted. Otherwise, we can leave around large host files. Guard this change in behavior behind a flag in MemoryFileOpts which defaults to the old behavior of not decommitting the host file. PiperOrigin-RevId: 505884271 --- pkg/sentry/pgalloc/pgalloc.go | 11 +++++++++++ runsc/boot/loader.go | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/pgalloc/pgalloc.go b/pkg/sentry/pgalloc/pgalloc.go index e8e643768..447bfd97f 100644 --- a/pkg/sentry/pgalloc/pgalloc.go +++ b/pkg/sentry/pgalloc/pgalloc.go @@ -196,6 +196,11 @@ type MemoryFileOpts struct { // no effect unless DelayedEviction is DelayedEvictionEnabled. UseHostMemcgPressure bool + // DecommitOnDestroy indicates whether the entire host file should be + // decommitted on destruction. This is appropriate for host filesystem based + // files that need to be explicitly cleaned up to release disk space. + DecommitOnDestroy bool + // If ManualZeroing is true, MemoryFile must not assume that new pages // obtained from the host are zero-filled, such that MemoryFile must manually // zero newly-allocated pages. @@ -1207,6 +1212,12 @@ func (f *MemoryFile) runReclaim() { f.mu.Unlock() panic("findReclaimable broke out of reclaim loop, but destroyed is no longer set") } + if f.opts.DecommitOnDestroy && f.fileSize > 0 { + if err := f.decommitFile(memmap.FileRange{Start: 0, End: uint64(f.fileSize)}); err != nil { + f.mu.Unlock() + panic(fmt.Sprintf("failed to decommit entire memory file during destruction: %v", err)) + } + } f.file.Close() // Ensure that any attempts to use f.file.Fd() fail instead of getting a fd // that has possibly been reassigned. diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 2f183894e..d48bcb8e0 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -275,7 +275,7 @@ func New(args Args) (*Loader, error) { } if args.OverlayFilestoreFD >= 0 { f := os.NewFile(uintptr(args.OverlayFilestoreFD), "overlay-filestore") - mf, err := pgalloc.NewMemoryFile(f, pgalloc.MemoryFileOpts{}) + mf, err := pgalloc.NewMemoryFile(f, pgalloc.MemoryFileOpts{DecommitOnDestroy: true}) if err != nil { return nil, fmt.Errorf("tmpfs.FilesystemType.GetFilesystem: failed to create memory file from host file: %w", err) }