diff options
author | Nicolas Lacasse <nlacasse@google.com> | 2021-03-22 11:42:41 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-03-22 11:44:31 -0700 |
commit | b428fd02e627fc151527f0f26cbff9b05f6240e0 (patch) | |
tree | 4417e89f5d60d4b362980c53a3bb34eae2e274c5 | |
parent | cbac2d9f97031bdc18cb301e95db7c052dccc1ee (diff) |
Avoid calling sync on each write in writethrough mode.
PiperOrigin-RevId: 364370595
-rw-r--r-- | pkg/sentry/fs/fsutil/inode_cached.go | 21 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/file.go | 27 |
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 |