From 119f9a18ce6cb247c503e9007ce989902b9e6eee Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 5 Jun 2015 13:48:41 +1000 Subject: [PATCH 1/8] Changed the guidance for dealing with lookup counts at unmount time. --- fuseops/ops.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index f8ff386..cd21c0e 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -237,7 +237,7 @@ func (o *SetInodeAttributesOp) toBazilfuseResponse() (bfResp interface{}) { // revalidating. // // In contrast to all other inodes, RootInodeID begins with an implicit -// reference count of one, without a corresponding op to increase it. (There +// lookup count of one, without a corresponding op to increase it. (There // could be no such op, because the root cannot be referred to by name.) Code // walk: // @@ -248,10 +248,10 @@ func (o *SetInodeAttributesOp) toBazilfuseResponse() (bfResp interface{}) { // // * (http://goo.gl/vPD9Oh) fuse_iget increments nlookup. // -// File systems should not make assumptions about whether they will or will not -// receive a ForgetInodeOp for the root inode. Experimentally, OS X seems to -// never send one, while Linux appears to send one only sometimes. (Cf. -// http://goo.gl/EUbxEg, fuse-devel thread "Root inode lookup count"). +// File systems should tolerate but not rely on receiving forget ops for +// remaining inodes when the file system unmounts, including the root inode. +// Rather they should take fuse.Connection.ReadOp returning io.EOF as +// implicitly decrementing all lookup counts to zero. type ForgetInodeOp struct { commonOp From c9c4a456b143d4b88756e0ad2f201415b595d8e3 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 5 Jun 2015 13:49:18 +1000 Subject: [PATCH 2/8] Removed the Linux destroy workaround. --- samples/forgetfs/forget_fs.go | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/samples/forgetfs/forget_fs.go b/samples/forgetfs/forget_fs.go index 997ff48..ecdee79 100644 --- a/samples/forgetfs/forget_fs.go +++ b/samples/forgetfs/forget_fs.go @@ -17,7 +17,6 @@ package forgetfs import ( "fmt" "os" - "runtime" "github.com/jacobsa/fuse" "github.com/jacobsa/fuse/fuseops" @@ -184,16 +183,6 @@ func (fs *fsImpl) Check() { fs.mu.Lock() defer fs.mu.Unlock() - // On Linux we often don't receive forget ops, and never receive destroy ops - // (cf. http://goo.gl/EUbxEg, fuse-devel thread "Root inode lookup count"). - // So there's not really much we can check here. - // - // TODO(jacobsa): Figure out why we don't receive destroy. If we can reliably - // receive it, we can treat it as "forget all". - if runtime.GOOS == "linux" { - return - } - for k, v := range fs.inodes { // Special case: we don't require the root inode to have reached zero. // OS X doesn't seem to send forgets for the root, and Linux only does From 6fff9f3f20e631d4c2d3027f1608a9c0d9f09a91 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 5 Jun 2015 13:51:27 +1000 Subject: [PATCH 3/8] Added FileSystem.Destroy. --- fuseutil/file_system.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fuseutil/file_system.go b/fuseutil/file_system.go index b8d894e..f1ed0a3 100644 --- a/fuseutil/file_system.go +++ b/fuseutil/file_system.go @@ -60,6 +60,11 @@ type FileSystem interface { FlushFile(*fuseops.FlushFileOp) error ReleaseFileHandle(*fuseops.ReleaseFileHandleOp) error ReadSymlink(*fuseops.ReadSymlinkOp) error + + // Regard all inodes (including the root inode) as having their lookup counts + // decremented to zero, and clean up any resources associated with the file + // system. No further calls to the file system will be made. + Destroy() } // Create a fuse.Server that handles ops by calling the associated FileSystem From 630f40de9404f47ce2343fef804623bfe149a186 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Fri, 5 Jun 2015 13:51:45 +1000 Subject: [PATCH 4/8] NotImplementedFileSystem.Destroy --- fuseutil/not_implemented_file_system.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fuseutil/not_implemented_file_system.go b/fuseutil/not_implemented_file_system.go index e4abb5f..9d12e21 100644 --- a/fuseutil/not_implemented_file_system.go +++ b/fuseutil/not_implemented_file_system.go @@ -147,3 +147,6 @@ func (fs *NotImplementedFileSystem) ReadSymlink( err = fuse.ENOSYS return } + +func (fs *NotImplementedFileSystem) Destroy() { +} From 9cc2a6f450e2a6e4f89eeca0780794bca6341f4c Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 9 Jun 2015 11:10:39 +1000 Subject: [PATCH 5/8] Implemented Destroy for forgetfs. --- samples/forgetfs/forget_fs.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/samples/forgetfs/forget_fs.go b/samples/forgetfs/forget_fs.go index ecdee79..d0b934b 100644 --- a/samples/forgetfs/forget_fs.go +++ b/samples/forgetfs/forget_fs.go @@ -164,6 +164,11 @@ func (in *inode) DecrementLookupCount(n uint64) { in.lookupCount -= n } +// Decrement the lookup count to zero. +func (in *inode) Destroy() { + in.DecrementLookupCount(in.lookupCount) +} + //////////////////////////////////////////////////////////////////////// // Helpers //////////////////////////////////////////////////////////////////////// @@ -372,3 +377,9 @@ func (fs *fsImpl) OpenDir( return } + +func (fs *fsImpl) Destroy() { + for _, in := range fs.inodes { + in.Destroy() + } +} From 0f62458e212999a0ea8e285dacd2e07d1400035d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 9 Jun 2015 11:11:52 +1000 Subject: [PATCH 6/8] Destroy when done. --- fuseutil/file_system.go | 7 ++++++- mounted_file_system.go | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fuseutil/file_system.go b/fuseutil/file_system.go index f1ed0a3..18510aa 100644 --- a/fuseutil/file_system.go +++ b/fuseutil/file_system.go @@ -90,7 +90,12 @@ type fileSystemServer struct { } func (s *fileSystemServer) ServeOps(c *fuse.Connection) { - defer s.opsInFlight.Wait() + // When we are done, we clean up by waiting for all in-flight ops then + // destroying the file system. + defer func() { + s.opsInFlight.Wait() + s.fs.Destroy() + }() for { op, err := c.ReadOp() diff --git a/mounted_file_system.go b/mounted_file_system.go index 4055a3d..9335cd9 100644 --- a/mounted_file_system.go +++ b/mounted_file_system.go @@ -27,7 +27,8 @@ import ( // A type that knows how to serve ops read from a connection. type Server interface { // Read and serve ops from the supplied connection until EOF. Do not return - // until all operations have been responded to. + // until all operations have been responded to. Must not be called more than + // once. ServeOps(*Connection) } From 43c8f9d8c2392deaa42309703f8a0d4636d24fd4 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 9 Jun 2015 13:25:28 +1000 Subject: [PATCH 7/8] Fixed a bug in forgetfs. This manifested as ForgetFSTest.MkDir pacicking now and then in a manner like this: https://gist.githubusercontent.com/jacobsa/2d12150beca1043f71b0/raw/28e0a01a9276799ad820970a05b522367222c331/gistfile1.txt --- samples/forgetfs/forget_fs.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/samples/forgetfs/forget_fs.go b/samples/forgetfs/forget_fs.go index d0b934b..eae5c4a 100644 --- a/samples/forgetfs/forget_fs.go +++ b/samples/forgetfs/forget_fs.go @@ -66,6 +66,13 @@ func NewFileSystem() (fs *ForgetFS) { // The root inode starts with a lookup count of one. impl.inodes[cannedID_Root].IncrementLookupCount() + // The canned inodes are supposed to be stable from the user's point of view, + // so we should allow them to be looked up at any point even if the kernel + // has balanced its lookups with its forgets. Ensure that they never go to + // zero until the file system is destroyed. + impl.inodes[cannedID_Foo].IncrementLookupCount() + impl.inodes[cannedID_Bar].IncrementLookupCount() + // Set up the mutex. impl.mu = syncutil.NewInvariantMutex(impl.checkInvariants) From 3cb67f5833d801141c29ee9947a021069256f0f3 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 9 Jun 2015 13:27:02 +1000 Subject: [PATCH 8/8] Tightened forgetfs's post-destruction check, now that we can. --- samples/forgetfs/forget_fs.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/samples/forgetfs/forget_fs.go b/samples/forgetfs/forget_fs.go index eae5c4a..52ead10 100644 --- a/samples/forgetfs/forget_fs.go +++ b/samples/forgetfs/forget_fs.go @@ -196,19 +196,6 @@ func (fs *fsImpl) Check() { defer fs.mu.Unlock() for k, v := range fs.inodes { - // Special case: we don't require the root inode to have reached zero. - // OS X doesn't seem to send forgets for the root, and Linux only does - // sometimes. But we want to make sure it's not greater than one, which - // would be weird. - if k == fuseops.RootInodeID { - if v.lookupCount > 1 { - panic(fmt.Sprintf("Root has lookup count %v", v.lookupCount)) - } - - continue - } - - // Check other inodes. if v.lookupCount != 0 { panic(fmt.Sprintf("Inode %v has lookup count %v", k, v.lookupCount)) }