From 718f4a898329a3e4ae6d21b01db6887bbdf1e0e8 Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Thu, 4 Jul 2024 08:44:07 +0000 Subject: [PATCH 1/9] Able to reproduce the hang --- internal/fs/fs.go | 23 +++++++++++++++++++- internal/fs/kernel_list_cache_test.go | 30 +++++++++++++++++++++++++-- 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 7c6b76915a..8c1de50304 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -774,6 +774,7 @@ func (fs *fileSystem) mintInode(ic inode.Core) (in inode.Inode) { // LOCK_FUNCTION(in) func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Inode) { + logger.Info("Entering lookUpOrCreateInodeIfNotStale: ", ic.FullName) // Sanity check. if err := ic.SanityCheck(); err != nil { panic(err.Error()) @@ -836,6 +837,8 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino return } + logger.Info("Calling lookUpOrCreateInodeIfNotStale: Midway") + oGen := inode.Generation{ Object: ic.MinObject.Generation, Metadata: ic.MinObject.MetaGeneration, @@ -852,6 +855,7 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino in = fs.mintInode(ic) fs.generationBackedInodes[in.Name()] = in.(inode.GenerationBackedInode) + logger.Info("Minting") in.Lock() return } @@ -861,6 +865,8 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino // acquiring the file system lock, so drop it while acquiring the inode's // lock, then reacquire. fs.mu.Unlock() + logger.Info("Sleeping for 5 sec") + time.Sleep(time.Second * 5) existingInode.Lock() fs.mu.Lock() @@ -916,6 +922,8 @@ func (fs *fileSystem) lookUpOrCreateChildInode( ctx context.Context, parent inode.DirInode, childName string) (child inode.Inode, err error) { + logger.Info("Entering lookUpOrCreateChildInode: ", childName) + // First check if the requested child is a localFileInode. child = fs.lookUpLocalFileInode(parent, childName) if child != nil { @@ -948,11 +956,14 @@ func (fs *fileSystem) lookUpOrCreateChildInode( core, err = getLookupResult() if err != nil { + logger.Info("Returning because of error") + return } if core == nil { err = fuse.ENOENT + logger.Info("Returning because of nil core for: ", childName) return } @@ -1325,6 +1336,7 @@ func (fs *fileSystem) StatFS( func (fs *fileSystem) LookUpInode( ctx context.Context, op *fuseops.LookUpInodeOp) (err error) { + logger.Infof("Entering LookUpInode for ID: %d, %s", op.OpContext.FuseID, op.Name) if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2137,6 +2149,9 @@ func (fs *fileSystem) Unlink( func (fs *fileSystem) OpenDir( ctx context.Context, op *fuseops.OpenDirOp) (err error) { + logger.Info("Entrying for openDir call...: ", op.Inode) + logger.Info("sleeping for 4 sec") + time.Sleep(time.Second * 4) fs.mu.Lock() defer fs.mu.Unlock() @@ -2154,10 +2169,16 @@ func (fs *fileSystem) OpenDir( // Enables kernel list-cache in case of non-zero kernelListCacheTTL. if fs.kernelListCacheTTL > 0 { - + logger.Info("sleeping for 3 sec") + time.Sleep(time.Second * 3) + // Following lock ordering rules to get the dirInode lock. First get inode + // lock and then fs lock. + fs.mu.Unlock() // Taking RLock() since ShouldInvalidateKernelListCache only reads the DirInode // properties, no modification. in.RLock() + // Acquiring fs lock early to use common defer function. + fs.mu.Lock() // Invalidates the kernel list-cache once the last cached response is out of // kernelListCacheTTL. op.KeepCache = !in.ShouldInvalidateKernelListCache(fs.kernelListCacheTTL) diff --git a/internal/fs/kernel_list_cache_test.go b/internal/fs/kernel_list_cache_test.go index 34be04cd57..20e81cac99 100644 --- a/internal/fs/kernel_list_cache_test.go +++ b/internal/fs/kernel_list_cache_test.go @@ -26,6 +26,7 @@ package fs_test import ( "os" "path" + "sync" "testing" "time" @@ -52,8 +53,8 @@ func (t *KernelListCacheTestCommon) SetupTest() { func (t *KernelListCacheTestCommon) TearDownTest() { cacheClock.AdvanceTime(util.MaxTimeDuration) - t.deleteFilesAndDirStructureInBucket() - t.fsTest.TearDown() + //t.deleteFilesAndDirStructureInBucket() + //t.fsTest.TearDown() } func (t *KernelListCacheTestCommon) TearDownSuite() { @@ -120,6 +121,31 @@ func TestKernelListCacheTestWithPositiveTtlSuite(t *testing.T) { suite.Run(t, new(KernelListCacheTestWithPositiveTtl)) } +// (a) First ReadDir() will be served from GCSFuse filesystem. +// (b) Second ReadDir() within ttl will be served from kernel page cache. +func (t *KernelListCacheTestWithPositiveTtl) Test_Debug_HangIssue() { + // Make sure, inode is created. + os.Stat(path.Join(mntDir, "explicitDir")) + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + time.Sleep(6 * time.Second) + os.Stat(path.Join(mntDir, "explicitDir")) + }() + + wg.Add(1) + go func() { + defer wg.Done() + f, _ := os.Open(path.Join(mntDir, "explicitDir")) + f.Close() + }() + + wg.Wait() + +} + // (a) First ReadDir() will be served from GCSFuse filesystem. // (b) Second ReadDir() within ttl will be served from kernel page cache. func (t *KernelListCacheTestWithPositiveTtl) TestKernelListCache_CacheHit() { From 45706ce8bc8fe5e7854a8c18ce2322cacc84a4bf Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Thu, 4 Jul 2024 09:32:32 +0000 Subject: [PATCH 2/9] Adding composite test for deadlock case --- internal/fs/fs.go | 26 ++++------------- internal/fs/kernel_list_cache_test.go | 40 +++++++++++++++++---------- 2 files changed, 30 insertions(+), 36 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 8c1de50304..8ffa3e10a8 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -773,8 +773,6 @@ func (fs *fileSystem) mintInode(ic inode.Core) (in inode.Inode) { // UNLOCK_FUNCTION(fs.mu) // LOCK_FUNCTION(in) func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Inode) { - - logger.Info("Entering lookUpOrCreateInodeIfNotStale: ", ic.FullName) // Sanity check. if err := ic.SanityCheck(); err != nil { panic(err.Error()) @@ -837,8 +835,6 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino return } - logger.Info("Calling lookUpOrCreateInodeIfNotStale: Midway") - oGen := inode.Generation{ Object: ic.MinObject.Generation, Metadata: ic.MinObject.MetaGeneration, @@ -855,7 +851,6 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino in = fs.mintInode(ic) fs.generationBackedInodes[in.Name()] = in.(inode.GenerationBackedInode) - logger.Info("Minting") in.Lock() return } @@ -865,8 +860,6 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino // acquiring the file system lock, so drop it while acquiring the inode's // lock, then reacquire. fs.mu.Unlock() - logger.Info("Sleeping for 5 sec") - time.Sleep(time.Second * 5) existingInode.Lock() fs.mu.Lock() @@ -922,8 +915,6 @@ func (fs *fileSystem) lookUpOrCreateChildInode( ctx context.Context, parent inode.DirInode, childName string) (child inode.Inode, err error) { - logger.Info("Entering lookUpOrCreateChildInode: ", childName) - // First check if the requested child is a localFileInode. child = fs.lookUpLocalFileInode(parent, childName) if child != nil { @@ -956,14 +947,11 @@ func (fs *fileSystem) lookUpOrCreateChildInode( core, err = getLookupResult() if err != nil { - logger.Info("Returning because of error") - return } if core == nil { err = fuse.ENOENT - logger.Info("Returning because of nil core for: ", childName) return } @@ -1336,7 +1324,6 @@ func (fs *fileSystem) StatFS( func (fs *fileSystem) LookUpInode( ctx context.Context, op *fuseops.LookUpInodeOp) (err error) { - logger.Infof("Entering LookUpInode for ID: %d, %s", op.OpContext.FuseID, op.Name) if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2149,9 +2136,6 @@ func (fs *fileSystem) Unlink( func (fs *fileSystem) OpenDir( ctx context.Context, op *fuseops.OpenDirOp) (err error) { - logger.Info("Entrying for openDir call...: ", op.Inode) - logger.Info("sleeping for 4 sec") - time.Sleep(time.Second * 4) fs.mu.Lock() defer fs.mu.Unlock() @@ -2169,16 +2153,16 @@ func (fs *fileSystem) OpenDir( // Enables kernel list-cache in case of non-zero kernelListCacheTTL. if fs.kernelListCacheTTL > 0 { - logger.Info("sleeping for 3 sec") - time.Sleep(time.Second * 3) - // Following lock ordering rules to get the dirInode lock. First get inode - // lock and then fs lock. + // Following lock ordering rules: first taking dirInode.lock() and then fs.lock(). fs.mu.Unlock() + // Taking RLock() since ShouldInvalidateKernelListCache only reads the DirInode // properties, no modification. in.RLock() - // Acquiring fs lock early to use common defer function. + + // Acquiring fs lock use common defer function. fs.mu.Lock() + // Invalidates the kernel list-cache once the last cached response is out of // kernelListCacheTTL. op.KeepCache = !in.ShouldInvalidateKernelListCache(fs.kernelListCacheTTL) diff --git a/internal/fs/kernel_list_cache_test.go b/internal/fs/kernel_list_cache_test.go index 20e81cac99..b17597c702 100644 --- a/internal/fs/kernel_list_cache_test.go +++ b/internal/fs/kernel_list_cache_test.go @@ -121,29 +121,39 @@ func TestKernelListCacheTestWithPositiveTtlSuite(t *testing.T) { suite.Run(t, new(KernelListCacheTestWithPositiveTtl)) } -// (a) First ReadDir() will be served from GCSFuse filesystem. -// (b) Second ReadDir() within ttl will be served from kernel page cache. -func (t *KernelListCacheTestWithPositiveTtl) Test_Debug_HangIssue() { - // Make sure, inode is created. - os.Stat(path.Join(mntDir, "explicitDir")) - +// Test_Parallel_OpenDirAndLookUpInode detects deadlock if openDir and LookUpInode +// is happening concurrently. +func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_OpenDirAndLookUpInode() { var wg sync.WaitGroup - wg.Add(1) + wg.Add(2) + // Fail if the operation takes more than timeout. + timeout := 5 * time.Second + go func() { defer wg.Done() - time.Sleep(6 * time.Second) - os.Stat(path.Join(mntDir, "explicitDir")) + for i := 0; i < 100; i++ { + f, _ := os.Open(path.Join(mntDir, "explicitDir")) + f.Close() + } }() - - wg.Add(1) go func() { defer wg.Done() - f, _ := os.Open(path.Join(mntDir, "explicitDir")) - f.Close() + for i := 0; i < 100; i++ { + os.Stat(path.Join(mntDir, "explicitDir")) + } }() - wg.Wait() - + done := make(chan bool, 1) + go func() { + wg.Wait() + done <- true + }() + select { + case <-done: + // Operation completed successfully before timeout. + case <-time.After(timeout): + assert.FailNow(t.T(), "Possible deadlock") + } } // (a) First ReadDir() will be served from GCSFuse filesystem. From 8fec64586ed1f338acd599a62e21cef53dfab6de Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Thu, 4 Jul 2024 09:41:13 +0000 Subject: [PATCH 3/9] Fixing lint issue --- internal/fs/kernel_list_cache_test.go | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/internal/fs/kernel_list_cache_test.go b/internal/fs/kernel_list_cache_test.go index b17597c702..e9fbcb44fe 100644 --- a/internal/fs/kernel_list_cache_test.go +++ b/internal/fs/kernel_list_cache_test.go @@ -53,8 +53,8 @@ func (t *KernelListCacheTestCommon) SetupTest() { func (t *KernelListCacheTestCommon) TearDownTest() { cacheClock.AdvanceTime(util.MaxTimeDuration) - //t.deleteFilesAndDirStructureInBucket() - //t.fsTest.TearDown() + t.deleteFilesAndDirStructureInBucket() + t.fsTest.TearDown() } func (t *KernelListCacheTestCommon) TearDownSuite() { @@ -121,8 +121,8 @@ func TestKernelListCacheTestWithPositiveTtlSuite(t *testing.T) { suite.Run(t, new(KernelListCacheTestWithPositiveTtl)) } -// Test_Parallel_OpenDirAndLookUpInode detects deadlock if openDir and LookUpInode -// is happening concurrently. +// Test_Parallel_OpenDirAndLookUpInode helps in detecting the deadlock when +// OpenDir() and LookUpInode() request for same directory comes in parallel. func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_OpenDirAndLookUpInode() { var wg sync.WaitGroup wg.Add(2) @@ -132,14 +132,18 @@ func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_OpenDirAndLookUpInode go func() { defer wg.Done() for i := 0; i < 100; i++ { - f, _ := os.Open(path.Join(mntDir, "explicitDir")) - f.Close() + f, err := os.Open(path.Join(mntDir, "explicitDir")) + assert.Nil(t.T(), err) + + err = f.Close() + assert.Nil(t.T(), err) } }() go func() { defer wg.Done() for i := 0; i < 100; i++ { - os.Stat(path.Join(mntDir, "explicitDir")) + _, err := os.Stat(path.Join(mntDir, "explicitDir")) + assert.Nil(t.T(), err) } }() From f8152c98e8108f70f7aa34e0f1e7b4d73c164514 Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Thu, 4 Jul 2024 09:43:35 +0000 Subject: [PATCH 4/9] reverting minor line deletion --- internal/fs/fs.go | 199 +++++++++++++++++++++++----------------------- 1 file changed, 100 insertions(+), 99 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 8ffa3e10a8..1814f4f32b 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -128,8 +128,8 @@ type ServerConfig struct { // Create a fuse file system server according to the supplied configuration. func NewFileSystem( - ctx context.Context, - cfg *ServerConfig) (fuseutil.FileSystem, error) { + ctx context.Context, + cfg *ServerConfig) (fuseutil.FileSystem, error) { // Check permissions bits. if cfg.FilePerms&^os.ModePerm != 0 { return nil, fmt.Errorf("Illegal file perms: %v", cfg.FilePerms) @@ -248,9 +248,9 @@ func createFileCacheHandler(cfg *ServerConfig) (fileCacheHandler *file.CacheHand } func makeRootForBucket( - ctx context.Context, - fs *fileSystem, - syncerBucket gcsx.SyncerBucket) inode.DirInode { + ctx context.Context, + fs *fileSystem, + syncerBucket gcsx.SyncerBucket) inode.DirInode { return inode.NewDirInode( fuseops.RootInodeID, inode.NewRootName(""), @@ -773,6 +773,7 @@ func (fs *fileSystem) mintInode(ic inode.Core) (in inode.Inode) { // UNLOCK_FUNCTION(fs.mu) // LOCK_FUNCTION(in) func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Inode) { + // Sanity check. if err := ic.SanityCheck(); err != nil { panic(err.Error()) @@ -912,9 +913,9 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino // LOCKS_EXCLUDED(parent) // LOCK_FUNCTION(child) func (fs *fileSystem) lookUpOrCreateChildInode( - ctx context.Context, - parent inode.DirInode, - childName string) (child inode.Inode, err error) { + ctx context.Context, + parent inode.DirInode, + childName string) (child inode.Inode, err error) { // First check if the requested child is a localFileInode. child = fs.lookUpLocalFileInode(parent, childName) if child != nil { @@ -1038,9 +1039,9 @@ func (fs *fileSystem) lookUpLocalFileInode(parent inode.DirInode, childName stri // LOCKS_EXCLUDED(parent) // LOCK_FUNCTION(child) func (fs *fileSystem) lookUpOrCreateChildDirInode( - ctx context.Context, - parent inode.DirInode, - childName string) (child inode.BucketOwnedDirInode, err error) { + ctx context.Context, + parent inode.DirInode, + childName string) (child inode.BucketOwnedDirInode, err error) { in, err := fs.lookUpOrCreateChildInode(ctx, parent, childName) if err != nil { return nil, fmt.Errorf("lookup or create %q: %w", childName, err) @@ -1059,8 +1060,8 @@ func (fs *fileSystem) lookUpOrCreateChildDirInode( // LOCKS_EXCLUDED(fs.mu) // LOCKS_REQUIRED(f) func (fs *fileSystem) syncFile( - ctx context.Context, - f *inode.FileInode) (err error) { + ctx context.Context, + f *inode.FileInode) (err error) { // SyncFile can be triggered for unlinked files if the fileHandle is open by // same or another user. Silently ignore the syncFile call. // This is in sync with non-local file behaviour. @@ -1171,8 +1172,8 @@ func (fs *fileSystem) unlockAndDecrementLookupCount(in inode.Inode, N uint64) { // LOCKS_EXCLUDED(fs.mu) // UNLOCK_FUNCTION(in) func (fs *fileSystem) unlockAndMaybeDisposeOfInode( - in inode.Inode, - err *error) { + in inode.Inode, + err *error) { // If there is no error, just unlock. if *err == nil { in.Unlock() @@ -1188,11 +1189,11 @@ func (fs *fileSystem) unlockAndMaybeDisposeOfInode( // // LOCKS_REQUIRED(in) func (fs *fileSystem) getAttributes( - ctx context.Context, - in inode.Inode) ( - attr fuseops.InodeAttributes, - expiration time.Time, - err error) { + ctx context.Context, + in inode.Inode) ( + attr fuseops.InodeAttributes, + expiration time.Time, + err error) { // Call through. attr, err = in.Attributes(ctx) if err != nil { @@ -1253,7 +1254,7 @@ func (fs *fileSystem) fileInodeOrDie(id fuseops.InodeID) (in *inode.FileInode) { // // LOCKS_REQUIRED(fs.mu) func (fs *fileSystem) symlinkInodeOrDie( - id fuseops.InodeID) (in *inode.SymlinkInode) { + id fuseops.InodeID) (in *inode.SymlinkInode) { tmp := fs.inodes[id] in, ok := tmp.(*inode.SymlinkInode) if !ok { @@ -1299,8 +1300,8 @@ func (fs *fileSystem) Destroy() { } func (fs *fileSystem) StatFS( - ctx context.Context, - op *fuseops.StatFSOp) (err error) { + ctx context.Context, + op *fuseops.StatFSOp) (err error) { // Simulate a large amount of free space so that the Finder doesn't refuse to // copy in files. (See issue #125.) Use 2^17 as the block size because that // is the largest that OS X will pass on. @@ -1322,8 +1323,8 @@ func (fs *fileSystem) StatFS( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) LookUpInode( - ctx context.Context, - op *fuseops.LookUpInodeOp) (err error) { + ctx context.Context, + op *fuseops.LookUpInodeOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1358,8 +1359,8 @@ func (fs *fileSystem) LookUpInode( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) GetInodeAttributes( - ctx context.Context, - op *fuseops.GetInodeAttributesOp) (err error) { + ctx context.Context, + op *fuseops.GetInodeAttributesOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1386,8 +1387,8 @@ func (fs *fileSystem) GetInodeAttributes( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) SetInodeAttributes( - ctx context.Context, - op *fuseops.SetInodeAttributesOp) (err error) { + ctx context.Context, + op *fuseops.SetInodeAttributesOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1436,8 +1437,8 @@ func (fs *fileSystem) SetInodeAttributes( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ForgetInode( - ctx context.Context, - op *fuseops.ForgetInodeOp) (err error) { + ctx context.Context, + op *fuseops.ForgetInodeOp) (err error) { // Find the inode. fs.mu.Lock() in := fs.inodeOrDie(op.Inode) @@ -1452,8 +1453,8 @@ func (fs *fileSystem) ForgetInode( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) MkDir( - ctx context.Context, - op *fuseops.MkDirOp) (err error) { + ctx context.Context, + op *fuseops.MkDirOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1511,8 +1512,8 @@ func (fs *fileSystem) MkDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) MkNode( - ctx context.Context, - op *fuseops.MkNodeOp) (err error) { + ctx context.Context, + op *fuseops.MkNodeOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1551,10 +1552,10 @@ func (fs *fileSystem) MkNode( // LOCKS_EXCLUDED(fs.mu) // LOCK_FUNCTION(child) func (fs *fileSystem) createFile( - ctx context.Context, - parentID fuseops.InodeID, - name string, - mode os.FileMode) (child inode.Inode, err error) { + ctx context.Context, + parentID fuseops.InodeID, + name string, + mode os.FileMode) (child inode.Inode, err error) { // Find the parent. fs.mu.Lock() parent := fs.dirInodeOrDie(parentID) @@ -1596,8 +1597,8 @@ func (fs *fileSystem) createFile( // UNLOCK_FUNCTION(fs.mu) // LOCK_FUNCTION(in) func (fs *fileSystem) createLocalFile( - parentID fuseops.InodeID, - name string) (child inode.Inode, err error) { + parentID fuseops.InodeID, + name string) (child inode.Inode, err error) { // Find the parent. fs.mu.Lock() parent := fs.dirInodeOrDie(parentID) @@ -1641,8 +1642,8 @@ func (fs *fileSystem) createLocalFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) CreateFile( - ctx context.Context, - op *fuseops.CreateFileOp) (err error) { + ctx context.Context, + op *fuseops.CreateFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1690,8 +1691,8 @@ func (fs *fileSystem) CreateFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) CreateSymlink( - ctx context.Context, - op *fuseops.CreateSymlinkOp) (err error) { + ctx context.Context, + op *fuseops.CreateSymlinkOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1748,20 +1749,20 @@ func (fs *fileSystem) CreateSymlink( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) RmDir( - // When rm -r or os.RemoveAll call is made, the following calls are made in order - // 1. RmDir (only in the case of os.RemoveAll) - // 2. Unlink all nested files, - // 3. lookupInode call on implicit directory - // 4. Rmdir on the directory. - // - // When type cache ttl is set, we construct an implicitDir even though one doesn't - // exist on GCS (https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/internal/fs/inode/dir.go#L452), - // and thus, we get rmDir call to GCSFuse. - // Whereas when ttl is zero, lookupInode call itself fails and RmDir is not called - // because object is not present in GCS. - - ctx context.Context, - op *fuseops.RmDirOp) (err error) { +// When rm -r or os.RemoveAll call is made, the following calls are made in order +// 1. RmDir (only in the case of os.RemoveAll) +// 2. Unlink all nested files, +// 3. lookupInode call on implicit directory +// 4. Rmdir on the directory. +// +// When type cache ttl is set, we construct an implicitDir even though one doesn't +// exist on GCS (https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/internal/fs/inode/dir.go#L452), +// and thus, we get rmDir call to GCSFuse. +// Whereas when ttl is zero, lookupInode call itself fails and RmDir is not called +// because object is not present in GCS. + + ctx context.Context, + op *fuseops.RmDirOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1862,8 +1863,8 @@ func (fs *fileSystem) RmDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) Rename( - ctx context.Context, - op *fuseops.RenameOp) (err error) { + ctx context.Context, + op *fuseops.RenameOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1922,12 +1923,12 @@ func (fs *fileSystem) Rename( // LOCKS_EXCLUDED(oldParent) // LOCKS_EXCLUDED(newParent) func (fs *fileSystem) renameFile( - ctx context.Context, - oldParent inode.DirInode, - oldName string, - oldObject *gcs.MinObject, - newParent inode.DirInode, - newFileName string) error { + ctx context.Context, + oldParent inode.DirInode, + oldName string, + oldObject *gcs.MinObject, + newParent inode.DirInode, + newFileName string) error { // Clone into the new location. newParent.Lock() _, err := newParent.CloneToChildFile(ctx, newFileName, oldObject) @@ -1968,11 +1969,11 @@ func (fs *fileSystem) renameFile( // LOCKS_EXCLUDED(oldParent) // LOCKS_EXCLUDED(newParent) func (fs *fileSystem) renameDir( - ctx context.Context, - oldParent inode.DirInode, - oldName string, - newParent inode.DirInode, - newName string) error { + ctx context.Context, + oldParent inode.DirInode, + oldName string, + newParent inode.DirInode, + newName string) error { // Set up a function that throws away the lookup count increment from // lookUpOrCreateChildInode (since the pending inodes are not sent back to @@ -2081,8 +2082,8 @@ func (fs *fileSystem) renameDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) Unlink( - ctx context.Context, - op *fuseops.UnlinkOp) (err error) { + ctx context.Context, + op *fuseops.UnlinkOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2117,7 +2118,7 @@ func (fs *fileSystem) Unlink( err = parent.DeleteChildFile( ctx, op.Name, - 0, // Latest generation + 0, // Latest generation nil) // No meta-generation precondition if err != nil { @@ -2134,8 +2135,8 @@ func (fs *fileSystem) Unlink( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) OpenDir( - ctx context.Context, - op *fuseops.OpenDirOp) (err error) { + ctx context.Context, + op *fuseops.OpenDirOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2175,8 +2176,8 @@ func (fs *fileSystem) OpenDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReadDir( - ctx context.Context, - op *fuseops.ReadDirOp) (err error) { + ctx context.Context, + op *fuseops.ReadDirOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2205,8 +2206,8 @@ func (fs *fileSystem) ReadDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReleaseDirHandle( - ctx context.Context, - op *fuseops.ReleaseDirHandleOp) (err error) { + ctx context.Context, + op *fuseops.ReleaseDirHandleOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2221,8 +2222,8 @@ func (fs *fileSystem) ReleaseDirHandle( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) OpenFile( - ctx context.Context, - op *fuseops.OpenFileOp) (err error) { + ctx context.Context, + op *fuseops.OpenFileOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2247,8 +2248,8 @@ func (fs *fileSystem) OpenFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReadFile( - ctx context.Context, - op *fuseops.ReadFileOp) (err error) { + ctx context.Context, + op *fuseops.ReadFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2280,8 +2281,8 @@ func (fs *fileSystem) ReadFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReadSymlink( - ctx context.Context, - op *fuseops.ReadSymlinkOp) (err error) { + ctx context.Context, + op *fuseops.ReadSymlinkOp) (err error) { // Find the inode. fs.mu.Lock() in := fs.symlinkInodeOrDie(op.Inode) @@ -2298,8 +2299,8 @@ func (fs *fileSystem) ReadSymlink( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) WriteFile( - ctx context.Context, - op *fuseops.WriteFileOp) (err error) { + ctx context.Context, + op *fuseops.WriteFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2325,8 +2326,8 @@ func (fs *fileSystem) WriteFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) SyncFile( - ctx context.Context, - op *fuseops.SyncFileOp) (err error) { + ctx context.Context, + op *fuseops.SyncFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2358,8 +2359,8 @@ func (fs *fileSystem) SyncFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) FlushFile( - ctx context.Context, - op *fuseops.FlushFileOp) (err error) { + ctx context.Context, + op *fuseops.FlushFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2385,8 +2386,8 @@ func (fs *fileSystem) FlushFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReleaseFileHandle( - ctx context.Context, - op *fuseops.ReleaseFileHandleOp) (err error) { + ctx context.Context, + op *fuseops.ReleaseFileHandleOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2400,13 +2401,13 @@ func (fs *fileSystem) ReleaseFileHandle( } func (fs *fileSystem) GetXattr( - ctx context.Context, - op *fuseops.GetXattrOp) (err error) { + ctx context.Context, + op *fuseops.GetXattrOp) (err error) { return syscall.ENOSYS } func (fs *fileSystem) ListXattr( - ctx context.Context, - op *fuseops.ListXattrOp) error { + ctx context.Context, + op *fuseops.ListXattrOp) error { return syscall.ENOSYS } From 395cedcf798167e98604f247322ecbcb75a4d70f Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Thu, 4 Jul 2024 09:44:29 +0000 Subject: [PATCH 5/9] go import --- internal/fs/fs.go | 198 +++++++++++++++++++++++----------------------- 1 file changed, 99 insertions(+), 99 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 1814f4f32b..7c1b2e92d3 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -128,8 +128,8 @@ type ServerConfig struct { // Create a fuse file system server according to the supplied configuration. func NewFileSystem( - ctx context.Context, - cfg *ServerConfig) (fuseutil.FileSystem, error) { + ctx context.Context, + cfg *ServerConfig) (fuseutil.FileSystem, error) { // Check permissions bits. if cfg.FilePerms&^os.ModePerm != 0 { return nil, fmt.Errorf("Illegal file perms: %v", cfg.FilePerms) @@ -248,9 +248,9 @@ func createFileCacheHandler(cfg *ServerConfig) (fileCacheHandler *file.CacheHand } func makeRootForBucket( - ctx context.Context, - fs *fileSystem, - syncerBucket gcsx.SyncerBucket) inode.DirInode { + ctx context.Context, + fs *fileSystem, + syncerBucket gcsx.SyncerBucket) inode.DirInode { return inode.NewDirInode( fuseops.RootInodeID, inode.NewRootName(""), @@ -913,9 +913,9 @@ func (fs *fileSystem) lookUpOrCreateInodeIfNotStale(ic inode.Core) (in inode.Ino // LOCKS_EXCLUDED(parent) // LOCK_FUNCTION(child) func (fs *fileSystem) lookUpOrCreateChildInode( - ctx context.Context, - parent inode.DirInode, - childName string) (child inode.Inode, err error) { + ctx context.Context, + parent inode.DirInode, + childName string) (child inode.Inode, err error) { // First check if the requested child is a localFileInode. child = fs.lookUpLocalFileInode(parent, childName) if child != nil { @@ -1039,9 +1039,9 @@ func (fs *fileSystem) lookUpLocalFileInode(parent inode.DirInode, childName stri // LOCKS_EXCLUDED(parent) // LOCK_FUNCTION(child) func (fs *fileSystem) lookUpOrCreateChildDirInode( - ctx context.Context, - parent inode.DirInode, - childName string) (child inode.BucketOwnedDirInode, err error) { + ctx context.Context, + parent inode.DirInode, + childName string) (child inode.BucketOwnedDirInode, err error) { in, err := fs.lookUpOrCreateChildInode(ctx, parent, childName) if err != nil { return nil, fmt.Errorf("lookup or create %q: %w", childName, err) @@ -1060,8 +1060,8 @@ func (fs *fileSystem) lookUpOrCreateChildDirInode( // LOCKS_EXCLUDED(fs.mu) // LOCKS_REQUIRED(f) func (fs *fileSystem) syncFile( - ctx context.Context, - f *inode.FileInode) (err error) { + ctx context.Context, + f *inode.FileInode) (err error) { // SyncFile can be triggered for unlinked files if the fileHandle is open by // same or another user. Silently ignore the syncFile call. // This is in sync with non-local file behaviour. @@ -1172,8 +1172,8 @@ func (fs *fileSystem) unlockAndDecrementLookupCount(in inode.Inode, N uint64) { // LOCKS_EXCLUDED(fs.mu) // UNLOCK_FUNCTION(in) func (fs *fileSystem) unlockAndMaybeDisposeOfInode( - in inode.Inode, - err *error) { + in inode.Inode, + err *error) { // If there is no error, just unlock. if *err == nil { in.Unlock() @@ -1189,11 +1189,11 @@ func (fs *fileSystem) unlockAndMaybeDisposeOfInode( // // LOCKS_REQUIRED(in) func (fs *fileSystem) getAttributes( - ctx context.Context, - in inode.Inode) ( - attr fuseops.InodeAttributes, - expiration time.Time, - err error) { + ctx context.Context, + in inode.Inode) ( + attr fuseops.InodeAttributes, + expiration time.Time, + err error) { // Call through. attr, err = in.Attributes(ctx) if err != nil { @@ -1254,7 +1254,7 @@ func (fs *fileSystem) fileInodeOrDie(id fuseops.InodeID) (in *inode.FileInode) { // // LOCKS_REQUIRED(fs.mu) func (fs *fileSystem) symlinkInodeOrDie( - id fuseops.InodeID) (in *inode.SymlinkInode) { + id fuseops.InodeID) (in *inode.SymlinkInode) { tmp := fs.inodes[id] in, ok := tmp.(*inode.SymlinkInode) if !ok { @@ -1300,8 +1300,8 @@ func (fs *fileSystem) Destroy() { } func (fs *fileSystem) StatFS( - ctx context.Context, - op *fuseops.StatFSOp) (err error) { + ctx context.Context, + op *fuseops.StatFSOp) (err error) { // Simulate a large amount of free space so that the Finder doesn't refuse to // copy in files. (See issue #125.) Use 2^17 as the block size because that // is the largest that OS X will pass on. @@ -1323,8 +1323,8 @@ func (fs *fileSystem) StatFS( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) LookUpInode( - ctx context.Context, - op *fuseops.LookUpInodeOp) (err error) { + ctx context.Context, + op *fuseops.LookUpInodeOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1359,8 +1359,8 @@ func (fs *fileSystem) LookUpInode( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) GetInodeAttributes( - ctx context.Context, - op *fuseops.GetInodeAttributesOp) (err error) { + ctx context.Context, + op *fuseops.GetInodeAttributesOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1387,8 +1387,8 @@ func (fs *fileSystem) GetInodeAttributes( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) SetInodeAttributes( - ctx context.Context, - op *fuseops.SetInodeAttributesOp) (err error) { + ctx context.Context, + op *fuseops.SetInodeAttributesOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1437,8 +1437,8 @@ func (fs *fileSystem) SetInodeAttributes( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ForgetInode( - ctx context.Context, - op *fuseops.ForgetInodeOp) (err error) { + ctx context.Context, + op *fuseops.ForgetInodeOp) (err error) { // Find the inode. fs.mu.Lock() in := fs.inodeOrDie(op.Inode) @@ -1453,8 +1453,8 @@ func (fs *fileSystem) ForgetInode( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) MkDir( - ctx context.Context, - op *fuseops.MkDirOp) (err error) { + ctx context.Context, + op *fuseops.MkDirOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1512,8 +1512,8 @@ func (fs *fileSystem) MkDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) MkNode( - ctx context.Context, - op *fuseops.MkNodeOp) (err error) { + ctx context.Context, + op *fuseops.MkNodeOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1552,10 +1552,10 @@ func (fs *fileSystem) MkNode( // LOCKS_EXCLUDED(fs.mu) // LOCK_FUNCTION(child) func (fs *fileSystem) createFile( - ctx context.Context, - parentID fuseops.InodeID, - name string, - mode os.FileMode) (child inode.Inode, err error) { + ctx context.Context, + parentID fuseops.InodeID, + name string, + mode os.FileMode) (child inode.Inode, err error) { // Find the parent. fs.mu.Lock() parent := fs.dirInodeOrDie(parentID) @@ -1597,8 +1597,8 @@ func (fs *fileSystem) createFile( // UNLOCK_FUNCTION(fs.mu) // LOCK_FUNCTION(in) func (fs *fileSystem) createLocalFile( - parentID fuseops.InodeID, - name string) (child inode.Inode, err error) { + parentID fuseops.InodeID, + name string) (child inode.Inode, err error) { // Find the parent. fs.mu.Lock() parent := fs.dirInodeOrDie(parentID) @@ -1642,8 +1642,8 @@ func (fs *fileSystem) createLocalFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) CreateFile( - ctx context.Context, - op *fuseops.CreateFileOp) (err error) { + ctx context.Context, + op *fuseops.CreateFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1691,8 +1691,8 @@ func (fs *fileSystem) CreateFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) CreateSymlink( - ctx context.Context, - op *fuseops.CreateSymlinkOp) (err error) { + ctx context.Context, + op *fuseops.CreateSymlinkOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1749,20 +1749,20 @@ func (fs *fileSystem) CreateSymlink( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) RmDir( -// When rm -r or os.RemoveAll call is made, the following calls are made in order -// 1. RmDir (only in the case of os.RemoveAll) -// 2. Unlink all nested files, -// 3. lookupInode call on implicit directory -// 4. Rmdir on the directory. -// -// When type cache ttl is set, we construct an implicitDir even though one doesn't -// exist on GCS (https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/internal/fs/inode/dir.go#L452), -// and thus, we get rmDir call to GCSFuse. -// Whereas when ttl is zero, lookupInode call itself fails and RmDir is not called -// because object is not present in GCS. - - ctx context.Context, - op *fuseops.RmDirOp) (err error) { + // When rm -r or os.RemoveAll call is made, the following calls are made in order + // 1. RmDir (only in the case of os.RemoveAll) + // 2. Unlink all nested files, + // 3. lookupInode call on implicit directory + // 4. Rmdir on the directory. + // + // When type cache ttl is set, we construct an implicitDir even though one doesn't + // exist on GCS (https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/internal/fs/inode/dir.go#L452), + // and thus, we get rmDir call to GCSFuse. + // Whereas when ttl is zero, lookupInode call itself fails and RmDir is not called + // because object is not present in GCS. + + ctx context.Context, + op *fuseops.RmDirOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1863,8 +1863,8 @@ func (fs *fileSystem) RmDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) Rename( - ctx context.Context, - op *fuseops.RenameOp) (err error) { + ctx context.Context, + op *fuseops.RenameOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -1923,12 +1923,12 @@ func (fs *fileSystem) Rename( // LOCKS_EXCLUDED(oldParent) // LOCKS_EXCLUDED(newParent) func (fs *fileSystem) renameFile( - ctx context.Context, - oldParent inode.DirInode, - oldName string, - oldObject *gcs.MinObject, - newParent inode.DirInode, - newFileName string) error { + ctx context.Context, + oldParent inode.DirInode, + oldName string, + oldObject *gcs.MinObject, + newParent inode.DirInode, + newFileName string) error { // Clone into the new location. newParent.Lock() _, err := newParent.CloneToChildFile(ctx, newFileName, oldObject) @@ -1969,11 +1969,11 @@ func (fs *fileSystem) renameFile( // LOCKS_EXCLUDED(oldParent) // LOCKS_EXCLUDED(newParent) func (fs *fileSystem) renameDir( - ctx context.Context, - oldParent inode.DirInode, - oldName string, - newParent inode.DirInode, - newName string) error { + ctx context.Context, + oldParent inode.DirInode, + oldName string, + newParent inode.DirInode, + newName string) error { // Set up a function that throws away the lookup count increment from // lookUpOrCreateChildInode (since the pending inodes are not sent back to @@ -2082,8 +2082,8 @@ func (fs *fileSystem) renameDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) Unlink( - ctx context.Context, - op *fuseops.UnlinkOp) (err error) { + ctx context.Context, + op *fuseops.UnlinkOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2118,7 +2118,7 @@ func (fs *fileSystem) Unlink( err = parent.DeleteChildFile( ctx, op.Name, - 0, // Latest generation + 0, // Latest generation nil) // No meta-generation precondition if err != nil { @@ -2135,8 +2135,8 @@ func (fs *fileSystem) Unlink( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) OpenDir( - ctx context.Context, - op *fuseops.OpenDirOp) (err error) { + ctx context.Context, + op *fuseops.OpenDirOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2176,8 +2176,8 @@ func (fs *fileSystem) OpenDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReadDir( - ctx context.Context, - op *fuseops.ReadDirOp) (err error) { + ctx context.Context, + op *fuseops.ReadDirOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2206,8 +2206,8 @@ func (fs *fileSystem) ReadDir( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReleaseDirHandle( - ctx context.Context, - op *fuseops.ReleaseDirHandleOp) (err error) { + ctx context.Context, + op *fuseops.ReleaseDirHandleOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2222,8 +2222,8 @@ func (fs *fileSystem) ReleaseDirHandle( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) OpenFile( - ctx context.Context, - op *fuseops.OpenFileOp) (err error) { + ctx context.Context, + op *fuseops.OpenFileOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2248,8 +2248,8 @@ func (fs *fileSystem) OpenFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReadFile( - ctx context.Context, - op *fuseops.ReadFileOp) (err error) { + ctx context.Context, + op *fuseops.ReadFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2281,8 +2281,8 @@ func (fs *fileSystem) ReadFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReadSymlink( - ctx context.Context, - op *fuseops.ReadSymlinkOp) (err error) { + ctx context.Context, + op *fuseops.ReadSymlinkOp) (err error) { // Find the inode. fs.mu.Lock() in := fs.symlinkInodeOrDie(op.Inode) @@ -2299,8 +2299,8 @@ func (fs *fileSystem) ReadSymlink( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) WriteFile( - ctx context.Context, - op *fuseops.WriteFileOp) (err error) { + ctx context.Context, + op *fuseops.WriteFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2326,8 +2326,8 @@ func (fs *fileSystem) WriteFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) SyncFile( - ctx context.Context, - op *fuseops.SyncFileOp) (err error) { + ctx context.Context, + op *fuseops.SyncFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2359,8 +2359,8 @@ func (fs *fileSystem) SyncFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) FlushFile( - ctx context.Context, - op *fuseops.FlushFileOp) (err error) { + ctx context.Context, + op *fuseops.FlushFileOp) (err error) { if fs.mountConfig.FileSystemConfig.IgnoreInterrupts { // When ignore interrupts config is set, we are creating a new context not // cancellable by parent context. @@ -2386,8 +2386,8 @@ func (fs *fileSystem) FlushFile( // LOCKS_EXCLUDED(fs.mu) func (fs *fileSystem) ReleaseFileHandle( - ctx context.Context, - op *fuseops.ReleaseFileHandleOp) (err error) { + ctx context.Context, + op *fuseops.ReleaseFileHandleOp) (err error) { fs.mu.Lock() defer fs.mu.Unlock() @@ -2401,13 +2401,13 @@ func (fs *fileSystem) ReleaseFileHandle( } func (fs *fileSystem) GetXattr( - ctx context.Context, - op *fuseops.GetXattrOp) (err error) { + ctx context.Context, + op *fuseops.GetXattrOp) (err error) { return syscall.ENOSYS } func (fs *fileSystem) ListXattr( - ctx context.Context, - op *fuseops.ListXattrOp) error { + ctx context.Context, + op *fuseops.ListXattrOp) error { return syscall.ENOSYS } From fd0c6dd7883695233689b7cab201a9a03aa0215e Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Thu, 4 Jul 2024 09:47:39 +0000 Subject: [PATCH 6/9] comment changes --- internal/fs/fs.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 7c1b2e92d3..8b53656149 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -2154,14 +2154,14 @@ func (fs *fileSystem) OpenDir( // Enables kernel list-cache in case of non-zero kernelListCacheTTL. if fs.kernelListCacheTTL > 0 { - // Following lock ordering rules: first taking dirInode.lock() and then fs.lock(). + // Following lock ordering rules: first taking dirInode.RLock() and then fs.lock(). fs.mu.Unlock() // Taking RLock() since ShouldInvalidateKernelListCache only reads the DirInode // properties, no modification. in.RLock() - // Acquiring fs lock use common defer function. + // Acquiring fs lock to use common defer function. fs.mu.Lock() // Invalidates the kernel list-cache once the last cached response is out of From db23da3be36fec7c6fd9fe5dbb52b153cf9830e1 Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Mon, 8 Jul 2024 04:51:34 +0000 Subject: [PATCH 7/9] Review comments --- internal/fs/fs.go | 14 +++----------- internal/fs/inode/dir.go | 13 +++++++------ 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 8b53656149..e639e5d997 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -2138,7 +2138,6 @@ func (fs *fileSystem) OpenDir( ctx context.Context, op *fuseops.OpenDirOp) (err error) { fs.mu.Lock() - defer fs.mu.Unlock() // Make sure the inode still exists and is a directory. If not, something has // screwed up because the VFS layer shouldn't have let us forget the inode @@ -2152,22 +2151,15 @@ func (fs *fileSystem) OpenDir( fs.handles[handleID] = handle.NewDirHandle(in, fs.implicitDirs) op.Handle = handleID + fs.mu.Unlock() + // Enables kernel list-cache in case of non-zero kernelListCacheTTL. + // Explicitly not adding any lock, since if fs.kernelListCacheTTL > 0 { - // Following lock ordering rules: first taking dirInode.RLock() and then fs.lock(). - fs.mu.Unlock() - - // Taking RLock() since ShouldInvalidateKernelListCache only reads the DirInode - // properties, no modification. - in.RLock() - - // Acquiring fs lock to use common defer function. - fs.mu.Lock() // Invalidates the kernel list-cache once the last cached response is out of // kernelListCacheTTL. op.KeepCache = !in.ShouldInvalidateKernelListCache(fs.kernelListCacheTTL) - in.RUnlock() op.CacheDir = true } diff --git a/internal/fs/inode/dir.go b/internal/fs/inode/dir.go index ee49ecdb43..f3032ba52d 100644 --- a/internal/fs/inode/dir.go +++ b/internal/fs/inode/dir.go @@ -201,7 +201,7 @@ type dirInode struct { // (via kernel) the directory listing from the filesystem. // Specially used when kernelListCacheTTL > 0 that means kernel list-cache is // enabled. - prevDirListingTimeStamp *time.Time + prevDirListingTimeStamp time.Time } var _ DirInode = &dirInode{} @@ -695,8 +695,7 @@ func (d *dirInode) ReadEntries( entries = append(entries, entry) } - nowTime := d.cacheClock.Now() - d.prevDirListingTimeStamp = &nowTime + d.prevDirListingTimeStamp = d.cacheClock.Now() return } @@ -892,13 +891,15 @@ func (d *dirInode) LocalFileEntries(localFileInodes map[Name]Inode) (localEntrie return } +// ShouldInvalidateKernelListCache doesn't require any lock as d.prevDirListingTimeStamp +// is concurrency safe, and we are okay with the in-consistent value. func (d *dirInode) ShouldInvalidateKernelListCache(ttl time.Duration) bool { - // prevDirListingTimeStamp = nil means listing has not happened yet, and we should + // prevDirListingTimeStamp.IsZero() true means listing has not happened yet, and we should // invalidate for clean start. - if d.prevDirListingTimeStamp == nil { + if d.prevDirListingTimeStamp.IsZero() { return true } - cachedDuration := d.cacheClock.Now().Sub(*d.prevDirListingTimeStamp) + cachedDuration := d.cacheClock.Now().Sub(d.prevDirListingTimeStamp) return cachedDuration >= ttl } From 0e4b40a232e717154a3afa70a82a1c5493043041 Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Tue, 9 Jul 2024 03:01:41 +0000 Subject: [PATCH 8/9] Liting fix --- internal/fs/inode/dir_test.go | 33 +++++++++++++++------------------ 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/internal/fs/inode/dir_test.go b/internal/fs/inode/dir_test.go index d67de340ab..ece1a50798 100644 --- a/internal/fs/inode/dir_test.go +++ b/internal/fs/inode/dir_test.go @@ -750,13 +750,13 @@ func (t *DirTest) ReadDescendants_NonEmpty() { func (t *DirTest) ReadEntries_Empty() { d := t.in.(*dirInode) AssertNe(nil, d) - AssertEq(nil, d.prevDirListingTimeStamp) + AssertTrue(d.prevDirListingTimeStamp.IsZero()) entries, err := t.readAllEntries() AssertEq(nil, err) ExpectThat(entries, ElementsAre()) // Make sure prevDirListingTimeStamp is initialized. - AssertNe(nil, d.prevDirListingTimeStamp) + AssertFalse(d.prevDirListingTimeStamp.IsZero()) } func (t *DirTest) ReadEntries_NonEmpty_ImplicitDirsDisabled() { @@ -783,7 +783,7 @@ func (t *DirTest) ReadEntries_NonEmpty_ImplicitDirsDisabled() { // Nil prevDirListingTimeStamp d := t.in.(*dirInode) AssertNe(nil, d) - AssertEq(nil, d.prevDirListingTimeStamp) + AssertTrue(d.prevDirListingTimeStamp.IsZero()) // Read entries. entries, err := t.readAllEntries() @@ -811,8 +811,8 @@ func (t *DirTest) ReadEntries_NonEmpty_ImplicitDirsDisabled() { ExpectEq(fuseutil.DT_Link, entry.Type) ExpectEq(metadata.SymlinkType, t.getTypeFromCache("symlink")) - // Make sure prevDirListingTimeStamp is not nil. - AssertNe(nil, d.prevDirListingTimeStamp) + // Make sure prevDirListingTimeStamp is initialized. + AssertFalse(d.prevDirListingTimeStamp.IsZero()) } func (t *DirTest) ReadEntries_NonEmpty_ImplicitDirsEnabled() { @@ -842,7 +842,7 @@ func (t *DirTest) ReadEntries_NonEmpty_ImplicitDirsEnabled() { // Nil prevDirListingTimeStamp d := t.in.(*dirInode) AssertNe(nil, d) - AssertEq(nil, d.prevDirListingTimeStamp) + AssertTrue(d.prevDirListingTimeStamp.IsZero()) // Read entries. entries, err := t.readAllEntries() @@ -875,8 +875,8 @@ func (t *DirTest) ReadEntries_NonEmpty_ImplicitDirsEnabled() { ExpectEq(fuseutil.DT_Link, entry.Type) ExpectEq(metadata.SymlinkType, t.getTypeFromCache("symlink")) - // Make sure prevDirListingTimeStamp is not nil. - AssertNe(nil, d.prevDirListingTimeStamp) + // Make sure prevDirListingTimeStamp is initialized. + AssertFalse(d.prevDirListingTimeStamp.IsZero()) } func (t *DirTest) ReadEntries_TypeCaching() { @@ -893,7 +893,7 @@ func (t *DirTest) ReadEntries_TypeCaching() { // Nil prevDirListingTimeStamp d := t.in.(*dirInode) AssertNe(nil, d) - AssertEq(nil, d.prevDirListingTimeStamp) + AssertTrue(d.prevDirListingTimeStamp.IsZero()) // Read the directory, priming the type cache. _, err = t.readAllEntries() @@ -925,8 +925,8 @@ func (t *DirTest) ReadEntries_TypeCaching() { ExpectEq(dirObjName, result.MinObject.Name) - // Make sure prevDirListingTimeStamp is not nil. - AssertNe(nil, d.prevDirListingTimeStamp) + // Make sure prevDirListingTimeStamp is initialized. + AssertFalse(d.prevDirListingTimeStamp.IsZero()) } func (t *DirTest) CreateChildFile_DoesntExist() { @@ -1484,7 +1484,7 @@ func (t *DirTest) LocalFileEntriesWithUnlinkedLocalChildFiles() { func (t *DirTest) Test_ShouldInvalidateKernelListCache_ListingNotHappenedYet() { d := t.in.(*dirInode) - d.prevDirListingTimeStamp = nil + d.prevDirListingTimeStamp = time.Time{} // Irrespective of the ttl value, this should always return true. shouldInvalidate := t.in.ShouldInvalidateKernelListCache(util.MaxTimeDuration) @@ -1494,8 +1494,7 @@ func (t *DirTest) Test_ShouldInvalidateKernelListCache_ListingNotHappenedYet() { func (t *DirTest) Test_ShouldInvalidateKernelListCache_WithinTtl() { d := t.in.(*dirInode) - currentTime := d.cacheClock.Now() - d.prevDirListingTimeStamp = ¤tTime + d.prevDirListingTimeStamp = d.cacheClock.Now() ttl := time.Second * 10 t.clock.AdvanceTime(ttl / 2) @@ -1506,8 +1505,7 @@ func (t *DirTest) Test_ShouldInvalidateKernelListCache_WithinTtl() { func (t *DirTest) Test_ShouldInvalidateKernelListCache_ExpiredTtl() { d := t.in.(*dirInode) - currentTime := d.cacheClock.Now() - d.prevDirListingTimeStamp = ¤tTime + d.prevDirListingTimeStamp = d.cacheClock.Now() ttl := 10 * time.Second t.clock.AdvanceTime(ttl + time.Second) @@ -1518,8 +1516,7 @@ func (t *DirTest) Test_ShouldInvalidateKernelListCache_ExpiredTtl() { func (t *DirTest) Test_ShouldInvalidateKernelListCache_ZeroTtl() { d := t.in.(*dirInode) - currentTime := d.cacheClock.Now() - d.prevDirListingTimeStamp = ¤tTime + d.prevDirListingTimeStamp = d.cacheClock.Now() ttl := time.Duration(0) shouldInvalidate := t.in.ShouldInvalidateKernelListCache(ttl) From ceb292e5bb73fe316c039b3881934f982934a995 Mon Sep 17 00:00:00 2001 From: Prince Kumar Date: Tue, 9 Jul 2024 04:32:49 +0000 Subject: [PATCH 9/9] Added more concurrent test --- internal/fs/fs.go | 2 - internal/fs/kernel_list_cache_test.go | 179 +++++++++++++++++++++++++- 2 files changed, 177 insertions(+), 4 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index e639e5d997..197980e722 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -2154,9 +2154,7 @@ func (fs *fileSystem) OpenDir( fs.mu.Unlock() // Enables kernel list-cache in case of non-zero kernelListCacheTTL. - // Explicitly not adding any lock, since if fs.kernelListCacheTTL > 0 { - // Invalidates the kernel list-cache once the last cached response is out of // kernelListCacheTTL. op.KeepCache = !in.ShouldInvalidateKernelListCache(fs.kernelListCacheTTL) diff --git a/internal/fs/kernel_list_cache_test.go b/internal/fs/kernel_list_cache_test.go index e9fbcb44fe..38dec4d66a 100644 --- a/internal/fs/kernel_list_cache_test.go +++ b/internal/fs/kernel_list_cache_test.go @@ -26,6 +26,7 @@ package fs_test import ( "os" "path" + "strings" "sync" "testing" "time" @@ -128,10 +129,11 @@ func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_OpenDirAndLookUpInode wg.Add(2) // Fail if the operation takes more than timeout. timeout := 5 * time.Second + iterationsPerGoroutine := 100 go func() { defer wg.Done() - for i := 0; i < 100; i++ { + for i := 0; i < iterationsPerGoroutine; i++ { f, err := os.Open(path.Join(mntDir, "explicitDir")) assert.Nil(t.T(), err) @@ -141,12 +143,13 @@ func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_OpenDirAndLookUpInode }() go func() { defer wg.Done() - for i := 0; i < 100; i++ { + for i := 0; i < iterationsPerGoroutine; i++ { _, err := os.Stat(path.Join(mntDir, "explicitDir")) assert.Nil(t.T(), err) } }() + // Wait for goroutines or timeout. done := make(chan bool, 1) go func() { wg.Wait() @@ -160,6 +163,178 @@ func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_OpenDirAndLookUpInode } } +// Test_Concurrent_ReadDir tests for potential deadlocks or race conditions +// when multiple goroutines call Readdir() concurrently on the same directory. +func (t *KernelListCacheTestWithPositiveTtl) Test_Concurrent_ReadDir() { + var wg sync.WaitGroup + goroutineCount := 10 // Number of concurrent goroutines + iterationsPerGoroutine := 10 // Number of iterations per goroutine + + wg.Add(goroutineCount) + timeout := 5 * time.Second + + dirPath := path.Join(mntDir, "explicitDir") + + for i := 0; i < goroutineCount; i++ { + go func() { + defer wg.Done() + + for j := 0; j < iterationsPerGoroutine; j++ { + f, err := os.Open(dirPath) + assert.Nil(t.T(), err) + + _, err = f.Readdirnames(-1) // Read all directory entries + assert.Nil(t.T(), err) + + err = f.Close() + assert.Nil(t.T(), err) + } + }() + } + + // Wait for goroutines or timeout + done := make(chan bool, 1) + go func() { + wg.Wait() + done <- true + }() + + select { + case <-done: + // Success: All Readdir operations finished before timeout + case <-time.After(timeout): + assert.FailNow(t.T(), "Possible deadlock or race condition detected during concurrent Readdir calls") + } +} + +// Test_Parallel_ReadDirAndFileOperations detects race conditions and deadlocks when one goroutine +// performs Readdir() while another concurrently creates and deletes files in the same directory. +func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_ReadDirAndFileOperations() { + var wg sync.WaitGroup + wg.Add(2) + timeout := 5 * time.Second // Adjust timeout as needed + iterationsPerGoroutine := 100 + + dirPath := path.Join(mntDir, "explicitDir") + + // Goroutine 1: Repeatedly calls Readdir + go func() { + defer wg.Done() + for i := 0; i < iterationsPerGoroutine; i++ { // Adjust iteration count if needed + f, err := os.Open(dirPath) + assert.Nil(t.T(), err) + + _, err = f.Readdirnames(-1) + if err != nil { + // This is expected, see the documentation for fixConflictingNames() call in dir_handle.go. + assert.True(t.T(), strings.Contains(err.Error(), "input/output error")) + } + + err = f.Close() + assert.Nil(t.T(), err) + } + }() + + // Goroutine 2: Creates and deletes files + go func() { + defer wg.Done() + for i := 0; i < iterationsPerGoroutine; i++ { // Adjust iteration count if needed + filePath := path.Join(dirPath, "tmp_file.txt") + renamedFilePath := path.Join(dirPath, "renamed_tmp_file.txt") + + // Create + f, err := os.Create(filePath) + assert.Nil(t.T(), err) + + err = f.Close() + assert.Nil(t.T(), err) + + // Rename + err = os.Rename(filePath, renamedFilePath) + assert.Nil(t.T(), err) + + // Delete + err = os.Remove(renamedFilePath) + assert.Nil(t.T(), err) + } + }() + + // Wait for goroutines or timeout + done := make(chan bool, 1) + go func() { + wg.Wait() + done <- true + }() + + select { + case <-done: + // Success: Both operations finished before timeout + case <-time.After(timeout): + assert.FailNow(t.T(), "Possible deadlock or race condition detected") + } +} + +// Test_Parallel_ReadDirAndDirOperations tests for potential deadlocks or race conditions when +// ReadDir() is called concurrently with directory creation and deletion operations. +func (t *KernelListCacheTestWithPositiveTtl) Test_Parallel_ReadDirAndDirOperations() { + var wg sync.WaitGroup + wg.Add(2) + timeout := 5 * time.Second + iterationsPerGoroutine := 100 + + parentDir := path.Join(mntDir, "explicitDir") + + // Goroutine 1: Repeatedly calls Readdir + go func() { + defer wg.Done() + for i := 0; i < iterationsPerGoroutine; i++ { + f, err := os.Open(parentDir) + assert.Nil(t.T(), err) + + _, err = f.Readdirnames(0) + assert.Nil(t.T(), err) + + err = f.Close() + assert.Nil(t.T(), err) + } + }() + + // Goroutine 2: Creates and deletes directories + go func() { + defer wg.Done() + for i := 0; i < iterationsPerGoroutine; i++ { + dirPath := path.Join(parentDir, "test_dir") + renamedDirPath := path.Join(parentDir, "renamed_test_dir") + + // Create + err := os.Mkdir(dirPath, 0755) + assert.Nil(t.T(), err) + + // Rename + err = os.Rename(dirPath, renamedDirPath) + assert.Nil(t.T(), err) + + // Delete + err = os.Remove(renamedDirPath) + assert.Nil(t.T(), err) + } + }() + + // Wait for goroutines or timeout + done := make(chan bool, 1) + go func() { + wg.Wait() + done <- true + }() + + select { + case <-done: + // Success: Both operations finished before timeout + case <-time.After(timeout): + assert.FailNow(t.T(), "Possible deadlock or race condition detected during Readdir and directory operations") + } +} + // (a) First ReadDir() will be served from GCSFuse filesystem. // (b) Second ReadDir() within ttl will be served from kernel page cache. func (t *KernelListCacheTestWithPositiveTtl) TestKernelListCache_CacheHit() {