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

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",

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
+}

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)

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 = "/"
 

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
 	}
 

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()
 	}

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.