Add a new cache policy FSCACHE_WRITETHROUGH.

The new policy is identical to FSCACHE (which caches everything in memory), but
it also flushes writes to the backing fs agent immediately.

All gofer cache policy decisions have been moved into the cachePolicy type.
Previously they were sprinkled around the codebase.

There are many different things that we cache (page cache, negative dirents,
dirent LRU, unstable attrs, readdir results....), and I don't think we should
have individual flags to control each of these.  Instead, we should have a few
high-level cache policies that are consistent and useful to users.  This
refactoring makes it easy to add more such policies.

PiperOrigin-RevId: 201206937
Change-Id: I6e225c382b2e5e1b0ad4ccf8ca229873f4cd389d

7 files changed, 163 insertions, 82 deletions

diff --git a/pkg/sentry/fs/gofer/BUILD b/pkg/sentry/fs/gofer/BUILD
index ca42b0a54..e6f659c53 100644
--- a/pkg/sentry/fs/gofer/BUILD
+++ b/pkg/sentry/fs/gofer/BUILD
@@ -22,6 +22,7 @@ go_library(
     name = "gofer",
     srcs = [
         "attr.go",
+        "cache_policy.go",
         "context_file.go",
         "device.go",
         "file.go",
diff --git a/pkg/sentry/fs/gofer/cache_policy.go b/pkg/sentry/fs/gofer/cache_policy.go
new file mode 100644
index 000000000..eec8c07cb
--- /dev/null
+++ b/pkg/sentry/fs/gofer/cache_policy.go
@@ -0,0 +1,103 @@
+// Copyright 2018 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package gofer
+
+import (
+	"fmt"
+
+	"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
+)
+
+// cachePolicy is a 9p cache policy. It has methods that determine what to
+// cache (if anything) for a given inode.
+type cachePolicy int
+
+const (
+	// Cache nothing.
+	cacheNone cachePolicy = iota
+
+	// Use virtual file system cache for everything.
+	cacheAll
+
+	// Use virtual file system cache for everything, but send writes to the
+	// fs agent immediately.
+	cacheAllWritethrough
+)
+
+func parseCachePolicy(policy string) (cachePolicy, error) {
+	switch policy {
+	case "fscache":
+		return cacheAll, nil
+	case "none":
+		return cacheNone, nil
+	case "fscache_writethrough":
+		return cacheAllWritethrough, nil
+	}
+	return cacheNone, fmt.Errorf("unsupported cache mode: %s", policy)
+}
+
+// cacheUAtters determines whether unstable attributes should be cached for the
+// given inode.
+func (cp cachePolicy) cacheUAttrs(inode *fs.Inode) bool {
+	if !fs.IsFile(inode.StableAttr) && !fs.IsDir(inode.StableAttr) {
+		return false
+	}
+	return cp == cacheAll || cp == cacheAllWritethrough
+}
+
+// cacheReaddir determines whether readdir results should be cached.
+func (cp cachePolicy) cacheReaddir() bool {
+	return cp == cacheAll || cp == cacheAllWritethrough
+}
+
+// usePageCache determines whether the page cache should be used for the given
+// inode.
+func (cp cachePolicy) usePageCache(inode *fs.Inode) bool {
+	// Do cached IO for regular files only. Some "character devices" expect
+	// no caching.
+	if !fs.IsFile(inode.StableAttr) {
+		return false
+	}
+	return cp == cacheAll || cp == cacheAllWritethrough
+}
+
+// writeThough indicates whether writes to the file should be synced to the
+// gofer immediately.
+func (cp cachePolicy) writeThrough(inode *fs.Inode) bool {
+	return cp == cacheNone || cp == cacheAllWritethrough
+}
+
+// revalidateDirent indicates that dirents should be revalidated after they are
+// looked up.
+func (cp cachePolicy) revalidateDirent() bool {
+	return cp == cacheNone
+}
+
+// keepDirent indicates that dirents should be kept pinned in the dirent tree
+// even if there are no application references on the file.
+func (cp cachePolicy) keepDirent(inode *fs.Inode) bool {
+	if cp == cacheNone {
+		return false
+	}
+	sattr := inode.StableAttr
+	// NOTE: Only cache files, directories, and symlinks.
+	return fs.IsFile(sattr) || fs.IsDir(sattr) || fs.IsSymlink(sattr)
+}
+
+// cacheNegativeDirents indicates that negative dirents should be held in the
+// dirent tree.
+func (cp cachePolicy) cacheNegativeDirents() bool {
+	return cp == cacheAll || cp == cacheAllWritethrough
+}
diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go
index 07c9bf01d..69cee7026 100644
--- a/pkg/sentry/fs/gofer/file.go
+++ b/pkg/sentry/fs/gofer/file.go
@@ -93,7 +93,7 @@ func (f *fileOperations) Readdir(ctx context.Context, file *fs.File, serializer
 		DirCursor:  &f.dirCursor,
 	}
 	n, err := fs.DirentReaddir(ctx, file.Dirent, f, root, dirCtx, file.Offset())
-	if f.inodeOperations.session().cachePolicy != cacheNone {
+	if f.inodeOperations.session().cachePolicy.cacheUAttrs(file.Dirent.Inode) {
 		f.inodeOperations.cachingInodeOps.TouchAccessTime(ctx, file.Dirent.Inode)
 	}
 	return n, err
@@ -105,7 +105,7 @@ func (f *fileOperations) IterateDir(ctx context.Context, dirCtx *fs.DirCtx, offs
 	defer f.inodeOperations.readdirMu.Unlock()
 
 	// Fetch directory entries if needed.
-	if f.inodeOperations.readdirCache == nil || f.inodeOperations.session().cachePolicy == cacheNone {
+	if !f.inodeOperations.session().cachePolicy.cacheReaddir() || f.inodeOperations.readdirCache == nil {
 		entries, err := f.readdirAll(ctx)
 		if err != nil {
 			return offset, err
@@ -183,13 +183,20 @@ func (f *fileOperations) Write(ctx context.Context, file *fs.File, src usermem.I
 		// Not all remote file systems enforce this so this client does.
 		return 0, syserror.EISDIR
 	}
+	cp := f.inodeOperations.session().cachePolicy
+	if cp.usePageCache(file.Dirent.Inode) {
+		n, err := f.inodeOperations.cachingInodeOps.Write(ctx, src, offset)
+		if err != nil {
+			return n, err
+		}
+		if cp.writeThrough(file.Dirent.Inode) {
+			// Write out the file.
+			err = f.inodeOperations.cachingInodeOps.WriteOut(ctx, file.Dirent.Inode)
+		}
+		return n, err
 
-	// Do cached IO for regular files only. Some character devices expect no caching.
-	isFile := fs.IsFile(file.Dirent.Inode.StableAttr)
-	if f.inodeOperations.session().cachePolicy == cacheNone || !isFile {
-		return src.CopyInTo(ctx, f.handles.readWriterAt(ctx, offset))
 	}
-	return f.inodeOperations.cachingInodeOps.Write(ctx, src, offset)
+	return src.CopyInTo(ctx, f.handles.readWriterAt(ctx, offset))
 }
 
 // Read implements fs.FileOperations.Read.
@@ -199,12 +206,10 @@ func (f *fileOperations) Read(ctx context.Context, file *fs.File, dst usermem.IO
 		return 0, syserror.EISDIR
 	}
 
-	// Do cached IO for regular files only. Some character devices expect no caching.
-	isFile := fs.IsFile(file.Dirent.Inode.StableAttr)
-	if f.inodeOperations.session().cachePolicy == cacheNone || !isFile {
-		return dst.CopyOutFrom(ctx, f.handles.readWriterAt(ctx, offset))
+	if f.inodeOperations.session().cachePolicy.usePageCache(file.Dirent.Inode) {
+		return f.inodeOperations.cachingInodeOps.Read(ctx, file, dst, offset)
 	}
-	return f.inodeOperations.cachingInodeOps.Read(ctx, file, dst, offset)
+	return dst.CopyOutFrom(ctx, f.handles.readWriterAt(ctx, offset))
 }
 
 // Fsync implements fs.FileOperations.Fsync.
@@ -243,7 +248,7 @@ func (f *fileOperations) Flush(ctx context.Context, file *fs.File) error {
 
 // ConfigureMMap implements fs.FileOperations.ConfigureMMap.
 func (f *fileOperations) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.MMapOpts) error {
-	if !isFileCachable(f.inodeOperations.session(), file.Dirent.Inode) {
+	if !f.inodeOperations.session().cachePolicy.usePageCache(file.Dirent.Inode) {
 		return syserror.ENODEV
 	}
 	return fsutil.GenericConfigureMMap(file, f.inodeOperations.cachingInodeOps, opts)
diff --git a/pkg/sentry/fs/gofer/fs.go b/pkg/sentry/fs/gofer/fs.go
index a8a3ec19d..e041074d2 100644
--- a/pkg/sentry/fs/gofer/fs.go
+++ b/pkg/sentry/fs/gofer/fs.go
@@ -56,27 +56,6 @@ const (
 	privateUnixSocketKey = "privateunixsocket"
 )
 
-// cachePolicy is a 9p cache policy.
-type cachePolicy int
-
-const (
-	// TODO: fully support cache=none.
-	cacheNone cachePolicy = iota
-
-	// Use virtual file system cache.
-	cacheAll
-)
-
-func parseCachePolicy(policy string) (cachePolicy, error) {
-	switch policy {
-	case "fscache":
-		return cacheAll, nil
-	case "none":
-		return cacheNone, nil
-	}
-	return cacheNone, fmt.Errorf("unsupported cache mode: %s", policy)
-}
-
 // defaultAname is the default attach name.
 const defaultAname = "/"
 
diff --git a/pkg/sentry/fs/gofer/inode.go b/pkg/sentry/fs/gofer/inode.go
index c00da5fec..fa9013b75 100644
--- a/pkg/sentry/fs/gofer/inode.go
+++ b/pkg/sentry/fs/gofer/inode.go
@@ -335,23 +335,15 @@ func (i *inodeOperations) Release(ctx context.Context) {
 
 // Mappable implements fs.InodeOperations.Mappable.
 func (i *inodeOperations) Mappable(inode *fs.Inode) memmap.Mappable {
-	if i.session().cachePolicy == cacheNone || !fs.IsFile(inode.StableAttr) {
-		return nil
+	if i.session().cachePolicy.usePageCache(inode) {
+		return i.cachingInodeOps
 	}
-	return i.cachingInodeOps
-}
-
-func isCachable(session *session, inode *fs.Inode) bool {
-	return session.cachePolicy != cacheNone && (fs.IsFile(inode.StableAttr) || fs.IsDir(inode.StableAttr))
-}
-
-func isFileCachable(session *session, inode *fs.Inode) bool {
-	return session.cachePolicy != cacheNone && fs.IsFile(inode.StableAttr)
+	return nil
 }
 
 // UnstableAttr implements fs.InodeOperations.UnstableAttr.
 func (i *inodeOperations) UnstableAttr(ctx context.Context, inode *fs.Inode) (fs.UnstableAttr, error) {
-	if isCachable(i.session(), inode) {
+	if i.session().cachePolicy.cacheUAttrs(inode) {
 		return i.cachingInodeOps.UnstableAttr(ctx, inode)
 	}
 	return i.fileState.unstableAttr(ctx)
@@ -433,7 +425,7 @@ func (i *inodeOperations) NonBlockingOpen(ctx context.Context, p fs.PermMask) (*
 }
 
 func (i *inodeOperations) getFileDefault(ctx context.Context, d *fs.Dirent, flags fs.FileFlags) (*fs.File, error) {
-	if !isFileCachable(i.session(), d.Inode) {
+	if !i.session().cachePolicy.usePageCache(d.Inode) {
 		h, err := newHandles(ctx, i.fileState.file, flags)
 		if err != nil {
 			return nil, err
@@ -456,7 +448,7 @@ func (i *inodeOperations) getFileDefault(ctx context.Context, d *fs.Dirent, flag
 
 // SetPermissions implements fs.InodeOperations.SetPermissions.
 func (i *inodeOperations) SetPermissions(ctx context.Context, inode *fs.Inode, p fs.FilePermissions) bool {
-	if isCachable(i.session(), inode) {
+	if i.session().cachePolicy.cacheUAttrs(inode) {
 		return i.cachingInodeOps.SetPermissions(ctx, inode, p)
 	}
 
@@ -473,7 +465,7 @@ func (i *inodeOperations) SetOwner(ctx context.Context, inode *fs.Inode, owner f
 		return nil
 	}
 
-	if isCachable(i.session(), inode) {
+	if i.session().cachePolicy.cacheUAttrs(inode) {
 		return i.cachingInodeOps.SetOwner(ctx, inode, owner)
 	}
 
@@ -492,7 +484,7 @@ func (i *inodeOperations) SetOwner(ctx context.Context, inode *fs.Inode, owner f
 
 // SetTimestamps implements fs.InodeOperations.SetTimestamps.
 func (i *inodeOperations) SetTimestamps(ctx context.Context, inode *fs.Inode, ts fs.TimeSpec) error {
-	if isCachable(i.session(), inode) {
+	if i.session().cachePolicy.cacheUAttrs(inode) {
 		return i.cachingInodeOps.SetTimestamps(ctx, inode, ts)
 	}
 
@@ -502,7 +494,7 @@ func (i *inodeOperations) SetTimestamps(ctx context.Context, inode *fs.Inode, ts
 // Truncate implements fs.InodeOperations.Truncate.
 func (i *inodeOperations) Truncate(ctx context.Context, inode *fs.Inode, length int64) error {
 	// This can only be called for files anyway.
-	if isFileCachable(i.session(), inode) {
+	if i.session().cachePolicy.usePageCache(inode) {
 		return i.cachingInodeOps.Truncate(ctx, inode, length)
 	}
 
@@ -511,7 +503,7 @@ func (i *inodeOperations) Truncate(ctx context.Context, inode *fs.Inode, length
 
 // WriteOut implements fs.InodeOperations.WriteOut.
 func (i *inodeOperations) WriteOut(ctx context.Context, inode *fs.Inode) error {
-	if !isCachable(i.session(), inode) {
+	if !i.session().cachePolicy.cacheUAttrs(inode) {
 		return nil
 	}
 
diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go
index 6c4c2eed9..e78172bda 100644
--- a/pkg/sentry/fs/gofer/path.go
+++ b/pkg/sentry/fs/gofer/path.go
@@ -29,7 +29,7 @@ import (
 // Lookup loads an Inode at name into a Dirent based on the session's cache
 // policy.
 func (i *inodeOperations) Lookup(ctx context.Context, dir *fs.Inode, name string) (*fs.Dirent, error) {
-	if i.session().cachePolicy != cacheNone {
+	if i.session().cachePolicy.cacheReaddir() {
 		// Check to see if we have readdirCache that indicates the
 		// child does not exist.  Avoid holding readdirMu longer than
 		// we need to.
@@ -46,7 +46,7 @@ func (i *inodeOperations) Lookup(ctx context.Context, dir *fs.Inode, name string
 	qids, newFile, mask, p9attr, err := i.fileState.file.walkGetAttr(ctx, []string{name})
 	if err != nil {
 		if err == syscall.ENOENT {
-			if i.session().cachePolicy != cacheNone {
+			if i.session().cachePolicy.cacheNegativeDirents() {
 				// Return a negative Dirent. It will stay cached until something
 				// is created over it.
 				return fs.NewNegativeDirent(name), nil
@@ -95,7 +95,7 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string
 		return nil, err
 	}
 
-	i.touchModificationTime(ctx)
+	i.touchModificationTime(ctx, dir)
 
 	// Get the attributes of the file.
 	qid, mask, p9attr, err := getattr(ctx, newFile)
@@ -124,7 +124,7 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string
 		File: newFile,
 		Host: hostFile,
 	}
-	if isFileCachable(iops.session(), d.Inode) {
+	if iops.session().cachePolicy.usePageCache(d.Inode) {
 		iops.fileState.setHandlesForCachedIO(flags, h)
 	}
 	return NewFile(ctx, d, flags, iops, h), nil
@@ -136,12 +136,12 @@ func (i *inodeOperations) CreateLink(ctx context.Context, dir *fs.Inode, oldname
 	if _, err := i.fileState.file.symlink(ctx, oldname, newname, p9.UID(owner.UID), p9.GID(owner.GID)); err != nil {
 		return err
 	}
-	i.touchModificationTime(ctx)
+	i.touchModificationTime(ctx, dir)
 	return nil
 }
 
 // CreateHardLink implements InodeOperations.CreateHardLink.
-func (i *inodeOperations) CreateHardLink(ctx context.Context, _ *fs.Inode, target *fs.Inode, newName string) error {
+func (i *inodeOperations) CreateHardLink(ctx context.Context, inode *fs.Inode, target *fs.Inode, newName string) error {
 	targetOpts, ok := target.InodeOperations.(*inodeOperations)
 	if !ok {
 		return syscall.EXDEV
@@ -150,11 +150,11 @@ func (i *inodeOperations) CreateHardLink(ctx context.Context, _ *fs.Inode, targe
 	if err := i.fileState.file.link(ctx, &targetOpts.fileState.file, newName); err != nil {
 		return err
 	}
-	if i.session().cachePolicy == cacheAll {
+	if i.session().cachePolicy.cacheUAttrs(inode) {
 		// Increase link count.
 		targetOpts.cachingInodeOps.IncLinks(ctx)
 	}
-	i.touchModificationTime(ctx)
+	i.touchModificationTime(ctx, inode)
 	return nil
 }
 
@@ -164,10 +164,11 @@ func (i *inodeOperations) CreateDirectory(ctx context.Context, dir *fs.Inode, s
 	if _, err := i.fileState.file.mkdir(ctx, s, p9.FileMode(perm.LinuxMode()), p9.UID(owner.UID), p9.GID(owner.GID)); err != nil {
 		return err
 	}
-	if i.session().cachePolicy == cacheAll {
+	if i.session().cachePolicy.cacheUAttrs(dir) {
 		// Increase link count.
 		i.cachingInodeOps.IncLinks(ctx)
-
+	}
+	if i.session().cachePolicy.cacheReaddir() {
 		// Invalidate readdir cache.
 		i.markDirectoryDirty()
 	}
@@ -202,7 +203,7 @@ func (i *inodeOperations) Bind(ctx context.Context, dir *fs.Inode, name string,
 	// We're not going to use this file.
 	hostFile.Close()
 
-	i.touchModificationTime(ctx)
+	i.touchModificationTime(ctx, dir)
 
 	// Get the attributes of the file to create inode key.
 	qid, _, attr, err := getattr(ctx, newFile)
@@ -254,7 +255,7 @@ func (i *inodeOperations) Remove(ctx context.Context, dir *fs.Inode, name string
 	if removeSocket {
 		i.session().endpoints.remove(key)
 	}
-	i.touchModificationTime(ctx)
+	i.touchModificationTime(ctx, dir)
 
 	return nil
 }
@@ -265,10 +266,11 @@ func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, na
 	if err := i.fileState.file.unlinkAt(ctx, name, 0x200); err != nil {
 		return err
 	}
-	if i.session().cachePolicy == cacheAll {
+	if i.session().cachePolicy.cacheUAttrs(dir) {
 		// Decrease link count and updates atime.
 		i.cachingInodeOps.DecLinks(ctx)
-
+	}
+	if i.session().cachePolicy.cacheReaddir() {
 		// Invalidate readdir cache.
 		i.markDirectoryDirty()
 	}
@@ -294,14 +296,17 @@ func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldNa
 		return err
 	}
 
-	// Update cached state.
-	if i.session().cachePolicy == cacheAll {
-		// Is the renamed entity a directory? Fix link counts.
-		if fs.IsDir(i.fileState.sattr) {
+	// Is the renamed entity a directory? Fix link counts.
+	if fs.IsDir(i.fileState.sattr) {
+		// Update cached state.
+		if i.session().cachePolicy.cacheUAttrs(oldParent) {
 			oldParentInodeOperations.cachingInodeOps.DecLinks(ctx)
+		}
+		if i.session().cachePolicy.cacheUAttrs(newParent) {
 			newParentInodeOperations.cachingInodeOps.IncLinks(ctx)
 		}
-
+	}
+	if i.session().cachePolicy.cacheReaddir() {
 		// Mark old directory dirty.
 		oldParentInodeOperations.markDirectoryDirty()
 		if oldParent != newParent {
@@ -312,10 +317,11 @@ func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldNa
 	return nil
 }
 
-func (i *inodeOperations) touchModificationTime(ctx context.Context) {
-	if i.session().cachePolicy == cacheAll {
+func (i *inodeOperations) touchModificationTime(ctx context.Context, inode *fs.Inode) {
+	if i.session().cachePolicy.cacheUAttrs(inode) {
 		i.cachingInodeOps.TouchModificationTime(ctx)
-
+	}
+	if i.session().cachePolicy.cacheReaddir() {
 		// Invalidate readdir cache.
 		i.markDirectoryDirty()
 	}
diff --git a/pkg/sentry/fs/gofer/session.go b/pkg/sentry/fs/gofer/session.go
index baf00d8e7..21dc5e08d 100644
--- a/pkg/sentry/fs/gofer/session.go
+++ b/pkg/sentry/fs/gofer/session.go
@@ -122,17 +122,12 @@ func (s *session) Destroy() {
 
 // Revalidate returns true if the cache policy is does not allow for VFS caching.
 func (s *session) Revalidate(*fs.Dirent) bool {
-	return s.cachePolicy == cacheNone
+	return s.cachePolicy.revalidateDirent()
 }
 
 // TakeRefs takes an extra reference on dirent if possible.
-func (s *session) Keep(dirent *fs.Dirent) bool {
-	sattr := dirent.Inode.StableAttr
-	if s.cachePolicy == cacheNone {
-		return false
-	}
-	// NOTE: Only cache files, directories, and symlinks.
-	return fs.IsFile(sattr) || fs.IsDir(sattr) || fs.IsSymlink(sattr)
+func (s *session) Keep(d *fs.Dirent) bool {
+	return s.cachePolicy.keepDirent(d.Inode)
 }
 
 // ResetInodeMappings implements fs.MountSourceOperations.ResetInodeMappings.