Make gofer.dentry a generic type.

This introduces a new gofer.dentry.impl field which can hold dentry
implementation specific details. For now there exists only one implementation,
which is a lisafs dentry.

This work is in preparation for adding a direct host dentry implementation,
which will make host syscalls instead of making RPCs.

This change should have no change in behavior or performance.

PiperOrigin-RevId: 503203994

pkg/lisafs/client_file.go

@@ -304,7 +304,13 @@ func (f *ClientFD) SetStat(ctx context.Context, stat *linux.Statx) (uint32, erro
 	ctx.UninterruptibleSleepStart(false)
 	err := f.client.SndRcvMessage(SetStat, uint32(req.SizeBytes()), req.MarshalUnsafe, resp.CheckedUnmarshal, nil, req.String, resp.String)
 	ctx.UninterruptibleSleepFinish(false)
-	return resp.FailureMask, unix.Errno(resp.FailureErrNo), err
+	if err != nil {
+		return 0, nil, err
+	}
+	if resp.FailureMask == 0 {
+		return 0, nil, nil
+	}
+	return resp.FailureMask, unix.Errno(resp.FailureErrNo), nil
 }
 
 // WalkMultiple makes the Walk RPC with multiple path components.

pkg/sentry/fsimpl/gofer/BUILD

@@ -53,6 +53,7 @@ go_template_instance(
 go_library(
     name = "gofer",
     srcs = [
+        "dentry_impl.go",
         "dentry_list.go",
         "directory.go",
         "filesystem.go",
@@ -60,6 +61,7 @@ go_library(
         "gofer.go",
         "handle.go",
         "host_named_pipe.go",
+        "lisafs_dentry.go",
         "regular_file.go",
         "revalidate.go",
         "save_restore.go",

pkg/sentry/fsimpl/gofer/dentry_impl.go

@@ -0,0 +1,424 @@
+// Copyright 2022 The gVisor Authors.
+//
+// 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 (
+	"golang.org/x/sys/unix"
+	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/context"
+	"gvisor.dev/gvisor/pkg/log"
+	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
+	"gvisor.dev/gvisor/pkg/sentry/vfs"
+)
+
+// We do *not* define an interface for dentry.impl because making interface
+// method calls is almost 2.5x slower than calling the same method on a
+// concrete type. Instead, we use type assertions in switch statements. The
+// asserted type is a concrete dentry implementation and methods are called
+// directly on the concrete type. This helps in the following ways:
+//
+// 1. This is faster because concrete type assertion just needs to compare the
+//    itab pointer in the interface value to a constant which is relatively
+//    cheap. Benchmarking showed that such type switches don't add almost any
+//    overhead.
+// 2. Passing any pointer to an interface method immediately causes the pointed
+//    object to escape to heap. Making concrete method calls allows escape
+//    analysis to proceed as usual and avoids heap allocations.
+//
+// Also note that the default case in these type switch statements panics. We
+// do not do panic(fmt.Sprintf("... %T", d.impl)) because somehow it adds a lot
+// of overhead to the type switch. So instead we panic with a constant string.
+
+// Precondition: d.handleMu must be locked.
+func (d *dentry) isReadHandleOk() bool {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.readFDLisa.Ok()
+	case nil: // synthetic dentry
+		return false
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: d.handleMu must be locked.
+func (d *dentry) isWriteHandleOk() bool {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.writeFDLisa.Ok()
+	case nil: // synthetic dentry
+		return false
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: d.handleMu must be locked.
+func (d *dentry) readHandle() handle {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return handle{
+			fdLisa: dt.readFDLisa,
+			fd:     d.readFD.RacyLoad(),
+		}
+	case nil: // synthetic dentry
+		return noHandle
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: d.handleMu must be locked.
+func (d *dentry) writeHandle() handle {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return handle{
+			fdLisa: dt.writeFDLisa,
+			fd:     d.writeFD.RacyLoad(),
+		}
+	case nil: // synthetic dentry
+		return noHandle
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) openHandle(ctx context.Context, read, write, trunc bool) (handle, error) {
+	flags := uint32(unix.O_RDONLY)
+	switch {
+	case read && write:
+		flags = unix.O_RDWR
+	case read:
+		flags = unix.O_RDONLY
+	case write:
+		flags = unix.O_WRONLY
+	default:
+		log.Debugf("openHandle called with read = write = false. Falling back to read only FD.")
+	}
+	if trunc {
+		flags |= unix.O_TRUNC
+	}
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		openFD, hostFD, err := dt.controlFD.OpenAt(ctx, flags)
+		if err != nil {
+			return noHandle, err
+		}
+		return handle{
+			fdLisa: dt.controlFD.Client().NewFD(openFD),
+			fd:     int32(hostFD),
+		}, nil
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Preconditions:
+//   - d.handleMu must be locked.
+//   - !d.isSynthetic().
+func (d *dentry) updateHandles(ctx context.Context, h handle, readable, writable bool) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		dt.updateHandles(ctx, h, readable, writable)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// updateMetadataLocked updates the dentry's metadata fields. The h parameter
+// is optional. If it is not provided, an appropriate FD should be chosen to
+// stat the remote file.
+//
+// Preconditions:
+//   - !d.isSynthetic().
+//   - d.metadataMu is locked.
+//
+// +checklocks:d.metadataMu
+func (d *dentry) updateMetadataLocked(ctx context.Context, h handle) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.updateMetadataLocked(ctx, h) // +checklocksforce: acquired by precondition.
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+func (d *dentry) chmod(ctx context.Context, mode uint16) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return chmod(ctx, dt.controlFD, mode)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Preconditions:
+//   - !d.isSynthetic().
+//   - d.handleMu is locked.
+func (d *dentry) setStatLocked(ctx context.Context, stat *linux.Statx) (uint32, error, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.SetStat(ctx, stat)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+func (d *dentry) destroyImpl(ctx context.Context) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		dt.destroy(ctx)
+	case nil: // synthetic dentry
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Postcondition: Caller must do dentry caching appropriately.
+func (d *dentry) getRemoteChild(ctx context.Context, name string) (*dentry, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.getRemoteChild(ctx, name)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Preconditions:
+//   - fs.renameMu must be locked.
+//   - parent.dirMu must be locked.
+//   - parent.isDir().
+//   - name is not "." or "..".
+//   - dentry at name must not already exist in dentry tree.
+//
+// Postcondition: The returned dentry is already cached appropriately.
+func (d *dentry) getRemoteChildAndWalkPathLocked(ctx context.Context, rp *vfs.ResolvingPath, ds **[]*dentry) (*dentry, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.getRemoteChildAndWalkPathLocked(ctx, rp, ds)
+		// TODO(b/258687694): For directfs, remember to use fs.getRemoteChildLocked
+		// so that dentry caching is done properly.
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) listXattrImpl(ctx context.Context, size uint64) ([]string, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.ListXattr(ctx, size)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) getXattrImpl(ctx context.Context, opts *vfs.GetXattrOptions) (string, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.GetXattr(ctx, opts.Name, opts.Size)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) setXattrImpl(ctx context.Context, opts *vfs.SetXattrOptions) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.SetXattr(ctx, opts.Name, opts.Value, opts.Flags)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) removeXattrImpl(ctx context.Context, name string) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.RemoveXattr(ctx, name)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) mknod(ctx context.Context, name string, creds *auth.Credentials, opts *vfs.MknodOptions) (*dentry, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.mknod(ctx, name, creds, opts)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) link(ctx context.Context, target *dentry, name string) (*dentry, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.link(ctx, target.impl.(*lisafsDentry), name)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) mkdir(ctx context.Context, name string, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.mkdir(ctx, name, mode, uid, gid)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) symlink(ctx context.Context, name, target string, creds *auth.Credentials) (*dentry, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.symlink(ctx, name, target, creds)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) openCreate(ctx context.Context, name string, accessFlags uint32, mode linux.FileMode, uid auth.KUID, gid auth.KGID) (*dentry, handle, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.openCreate(ctx, name, accessFlags, mode, uid, gid)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Preconditions:
+//   - d.isDir().
+//   - d.handleMu must be locked.
+//   - !d.isSynthetic().
+func (d *dentry) getDirentsLocked(ctx context.Context, count int, recordDirent func(name string, key inoKey, dType uint8)) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.getDirentsLocked(ctx, count, recordDirent)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) flush(ctx context.Context) error {
+	d.handleMu.RLock()
+	defer d.handleMu.RUnlock()
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return flush(ctx, dt.writeFDLisa)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) allocate(ctx context.Context, mode, offset, length uint64) error {
+	d.handleMu.RLock()
+	defer d.handleMu.RUnlock()
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.writeFDLisa.Allocate(ctx, mode, offset, length)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) connect(ctx context.Context, sockType linux.SockType) (int, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.Connect(ctx, sockType)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) readlinkImpl(ctx context.Context) (string, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.ReadLinkAt(ctx)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) unlink(ctx context.Context, name string, flags uint32) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.UnlinkAt(ctx, name, flags)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) rename(ctx context.Context, oldName string, newParent *dentry, newName string) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.controlFD.RenameAt(ctx, oldName, newParent.impl.(*lisafsDentry).controlFD.ID(), newName)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Precondition: !d.isSynthetic().
+func (d *dentry) statfs(ctx context.Context) (linux.Statfs, error) {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		return dt.statfs(ctx)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+func (fs *filesystem) restoreRoot(ctx context.Context, opts *vfs.CompleteRestoreOptions) error {
+	// The root is always non-synthetic.
+	switch dt := fs.root.impl.(type) {
+	case *lisafsDentry:
+		rootInode, err := fs.initClient(ctx)
+		if err != nil {
+			return err
+		}
+		return dt.restoreFile(ctx, &rootInode, opts)
+	default:
+		panic("unknown dentry implementation")
+	}
+}
+
+// Preconditions:
+//   - !d.isSynthetic().
+//   - d.parent != nil and has been restored.
+func (d *dentry) restoreFile(ctx context.Context, opts *vfs.CompleteRestoreOptions) error {
+	switch dt := d.impl.(type) {
+	case *lisafsDentry:
+		inode, err := d.parent.impl.(*lisafsDentry).controlFD.Walk(ctx, d.name)
+		if err != nil {
+			return err
+		}
+		return dt.restoreFile(ctx, &inode, opts)
+	default:
+		panic("unknown dentry implementation")
+	}
+}

pkg/sentry/fsimpl/gofer/directory.go

@@ -22,9 +22,6 @@ import (
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/hostarch"
-	"gvisor.dev/gvisor/pkg/lisafs"
-	"gvisor.dev/gvisor/pkg/log"
-	"gvisor.dev/gvisor/pkg/refs"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/pipe"
 	"gvisor.dev/gvisor/pkg/sentry/socket/unix/transport"
@@ -36,28 +33,6 @@ func (d *dentry) isDir() bool {
 	return d.fileType() == linux.S_IFDIR
 }
 
-// Preconditions:
-//   - filesystem.renameMu must be locked.
-//   - d.dirMu must be locked.
-//   - d.isDir().
-//   - child must be a newly-created dentry that has never had a parent.
-func (d *dentry) insertCreatedChildLocked(ctx context.Context, childIno *lisafs.Inode, childName string, updateChild func(child *dentry), ds **[]*dentry) error {
-	child, err := d.fs.newDentry(ctx, childIno)
-	if err != nil {
-		if err := d.controlFDLisa.UnlinkAt(ctx, childName, 0 /* flags */); err != nil {
-			log.Warningf("failed to clean up created child %s after newDentry() failed: %v", childName, err)
-		}
-		d.fs.client.CloseFD(ctx, childIno.ControlFD, false /* flush */)
-		return err
-	}
-	d.cacheNewChildLocked(child, childName)
-	appendNewChildDentry(ds, d, child)
-	if updateChild != nil {
-		updateChild(child)
-	}
-	return nil
-}
-
 // Preconditions:
 //   - filesystem.renameMu must be locked.
 //   - d.dirMu must be locked.
@@ -129,19 +104,13 @@ type createSyntheticOpts struct {
 	pipe *pipe.VFSPipe
 }
 
-// createSyntheticChildLocked creates a synthetic file with the given name
-// in d.
-//
-// Preconditions:
-//   - d.dirMu must be locked.
-//   - d.isDir().
-//   - d does not already contain a child with the given name.
-func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) {
-	now := d.fs.clock.Now().Nanoseconds()
+// newSyntheticDentry creates a synthetic file with the given name.
+func (fs *filesystem) newSyntheticDentry(opts *createSyntheticOpts) *dentry {
+	now := fs.clock.Now().Nanoseconds()
 	child := &dentry{
-		refs:      atomicbitops.FromInt64(1), // held by d
-		fs:        d.fs,
-		ino:       d.fs.nextIno(),
+		refs:      atomicbitops.FromInt64(1), // held by parent.
+		fs:        fs,
+		ino:       fs.nextIno(),
 		mode:      atomicbitops.FromUint32(uint32(opts.mode)),
 		uid:       atomicbitops.FromUint32(uint32(opts.kuid)),
 		gid:       atomicbitops.FromUint32(uint32(opts.kgid)),
@@ -155,7 +124,6 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) {
 		mmapFD:    atomicbitops.FromInt32(-1),
 		nlink:     atomicbitops.FromUint32(2),
 	}
-	refs.Register(child)
 	switch opts.mode.FileType() {
 	case linux.S_IFDIR:
 		// Nothing else needs to be done.
@@ -166,13 +134,8 @@ func (d *dentry) createSyntheticChildLocked(opts *createSyntheticOpts) {
 	default:
 		panic(fmt.Sprintf("failed to create synthetic file of unrecognized type: %v", opts.mode.FileType()))
 	}
-	child.pf.dentry = child
-	child.cacheEntry.d = child
-	child.syncableListEntry.d = child
-	child.vfsd.Init(child)
-
-	d.cacheNewChildLocked(child, opts.name)
-	d.syntheticChildren++
+	child.init(nil /* impl */)
+	return child
 }
 
 // Preconditions:
@@ -274,53 +237,28 @@ func (d *dentry) getDirents(ctx context.Context) ([]vfs.Dirent, error) {
 			// duplicate entries for synthetic children.
 			realChildren = make(map[string]struct{})
 		}
-		const count = 64 * 1024 // for consistency with the vfs1 client
 		d.handleMu.RLock()
-		if !d.isReadFileOk() {
+		if !d.isReadHandleOk() {
 			// This should not be possible because a readable handle should
 			// have been opened when the calling directoryFD was opened.
-			d.handleMu.RUnlock()
 			panic("gofer.dentry.getDirents called without a readable handle")
 		}
-		// shouldSeek0 indicates whether the server should SEEK to 0 before reading
-		// directory entries.
-		shouldSeek0 := true
-		for {
-			countLisa := int32(count)
-			if shouldSeek0 {
-				// See lisafs.Getdents64Req.Count.
-				countLisa = -countLisa
-				shouldSeek0 = false
+		const count = 64 * 1024 // for consistency with the vfs1 client
+		err := d.getDirentsLocked(ctx, count, func(name string, key inoKey, dType uint8) {
+			dirent := vfs.Dirent{
+				Name:    name,
+				Ino:     d.fs.inoFromKey(key),
+				NextOff: int64(len(dirents) + 1),
+				Type:    dType,
 			}
-			direntsLisa, err := d.readFDLisa.Getdents64(ctx, countLisa)
-			if err != nil {
-				d.handleMu.RUnlock()
-				return nil, err
-			}
-			if len(direntsLisa) == 0 {
-				d.handleMu.RUnlock()
-				break
-			}
-			for i := range direntsLisa {
-				name := string(direntsLisa[i].Name)
-				if name == "." || name == ".." {
-					continue
-				}
-				dirent := vfs.Dirent{
-					Name: name,
-					Ino: d.fs.inoFromKey(inoKey{
-						ino:      uint64(direntsLisa[i].Ino),
-						devMinor: uint32(direntsLisa[i].DevMinor),
-						devMajor: uint32(direntsLisa[i].DevMajor),
-					}),
-					NextOff: int64(len(dirents) + 1),
-					Type:    uint8(direntsLisa[i].Type),
-				}
-				dirents = append(dirents, dirent)
-				if realChildren != nil {
-					realChildren[name] = struct{}{}
-				}
+			dirents = append(dirents, dirent)
+			if realChildren != nil {
+				realChildren[name] = struct{}{}
 			}
+		})
+		d.handleMu.RUnlock()
+		if err != nil {
+			return nil, err
 		}
 	}
 	// Emit entries for synthetic children.

pkg/sentry/fsimpl/gofer/gofer_test.go

@@ -42,7 +42,7 @@ func TestDestroyIdempotent(t *testing.T) {
 			Mode: linux.S_IFDIR | 0666,
 		},
 	}
-	parent, err := fs.newDentry(ctx, &parentInode)
+	parent, err := fs.newLisafsDentry(ctx, &parentInode)
 	if err != nil {
 		t.Fatalf("fs.newDentry(): %v", err)
 	}
@@ -55,7 +55,7 @@ func TestDestroyIdempotent(t *testing.T) {
 			Size: 0,
 		},
 	}
-	child, err := fs.newDentry(ctx, &childInode)
+	child, err := fs.newLisafsDentry(ctx, &childInode)
 	if err != nil {
 		t.Fatalf("fs.newDentry(): %v", err)
 	}

pkg/sentry/fsimpl/gofer/handle.go

@@ -18,12 +18,16 @@ import (
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/lisafs"
-	"gvisor.dev/gvisor/pkg/log"
 	"gvisor.dev/gvisor/pkg/safemem"
 	"gvisor.dev/gvisor/pkg/sentry/hostfd"
 	"gvisor.dev/gvisor/pkg/sync"
 )
 
+var noHandle = handle{
+	fdLisa: lisafs.ClientFD{}, // zero value is fine.
+	fd:     -1,
+}
+
 // handle represents a remote "open file descriptor", consisting of an opened
 // lisafs FD and optionally a host file descriptor.
 //
@@ -33,39 +37,10 @@ type handle struct {
 	fd     int32 // -1 if unavailable
 }
 
-// Preconditions: read || write.
-func openHandle(ctx context.Context, fdLisa lisafs.ClientFD, read, write, trunc bool) (handle, error) {
-	flags := uint32(unix.O_RDONLY)
-	switch {
-	case read && write:
-		flags = unix.O_RDWR
-	case read:
-		flags = unix.O_RDONLY
-	case write:
-		flags = unix.O_WRONLY
-	default:
-		log.Debugf("openHandle called with read = write = false. Falling back to read only FD.")
-	}
-	if trunc {
-		flags |= unix.O_TRUNC
-	}
-	openFD, hostFD, err := fdLisa.OpenAt(ctx, flags)
-	if err != nil {
-		return handle{fd: -1}, err
-	}
-	h := handle{
-		fdLisa: fdLisa.Client().NewFD(openFD),
-		fd:     int32(hostFD),
-	}
-	return h, nil
-}
-
-func (h *handle) isOpen() bool {
-	return h.fdLisa.Ok()
-}
-
 func (h *handle) close(ctx context.Context) {
-	h.fdLisa.Close(ctx, true /* flush */)
+	if h.fdLisa.Ok() {
+		h.fdLisa.Close(ctx, true /* flush */)
+	}
 	if h.fd >= 0 {
 		unix.Close(int(h.fd))
 		h.fd = -1
@@ -102,6 +77,31 @@ func (h *handle) writeFromBlocksAt(ctx context.Context, srcs safemem.BlockSeq, o
 	return safemem.FromIOWriter{rw}.WriteFromBlocks(srcs)
 }
 
+func (h *handle) allocate(ctx context.Context, mode, offset, length uint64) error {
+	if h.fdLisa.Ok() {
+		return h.fdLisa.Allocate(ctx, mode, offset, length)
+	}
+	if h.fd >= 0 {
+		return unix.Fallocate(int(h.fd), uint32(mode), int64(offset), int64(length))
+	}
+	return nil
+}
+
+func (h *handle) sync(ctx context.Context) error {
+	// If we have a host FD, fsyncing it is likely to be faster than an fsync
+	// RPC.
+	if h.fd >= 0 {
+		ctx.UninterruptibleSleepStart(false)
+		err := unix.Fsync(int(h.fd))
+		ctx.UninterruptibleSleepFinish(false)
+		return err
+	}
+	if h.fdLisa.Ok() {
+		return h.fdLisa.Sync(ctx)
+	}
+	return nil
+}
+
 type handleReadWriter struct {
 	ctx context.Context
 	h   *handle

pkg/sentry/fsimpl/gofer/regular_file.go

@@ -94,21 +94,14 @@ func (fd *regularFileFD) OnClose(ctx context.Context) error {
 			return nil
 		}
 	}
-	d.handleMu.RLock()
-	defer d.handleMu.RUnlock()
-	if !d.writeFDLisa.Ok() {
-		return nil
-	}
-	return d.writeFDLisa.Flush(ctx)
+	return d.flush(ctx)
 }
 
 // Allocate implements vfs.FileDescriptionImpl.Allocate.
 func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint64) error {
 	d := fd.dentry()
 	return d.doAllocate(ctx, offset, length, func() error {
-		d.handleMu.RLock()
-		defer d.handleMu.RUnlock()
-		return d.writeFDLisa.Allocate(ctx, mode, offset, length)
+		return d.allocate(ctx, mode, offset, length)
 	})
 }
 
@@ -279,15 +272,10 @@ func (fd *regularFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
 		// If setuid or setgid were set, update d.mode and propagate
 		// changes to the host.
 		if newMode := vfs.ClearSUIDAndSGID(oldMode); newMode != oldMode {
-			d.mode.Store(newMode)
-			stat := linux.Statx{Mask: linux.STATX_MODE, Mode: uint16(newMode)}
-			failureMask, failureErr, err := d.controlFDLisa.SetStat(ctx, &stat)
-			if err != nil {
+			if err := d.chmod(ctx, uint16(newMode)); err != nil {
 				return 0, offset, err
 			}
-			if failureMask != 0 {
-				return 0, offset, failureErr
-			}
+			d.mode.Store(newMode)
 		}
 	}
 
@@ -379,9 +367,9 @@ func (rw *dentryReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error)
 	// coherence with memory-mapped I/O), or if InteropModeShared is in effect
 	// (which prevents us from caching file contents and makes dentry.size
 	// unreliable), or if the file was opened O_DIRECT, read directly from
-	// dentry.readHandleLocked() without locking dentry.dataMu.
+	// readHandle() without locking dentry.dataMu.
 	rw.d.handleMu.RLock()
-	h := rw.d.readHandleLocked()
+	h := rw.d.readHandle()
 	if (rw.d.mmapFD.RacyLoad() >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct {
 		n, err := h.readToBlocksAt(rw.ctx, dsts, rw.off)
 		rw.d.handleMu.RUnlock()
@@ -498,10 +486,10 @@ func (rw *dentryReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, erro
 	// If we have a mmappable host FD (which must be used here to ensure
 	// coherence with memory-mapped I/O), or if InteropModeShared is in effect
 	// (which prevents us from caching file contents), or if the file was
-	// opened with O_DIRECT, write directly to dentry.writeHandleLocked()
+	// opened with O_DIRECT, write directly to dentry.writeHandle()
 	// without locking dentry.dataMu.
 	rw.d.handleMu.RLock()
-	h := rw.d.writeHandleLocked()
+	h := rw.d.writeHandle()
 	if (rw.d.mmapFD.RacyLoad() >= 0 && !rw.d.fs.opts.forcePageCache) || rw.d.fs.opts.interop == InteropModeShared || rw.direct {
 		n, err := h.writeFromBlocksAt(rw.ctx, srcs, rw.off)
 		rw.off += n
@@ -609,7 +597,7 @@ func (d *dentry) writeback(ctx context.Context, offset, size int64) error {
 	}
 	d.handleMu.RLock()
 	defer d.handleMu.RUnlock()
-	h := d.writeHandleLocked()
+	h := d.writeHandle()
 	d.dataMu.Lock()
 	defer d.dataMu.Unlock()
 	// Compute the range of valid bytes (overflow-checked).
@@ -649,7 +637,7 @@ func regularFileSeekLocked(ctx context.Context, d *dentry, fdOffset, offset int6
 	case linux.SEEK_END, linux.SEEK_DATA, linux.SEEK_HOLE:
 		// Ensure file size is up to date.
 		if !d.cachedMetadataAuthoritative() {
-			if err := d.updateMetadata(ctx, nil); err != nil {
+			if err := d.updateMetadata(ctx); err != nil {
 				return 0, err
 			}
 		}
@@ -809,7 +797,7 @@ func (d *dentry) Translate(ctx context.Context, required, optional memmap.Mappab
 	}
 
 	mf := d.fs.mfp.MemoryFile()
-	h := d.readHandleLocked()
+	h := d.readHandle()
 	_, cerr := d.cache.Fill(ctx, required, maxFillRange(required, optional), d.size.Load(), mf, usage.PageCache, true /* populate */, h.readToBlocksAt)
 
 	var ts []memmap.Translation
@@ -881,7 +869,7 @@ func (d *dentry) InvalidateUnsavable(ctx context.Context) error {
 	mf := d.fs.mfp.MemoryFile()
 	d.handleMu.RLock()
 	defer d.handleMu.RUnlock()
-	h := d.writeHandleLocked()
+	h := d.writeHandle()
 	d.dataMu.Lock()
 	defer d.dataMu.Unlock()
 	if err := fsutil.SyncDirtyAll(ctx, &d.cache, &d.dirty, d.size.Load(), mf, h.writeFromBlocksAt); err != nil {
@@ -905,7 +893,7 @@ func (d *dentry) Evict(ctx context.Context, er pgalloc.EvictableRange) {
 	defer d.mapsMu.Unlock()
 	d.handleMu.RLock()
 	defer d.handleMu.RUnlock()
-	h := d.writeHandleLocked()
+	h := d.writeHandle()
 	d.dataMu.Lock()
 	defer d.dataMu.Unlock()
 

pkg/sentry/fsimpl/gofer/revalidate.go

@@ -15,6 +15,7 @@
 package gofer
 
 import (
+	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 	"gvisor.dev/gvisor/pkg/sync"
@@ -235,27 +236,57 @@ func (fs *filesystem) revalidateHelper(ctx context.Context, vfsObj *vfs.VirtualF
 	// Lock metadata on all dentries *before* getting attributes for them.
 	state.lockAllMetadata()
 
-	stats, err := state.start.controlFDLisa.WalkStat(ctx, state.names)
-	if err != nil {
-		return err
+	var stats []linux.Statx
+	switch dt := state.start.impl.(type) {
+	case *lisafsDentry:
+		var err error
+		stats, err = dt.controlFD.WalkStat(ctx, state.names)
+		if err != nil {
+			return err
+		}
+	default:
+		panic("unknown dentry implementation")
 	}
 
 	i := -1
 	for d := state.popFront(); d != nil; d = state.popFront() {
 		i++
-		found := i < len(stats)
+		var found bool
+		switch state.start.impl.(type) {
+		case *lisafsDentry:
+			found = i < len(stats)
+		default:
+			panic("unknown dentry implementation")
+		}
 		if i == 0 && len(state.names[0]) == 0 {
 			if found && !d.isSynthetic() {
 				// First dentry is where the search is starting, just update attributes
 				// since it cannot be replaced.
-				d.updateMetadataFromStatLocked(&stats[i]) // +checklocksforce: acquired by lockAllMetadata.
+				switch dt := d.impl.(type) {
+				case *lisafsDentry:
+					dt.updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: acquired by lockAllMetadata.
+				default:
+					panic("unknown dentry implementation")
+				}
 			}
 			d.metadataMu.Unlock() // +checklocksforce: see above.
 			continue
 		}
 
+		var fileChanged bool
+		if found {
+			switch d.impl.(type) {
+			case *lisafsDentry:
+				fileChanged = d.inoKey != inoKeyFromStatx(&stats[i])
+			case nil:
+				// A remote file was found to replace this synthetic file.
+				fileChanged = true
+			default:
+				panic("unknown dentry implementation")
+			}
+		}
 		// Note that synthetic dentries will always fail this comparison check.
-		if !found || d.inoKey != inoKeyFromStat(&stats[i]) {
+		if !found || fileChanged {
 			d.metadataMu.Unlock() // +checklocksforce: see above.
 			if !found && d.isSynthetic() {
 				// We have a synthetic file, and no remote file has arisen to replace
@@ -306,7 +337,12 @@ func (fs *filesystem) revalidateHelper(ctx context.Context, vfsObj *vfs.VirtualF
 		}
 
 		// The file at this path hasn't changed. Just update cached metadata.
-		d.updateMetadataFromStatLocked(&stats[i]) // +checklocksforce: see above.
+		switch dt := d.impl.(type) {
+		case *lisafsDentry:
+			dt.updateMetadataFromStatxLocked(&stats[i]) // +checklocksforce: see above.
+		default:
+			panic("unknown dentry implementation")
+		}
 		d.metadataMu.Unlock()
 	}
 

pkg/sentry/fsimpl/gofer/save_restore.go

@@ -24,7 +24,6 @@ import (
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/fdnotifier"
 	"gvisor.dev/gvisor/pkg/hostarch"
-	"gvisor.dev/gvisor/pkg/lisafs"
 	"gvisor.dev/gvisor/pkg/refs"
 	"gvisor.dev/gvisor/pkg/safemem"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
@@ -108,14 +107,14 @@ func (d *dentry) prepareSaveRecursive(ctx context.Context) error {
 	if d.isRegularFile() && !d.cachedMetadataAuthoritative() {
 		// Get updated metadata for d in case we need to perform metadata
 		// validation during restore.
-		if err := d.updateMetadata(ctx, nil); err != nil {
+		if err := d.updateMetadata(ctx); err != nil {
 			return err
 		}
 	}
-	if d.readFDLisa.Ok() || d.writeFDLisa.Ok() {
+	if d.isReadHandleOk() || d.isWriteHandleOk() {
 		d.fs.savedDentryRW[d] = savedDentryRW{
-			read:  d.readFDLisa.Ok(),
-			write: d.writeFDLisa.Ok(),
+			read:  d.isReadHandleOk(),
+			write: d.isWriteHandleOk(),
 		}
 	}
 	d.dirMu.Lock()
@@ -179,11 +178,7 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest
 	fs.opts.fd = fd
 	fs.inoByKey = make(map[inoKey]uint64)
 
-	rootInode, err := fs.initClient(ctx)
-	if err != nil {
-		return err
-	}
-	if err := fs.root.restoreFile(ctx, &rootInode, &opts); err != nil {
+	if err := fs.restoreRoot(ctx, &opts); err != nil {
 		return err
 	}
 
@@ -223,90 +218,28 @@ func (fs *filesystem) CompleteRestore(ctx context.Context, opts vfs.CompleteRest
 	return nil
 }
 
-func (d *dentry) restoreFile(ctx context.Context, inode *lisafs.Inode, opts *vfs.CompleteRestoreOptions) error {
-	d.controlFDLisa = d.fs.client.NewFD(inode.ControlFD)
-
-	// Gofers do not preserve inoKey across checkpoint/restore, so:
-	//
-	//	- We must assume that the remote filesystem did not change in a way that
-	//		would invalidate dentries, since we can't revalidate dentries by
-	//		checking inoKey.
-	//
-	//	- We need to associate the new inoKey with the existing d.ino.
-	d.inoKey = inoKeyFromStat(&inode.Stat)
-	d.fs.inoMu.Lock()
-	d.fs.inoByKey[d.inoKey] = d.ino
-	d.fs.inoMu.Unlock()
-
-	// Check metadata stability before updating metadata.
-	d.metadataMu.Lock()
-	defer d.metadataMu.Unlock()
-	if d.isRegularFile() {
-		if opts.ValidateFileSizes {
-			if inode.Stat.Mask&linux.STATX_SIZE == 0 {
-				return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: file size not available", genericDebugPathname(d))}
-			}
-			if d.size.RacyLoad() != inode.Stat.Size {
-				return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: file size validation failed: size changed from %d to %d", genericDebugPathname(d), d.size.Load(), inode.Stat.Size)}
-			}
-		}
-		if opts.ValidateFileModificationTimestamps {
-			if inode.Stat.Mask&linux.STATX_MTIME != 0 {
-				return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime not available", genericDebugPathname(d))}
-			}
-			if want := dentryTimestamp(inode.Stat.Mtime); d.mtime.RacyLoad() != want {
-				return vfs.ErrCorruption{fmt.Errorf("gofer.dentry(%q).restoreFile: mtime validation failed: mtime changed from %+v to %+v", genericDebugPathname(d), linux.NsecToStatxTimestamp(d.mtime.RacyLoad()), linux.NsecToStatxTimestamp(want))}
-			}
-		}
-	}
-	if !d.cachedMetadataAuthoritative() {
-		d.updateMetadataFromStatLocked(&inode.Stat)
-	}
-
-	if rw, ok := d.fs.savedDentryRW[d]; ok {
-		if err := d.ensureSharedHandle(ctx, rw.read, rw.write, false /* trunc */); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // Preconditions: d is not synthetic.
 func (d *dentry) restoreDescendantsRecursive(ctx context.Context, opts *vfs.CompleteRestoreOptions) error {
 	for _, child := range d.children {
 		if child == nil {
 			continue
 		}
-		// child is synthetic if it does not exist in fs.syncableDentries.
-		if child.syncableListEntry.Next() == nil && child.syncableListEntry.Prev() == nil && d.fs.syncableDentries.Front() != &child.syncableListEntry {
+		if child.isSynthetic() {
 			continue
 		}
-		if err := child.restoreRecursive(ctx, opts); err != nil {
+		if err := child.restoreFile(ctx, opts); err != nil {
+			return err
+		}
+		if err := child.restoreDescendantsRecursive(ctx, opts); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-// Preconditions: d is not synthetic (but note that since this function
-// restores d.file, d.file.isNil() is always true at this point, so this can
-// only be detected by checking filesystem.syncableDentries). d.parent has been
-// restored.
-func (d *dentry) restoreRecursive(ctx context.Context, opts *vfs.CompleteRestoreOptions) error {
-	inode, err := d.parent.controlFDLisa.Walk(ctx, d.name)
-	if err != nil {
-		return err
-	}
-	if err := d.restoreFile(ctx, &inode, opts); err != nil {
-		return err
-	}
-	return d.restoreDescendantsRecursive(ctx, opts)
-}
-
 func (fd *specialFileFD) completeRestore(ctx context.Context) error {
 	d := fd.dentry()
-	h, err := openHandle(ctx, d.controlFDLisa, fd.vfsfd.IsReadable(), fd.vfsfd.IsWritable(), false /* trunc */)
+	h, err := d.openHandle(ctx, fd.vfsfd.IsReadable(), fd.vfsfd.IsWritable(), false /* trunc */)
 	if err != nil {
 		return err
 	}

pkg/sentry/fsimpl/gofer/socket.go

@@ -36,7 +36,8 @@ func (d *dentry) isSocket() bool {
 //
 // +stateify savable
 type endpoint struct {
-	// dentry is the filesystem dentry which produced this endpoint.
+	// dentry is the filesystem dentry which produced this endpoint. dentry is
+	// not synthetic.
 	dentry *dentry
 
 	// path is the sentry path where this endpoint is bound.
@@ -93,7 +94,7 @@ func (e *endpoint) UnidirectionalConnect(ctx context.Context) (transport.Connect
 }
 
 func (e *endpoint) newConnectedEndpoint(ctx context.Context, sockType linux.SockType, queue *waiter.Queue) (*transport.SCMConnectedEndpoint, *syserr.Error) {
-	hostSockFD, err := e.dentry.controlFDLisa.Connect(ctx, sockType)
+	hostSockFD, err := e.dentry.connect(ctx, sockType)
 	if err != nil {
 		return nil, syserr.ErrConnectionRefused
 	}

pkg/sentry/fsimpl/gofer/special_file.go

@@ -17,7 +17,6 @@ package gofer
 import (
 	"fmt"
 
-	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/atomicbitops"
 	"gvisor.dev/gvisor/pkg/context"
@@ -150,7 +149,7 @@ func (fd *specialFileFD) OnClose(ctx context.Context) error {
 	if !fd.vfsfd.IsWritable() {
 		return nil
 	}
-	return fd.handle.fdLisa.Flush(ctx)
+	return flush(ctx, fd.handle.fdLisa)
 }
 
 // Readiness implements waiter.Waitable.Readiness.
@@ -198,7 +197,7 @@ func (fd *specialFileFD) Allocate(ctx context.Context, mode, offset, length uint
 	if fd.isRegularFile {
 		d := fd.dentry()
 		return d.doAllocate(ctx, offset, length, func() error {
-			return fd.handle.fdLisa.Allocate(ctx, mode, offset, length)
+			return fd.handle.allocate(ctx, mode, offset, length)
 		})
 	}
 	return fd.FileDescriptionDefaultImpl.Allocate(ctx, mode, offset, length)
@@ -304,7 +303,7 @@ func (fd *specialFileFD) pwrite(ctx context.Context, src usermem.IOSequence, off
 		// size is updated. There is a possible race here if size is modified
 		// externally after metadata cache is updated.
 		if fd.vfsfd.StatusFlags()&linux.O_APPEND != 0 && !d.cachedMetadataAuthoritative() {
-			if err := d.updateMetadata(ctx, nil); err != nil {
+			if err := d.updateMetadata(ctx); err != nil {
 				return 0, offset, err
 			}
 		}
@@ -416,21 +415,7 @@ func (fd *specialFileFD) sync(ctx context.Context, forFilesystemSync bool) error
 	fd.releaseMu.RLock()
 	defer fd.releaseMu.RUnlock()
 
-	if !fd.handle.isOpen() {
-		return nil
-	}
-	err := func() error {
-		// If we have a host FD, fsyncing it is likely to be faster than an fsync
-		// RPC.
-		if fd.handle.fd >= 0 {
-			ctx.UninterruptibleSleepStart(false)
-			err := unix.Fsync(int(fd.handle.fd))
-			ctx.UninterruptibleSleepFinish(false)
-			return err
-		}
-		return fd.handle.fdLisa.Sync(ctx)
-	}()
-	if err != nil {
+	if err := fd.handle.sync(ctx); err != nil {
 		if !forFilesystemSync {
 			return err
 		}
@@ -533,3 +518,10 @@ func (fd *specialFileFD) requireHostFD() {
 		panic("gofer.specialFileFD can no longer be memory-mapped without a host FD")
 	}
 }
+
+func (fd *specialFileFD) updateMetadata(ctx context.Context) error {
+	d := fd.dentry()
+	d.metadataMu.Lock()
+	defer d.metadataMu.Unlock()
+	return d.updateMetadataLocked(ctx, fd.handle)
+}

pkg/sentry/fsimpl/gofer/symlink.go

@@ -35,7 +35,7 @@ func (d *dentry) readlink(ctx context.Context, mnt *vfs.Mount) (string, error) {
 			return target, nil
 		}
 	}
-	target, err := d.controlFDLisa.ReadLinkAt(ctx)
+	target, err := d.readlinkImpl(ctx)
 	if d.fs.opts.interop != InteropModeShared {
 		if err == nil {
 			d.haveTarget = true