From 0d52b50f83c266f0d7bf1a45afae6811f24cf09e Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Thu, 13 Jul 2023 22:30:32 -0700 Subject: [PATCH] Use write(2) host syscall to perform writes on disk-backed MemoryFiles. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prepopulating pages for disk-backed MemoryFiles has proved to be futile. The mf.MapInternal()+safemem.CopySeq() approach used right now incurs a lot of page faults without page population. Page-by-page faults incurs a lot of context switching. On the other hand, the write syscall makes one context switch to kernel, and faults all the pages that are touched during write. Note that safemem.CopySeq() avoids a syscall and hence can be faster sometimes when the underlying page is populated. But with disk writebacks, it is hard to predict/account what is populated. Writebacks can happen asynchronously based on system load. Benchmark results show that FIO write performance improves a lot on rootfs: ``` goos: linux goarch: amd64 cpu: Intel(R) Xeon(R) CPU @ 2.80GHz │ benchout.runsc-before │ benchout.runsc-after │ │ sec/op │ sec/op vs base │ BuildABSL/page_cache.clean/filesystem.bindfs-4 90.20 ± 2% 89.44 ± 1% ~ (p=0.382 n=8) BuildGRPC/page_cache.clean/filesystem.bindfs-4 626.0 ± 1% 626.8 ± 0% ~ (p=0.505 n=8) RubySpecTest/page_cache.clean/filesystem.bindfs-4 52.11 ± 1% 52.37 ± 1% ~ (p=0.105 n=8) Fio/operation.write/blockSize.4K/filesystem.rootfs-4 2.509m ± 0% 2.509m ± 0% ~ (p=0.878 n=8) Fio/operation.write/blockSize.64K/filesystem.rootfs-4 2.009m ± 0% 1.507m ± 0% -24.98% (p=0.000 n=8) Fio/operation.write/blockSize.1024K/filesystem.rootfs-4 2.008m ± 0% 1.508m ± 0% -24.90% (p=0.000 n=8) │ benchout.runsc-before │ benchout.runsc-after │ │ bandwidth.bytes_per_second │ bandwidth.bytes_per_second vs base │ Fio/operation.write/blockSize.4K/filesystem.rootfs-4 649.1M ± 2% 705.2M ± 2% +8.64% (p=0.000 n=8) Fio/operation.write/blockSize.64K/filesystem.rootfs-4 991.1M ± 1% 1499.1M ± 3% +51.25% (p=0.000 n=8) Fio/operation.write/blockSize.1024K/filesystem.rootfs-4 1.198G ± 2% 1.945G ± 2% +62.34% (p=0.000 n=8) │ benchout.runsc-before │ benchout.runsc-after │ │ io_ops.ops_per_second │ io_ops.ops_per_second vs base │ Fio/operation.write/blockSize.4K/filesystem.rootfs-4 158.5k ± 2% 172.2k ± 2% +8.64% (p=0.000 n=8) Fio/operation.write/blockSize.64K/filesystem.rootfs-4 15.12k ± 1% 22.87k ± 3% +51.25% (p=0.000 n=8) Fio/operation.write/blockSize.1024K/filesystem.rootfs-4 1.143k ± 2% 1.855k ± 2% +62.34% (p=0.000 n=8) │ benchout.runsc-before │ benchout.runsc-after │ │ load.sec │ load.sec vs base │ RubySpecTest/page_cache.clean/filesystem.bindfs-4 7.555 ± 1% 7.585 ± 1% ~ (p=0.457 n=8) ``` PiperOrigin-RevId: 548021076 --- pkg/sentry/fsimpl/tmpfs/BUILD | 1 + pkg/sentry/fsimpl/tmpfs/regular_file.go | 34 ++++++++++++++++++------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/sentry/fsimpl/tmpfs/BUILD b/pkg/sentry/fsimpl/tmpfs/BUILD index ad768b9d4..66f55ab79 100644 --- a/pkg/sentry/fsimpl/tmpfs/BUILD +++ b/pkg/sentry/fsimpl/tmpfs/BUILD @@ -104,6 +104,7 @@ go_library( "//pkg/sentry/fsimpl/lock", "//pkg/sentry/fsmetric", "//pkg/sentry/fsutil", + "//pkg/sentry/hostfd", "//pkg/sentry/kernel/auth", "//pkg/sentry/kernel/pipe", "//pkg/sentry/kernel/time", diff --git a/pkg/sentry/fsimpl/tmpfs/regular_file.go b/pkg/sentry/fsimpl/tmpfs/regular_file.go index 34a8620f8..bdd9e074e 100644 --- a/pkg/sentry/fsimpl/tmpfs/regular_file.go +++ b/pkg/sentry/fsimpl/tmpfs/regular_file.go @@ -27,6 +27,7 @@ import ( "gvisor.dev/gvisor/pkg/safemem" "gvisor.dev/gvisor/pkg/sentry/fsmetric" "gvisor.dev/gvisor/pkg/sentry/fsutil" + "gvisor.dev/gvisor/pkg/sentry/hostfd" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/memmap" "gvisor.dev/gvisor/pkg/sentry/pgalloc" @@ -703,15 +704,7 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, mr := memmap.MappableRange{uint64(rw.off), uint64(end)} switch { case seg.Ok(): - // Get internal mappings. - ims, err := rw.file.inode.fs.mf.MapInternal(seg.FileRangeOf(seg.Range().Intersect(mr)), hostarch.Write) - if err != nil { - retErr = err - goto exitLoop - } - - // Copy to internal mappings. - n, err := safemem.CopySeq(ims, srcs) + n, err := rw.writeToMF(seg.FileRangeOf(seg.Range().Intersect(mr)), srcs) done += n rw.off += uint64(n) srcs = srcs.DropFirst64(n) @@ -774,6 +767,29 @@ exitLoop: return done, retErr } +func (rw *regularFileReadWriter) writeToMF(fr memmap.FileRange, srcs safemem.BlockSeq) (uint64, error) { + if rw.file.inode.fs.mf.IsDiskBacked() { + // Disk-backed files are not prepopulated. The safemem.CopySeq() approach + // used below incurs a lot of page faults without page prepopulation, which + // causes a lot of context switching. Use write(2) host syscall instead, + // which makes one context switch and faults all the pages that are touched + // during the write. + return hostfd.Pwritev2( + int32(rw.file.inode.fs.mf.FD()), // fd + srcs.TakeFirst64(fr.Length()), // srcs + int64(fr.Start), // offset + 0, // flags + ) + } + // Get internal mappings. + ims, err := rw.file.inode.fs.mf.MapInternal(fr, hostarch.Write) + if err != nil { + return 0, err + } + // Copy to internal mappings. + return safemem.CopySeq(ims, srcs) +} + // GetSeals returns the current set of seals on a memfd inode. func GetSeals(fd *vfs.FileDescription) (uint32, error) { f, ok := fd.Impl().(*regularFileFD)