From b428fd02e627fc151527f0f26cbff9b05f6240e0 Mon Sep 17 00:00:00 2001
From: Nicolas Lacasse <nlacasse@google.com>
Date: Mon, 22 Mar 2021 11:42:41 -0700
Subject: Avoid calling sync on each write in writethrough mode.

PiperOrigin-RevId: 364370595
---
 pkg/sentry/fs/fsutil/inode_cached.go | 21 ++++++++++++++-------
 pkg/sentry/fs/gofer/file.go          | 27 ++++++++++++++-------------
 2 files changed, 28 insertions(+), 20 deletions(-)

(limited to 'pkg/sentry')

diff --git a/pkg/sentry/fs/fsutil/inode_cached.go b/pkg/sentry/fs/fsutil/inode_cached.go
index 82eda3e43..0ed7aafa5 100644
--- a/pkg/sentry/fs/fsutil/inode_cached.go
+++ b/pkg/sentry/fs/fsutil/inode_cached.go
@@ -380,16 +380,17 @@ func (c *CachingInodeOperations) Allocate(ctx context.Context, offset, length in
 	return nil
 }
 
-// WriteOut implements fs.InodeOperations.WriteOut.
-func (c *CachingInodeOperations) WriteOut(ctx context.Context, inode *fs.Inode) error {
+// WriteDirtyPagesAndAttrs will write the dirty pages and attributes to the
+// gofer without calling Fsync on the remote file.
+func (c *CachingInodeOperations) WriteDirtyPagesAndAttrs(ctx context.Context, inode *fs.Inode) error {
 	c.attrMu.Lock()
+	defer c.attrMu.Unlock()
+	c.dataMu.Lock()
+	defer c.dataMu.Unlock()
 
 	// Write dirty pages back.
-	c.dataMu.Lock()
 	err := SyncDirtyAll(ctx, &c.cache, &c.dirty, uint64(c.attr.Size), c.mfp.MemoryFile(), c.backingFile.WriteFromBlocksAt)
-	c.dataMu.Unlock()
 	if err != nil {
-		c.attrMu.Unlock()
 		return err
 	}
 
@@ -399,12 +400,18 @@ func (c *CachingInodeOperations) WriteOut(ctx context.Context, inode *fs.Inode)
 
 	// Write out cached attributes.
 	if err := c.backingFile.SetMaskedAttributes(ctx, c.dirtyAttr, c.attr, false); err != nil {
-		c.attrMu.Unlock()
 		return err
 	}
 	c.dirtyAttr = fs.AttrMask{}
 
-	c.attrMu.Unlock()
+	return nil
+}
+
+// WriteOut implements fs.InodeOperations.WriteOut.
+func (c *CachingInodeOperations) WriteOut(ctx context.Context, inode *fs.Inode) error {
+	if err := c.WriteDirtyPagesAndAttrs(ctx, inode); err != nil {
+		return err
+	}
 
 	// Fsync the remote file.
 	return c.backingFile.Sync(ctx)
diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go
index 06d450ba6..8f5a87120 100644
--- a/pkg/sentry/fs/gofer/file.go
+++ b/pkg/sentry/fs/gofer/file.go
@@ -204,20 +204,8 @@ func (f *fileOperations) readdirAll(ctx context.Context) (map[string]fs.DentAttr
 	return entries, nil
 }
 
-// maybeSync will call FSync on the file if either the cache policy or file
-// flags require it.
+// maybeSync will call FSync on the file if the file flags require it.
 func (f *fileOperations) maybeSync(ctx context.Context, file *fs.File, offset, n int64) error {
-	if n == 0 {
-		// Nothing to sync.
-		return nil
-	}
-
-	if f.inodeOperations.session().cachePolicy.writeThrough(file.Dirent.Inode) {
-		// Call WriteOut directly, as some "writethrough" filesystems
-		// do not support sync.
-		return f.inodeOperations.cachingInodeOps.WriteOut(ctx, file.Dirent.Inode)
-	}
-
 	flags := file.Flags()
 	var syncType fs.SyncType
 	switch {
@@ -254,6 +242,19 @@ func (f *fileOperations) Write(ctx context.Context, file *fs.File, src usermem.I
 		n, err = src.CopyInTo(ctx, f.handles.readWriterAt(ctx, offset))
 	}
 
+	if n == 0 {
+		// Nothing written. We are done.
+		return 0, err
+	}
+
+	// Write the dirty pages and attributes if cache policy tells us to.
+	if f.inodeOperations.session().cachePolicy.writeThrough(file.Dirent.Inode) {
+		if werr := f.inodeOperations.cachingInodeOps.WriteDirtyPagesAndAttrs(ctx, file.Dirent.Inode); werr != nil {
+			// Report no bytes written since the write faild.
+			return 0, werr
+		}
+	}
+
 	// We may need to sync the written bytes.
 	if syncErr := f.maybeSync(ctx, file, offset, n); syncErr != nil {
 		// Sync failed. Report 0 bytes written, since none of them are
-- 
cgit v1.2.3