diff options
author | Fabricio Voznika <fvoznika@google.com> | 2019-01-25 17:22:04 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-01-25 17:23:07 -0800 |
commit | 55e8eb775b422a7485d6d1dc4f8e4c8fd32096da (patch) | |
tree | 7dff5a374ff29f6bffb3c543c0a37b1657f36ea0 /pkg/sentry/fs/gofer | |
parent | c6facd0358ae61849786dbbc0f4f5a07a25cb6f1 (diff) |
Make cacheRemoteRevalidating detect changes to file size
When file size changes outside the sandbox, page cache was not
refreshing file size which is required for cacheRemoteRevalidating.
In fact, cacheRemoteRevalidating should be skipping the cache
completely since it's not really benefiting from it. The cache is
cache is already bypassed for unstable attributes (see
cachePolicy.cacheUAttrs). And althought the cache is called to
map pages, they will always miss the cache and map directly from
the host.
Created a HostMappable struct that maps directly to the host and
use it for files with cacheRemoteRevalidating.
Closes #124
PiperOrigin-RevId: 230998440
Change-Id: Ic5f632eabe33b47241e05e98c95e9b2090ae08fc
Diffstat (limited to 'pkg/sentry/fs/gofer')
-rw-r--r-- | pkg/sentry/fs/gofer/cache_policy.go | 22 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/file.go | 9 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/inode.go | 30 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/path.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/session.go | 14 |
5 files changed, 58 insertions, 19 deletions
diff --git a/pkg/sentry/fs/gofer/cache_policy.go b/pkg/sentry/fs/gofer/cache_policy.go index 3d380f0e8..507d6900f 100644 --- a/pkg/sentry/fs/gofer/cache_policy.go +++ b/pkg/sentry/fs/gofer/cache_policy.go @@ -90,17 +90,29 @@ func (cp cachePolicy) cacheReaddir() bool { return cp == cacheAll || cp == cacheAllWritethrough } -// usePageCache determines whether the page cache should be used for the given -// inode. If the remote filesystem donates host FDs to the sentry, then the -// host kernel's page cache will be used, otherwise we will use a +// useCachingInodeOps determines whether the page cache should be used for the +// given inode. If the remote filesystem donates host FDs to the sentry, then +// the host kernel's page cache will be used, otherwise we will use a // sentry-internal page cache. -func (cp cachePolicy) usePageCache(inode *fs.Inode) bool { +func (cp cachePolicy) useCachingInodeOps(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 || cp == cacheRemoteRevalidating + return cp == cacheAll || cp == cacheAllWritethrough +} + +// cacheHandles determine whether handles need to be cached with the given +// inode. Handles must be cached when inode can be mapped into memory to +// implement InodeOperations.Mappable with stable handles. +func (cp cachePolicy) cacheHandles(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.useCachingInodeOps(inode) || cp == cacheRemoteRevalidating } // writeThough indicates whether writes to the file should be synced to the diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go index 3578b07a0..2181ddc68 100644 --- a/pkg/sentry/fs/gofer/file.go +++ b/pkg/sentry/fs/gofer/file.go @@ -204,7 +204,7 @@ func (f *fileOperations) Write(ctx context.Context, file *fs.File, src usermem.I return 0, syserror.EISDIR } cp := f.inodeOperations.session().cachePolicy - if cp.usePageCache(file.Dirent.Inode) { + if cp.useCachingInodeOps(file.Dirent.Inode) { n, err := f.inodeOperations.cachingInodeOps.Write(ctx, src, offset) if err != nil { return n, err @@ -225,7 +225,7 @@ func (f *fileOperations) Read(ctx context.Context, file *fs.File, dst usermem.IO return 0, syserror.EISDIR } - if f.inodeOperations.session().cachePolicy.usePageCache(file.Dirent.Inode) { + if f.inodeOperations.session().cachePolicy.useCachingInodeOps(file.Dirent.Inode) { return f.inodeOperations.cachingInodeOps.Read(ctx, file, dst, offset) } return dst.CopyOutFrom(ctx, f.handles.readWriterAt(ctx, offset)) @@ -267,10 +267,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 !f.inodeOperations.session().cachePolicy.usePageCache(file.Dirent.Inode) { - return syserror.ENODEV - } - return fsutil.GenericConfigureMMap(file, f.inodeOperations.cachingInodeOps, opts) + return f.inodeOperations.configureMMap(file, opts) } // Seek implements fs.FileOperations.Seek. diff --git a/pkg/sentry/fs/gofer/inode.go b/pkg/sentry/fs/gofer/inode.go index f0dc99fd0..043705c58 100644 --- a/pkg/sentry/fs/gofer/inode.go +++ b/pkg/sentry/fs/gofer/inode.go @@ -125,6 +125,10 @@ type inodeFileState struct { // failures. S/R is transparent to Sentry and the latter will continue // using its cached values after restore. savedUAttr *fs.UnstableAttr + + // hostMappable is created when using 'cacheRemoteRevalidating' to map pages + // directly from host. + hostMappable *fsutil.HostMappable } // Release releases file handles. @@ -166,6 +170,9 @@ func (i *inodeFileState) setHandlesForCachedIO(flags fs.FileFlags, h *handles) { i.writebackRW = true } } + if i.hostMappable != nil { + i.hostMappable.UpdateFD(i.fdLocked()) + } } // getCachedHandles returns any cached handles which would accelerate @@ -287,7 +294,10 @@ func (i *inodeFileState) Sync(ctx context.Context) error { func (i *inodeFileState) FD() int { i.handlesMu.RLock() defer i.handlesMu.RUnlock() + return i.fdLocked() +} +func (i *inodeFileState) fdLocked() int { // Assert that the file was actually opened. if i.writeback == nil && i.readthrough == nil { panic("cannot get host FD for a file that was never opened") @@ -344,9 +354,13 @@ 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.usePageCache(inode) { + if i.session().cachePolicy.useCachingInodeOps(inode) { return i.cachingInodeOps } + // This check is necessary because it's returning an interface type. + if i.fileState.hostMappable != nil { + return i.fileState.hostMappable + } return nil } @@ -434,7 +448,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 !i.session().cachePolicy.usePageCache(d.Inode) { + if !i.session().cachePolicy.cacheHandles(d.Inode) { h, err := newHandles(ctx, i.fileState.file, flags) if err != nil { return nil, err @@ -503,7 +517,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 i.session().cachePolicy.usePageCache(inode) { + if i.session().cachePolicy.useCachingInodeOps(inode) { return i.cachingInodeOps.Truncate(ctx, inode, length) } @@ -561,6 +575,16 @@ func (i *inodeOperations) StatFS(ctx context.Context) (fs.Info, error) { return info, nil } +func (i *inodeOperations) configureMMap(file *fs.File, opts *memmap.MMapOpts) error { + if i.session().cachePolicy.useCachingInodeOps(file.Dirent.Inode) { + return fsutil.GenericConfigureMMap(file, i.cachingInodeOps, opts) + } + if i.fileState.hostMappable != nil { + return fsutil.GenericConfigureMMap(file, i.fileState.hostMappable, opts) + } + return syserror.ENODEV +} + func init() { syserror.AddErrorUnwrapper(func(err error) (syscall.Errno, bool) { if _, ok := err.(p9.ErrSocket); ok { diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go index a324dc990..faedfb81c 100644 --- a/pkg/sentry/fs/gofer/path.go +++ b/pkg/sentry/fs/gofer/path.go @@ -128,7 +128,7 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string File: newFile, Host: hostFile, } - if iops.session().cachePolicy.usePageCache(d.Inode) { + if iops.session().cachePolicy.cacheHandles(d.Inode) { iops.fileState.setHandlesForCachedIO(flags, h) } return NewFile(ctx, d, name, flags, iops, h), nil diff --git a/pkg/sentry/fs/gofer/session.go b/pkg/sentry/fs/gofer/session.go index f76a83cd9..b5b1c8202 100644 --- a/pkg/sentry/fs/gofer/session.go +++ b/pkg/sentry/fs/gofer/session.go @@ -197,11 +197,17 @@ func newInodeOperations(ctx context.Context, s *session, file contextFile, qid p } } + var hm *fsutil.HostMappable + if s.cachePolicy == cacheRemoteRevalidating && fs.IsFile(sattr) { + hm = fsutil.NewHostMappable() + } + fileState := &inodeFileState{ - s: s, - file: file, - sattr: sattr, - key: deviceKey, + s: s, + file: file, + sattr: sattr, + key: deviceKey, + hostMappable: hm, } uattr := unstable(ctx, valid, attr, s.mounter, s.client) |