summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorNicolas Lacasse <nlacasse@google.com>2021-03-22 11:42:41 -0700
committergVisor bot <gvisor-bot@google.com>2021-03-22 11:44:31 -0700
commitb428fd02e627fc151527f0f26cbff9b05f6240e0 (patch)
tree4417e89f5d60d4b362980c53a3bb34eae2e274c5
parentcbac2d9f97031bdc18cb301e95db7c052dccc1ee (diff)
Avoid calling sync on each write in writethrough mode.
PiperOrigin-RevId: 364370595
-rw-r--r--pkg/sentry/fs/fsutil/inode_cached.go21
-rw-r--r--pkg/sentry/fs/gofer/file.go27
2 files changed, 28 insertions, 20 deletions
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