replace flock with internal per-inode lock

[chrisd8088]

Our current implementation uses flock(2) to attempt to acquire an (advisory) exclusive lock on an inode while determining its projection state; however, as noted, this may interfere with clients' independent use of flock(2).

The VFSForGit application and .NET Core are an example of such a client; in particular, the UpdatePlaceholderTests functional tests are currently failing because they acquire file handles using the FileShare.Delete mode, which uses a flock() mode of LOCK_SH. The GitIndexProjection.UpdateOrDeleteFilePlaceholder() method then invokes the DeleteFile() method of the Linux VirtualizationInstance, which attempts a simple File.Delete() but receives EAGAIN from libprojfs's failed flock(2) call using LOCK_EX | LOCK_NB.

Replacing the use of flock(2) with an internal inode-to-lock mapping in libprojfs would alleviate this type of contention for locks with all clients, including VFSForGit.

Note that lock contention of this type may also be the reason we have to handle EAGAIN return codes from attempts to write to files in other places in the VFSForGit functional test suite; see, for example, (github/VFSForGit@88cc76a).

It may also be the cause of the apparently transient failures sometimes encountered with the GitCommands.StatusTests.CreateFileWithoutClose() and GitCommands.StatusTests.WriteWithoutClose() tests, which have been seen to report Resource temporarily unavailable (i.e., an EAGAIN error code) while attempting to clean up their test files. For example:

Failed : GVFS.FunctionalTests.Tests.GitCommands.StatusTests(False).CreateFileWithoutClose()
clean -d -f -x Errors Lines
clean -d -f -x Errors Lines counts do not match. was: 1 expected: 0
Extra: warning: failed to remove CreateFileWithoutClose.md: Resource temporarily unavailable
   at GVFS.Tests.Should.EnumerableShouldExtensions.ShouldMatchInOrder[T](IEnumerable`1 group, IEnumerable`1 expectedValues, Func`3 equals, String message) in GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs:line 119
   at GVFS.FunctionalTests.Tools.GitHelpers.ErrorsShouldMatch(String command, ProcessResult expectedResult, ProcessResult actualResult) in GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs:line 190
   at GVFS.FunctionalTests.Tests.GitCommands.GitRepoTests.RunGitCommand(String command, Boolean ignoreErrors, Boolean checkStatus) in GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs:line 194
   at GVFS.FunctionalTests.Tests.GitCommands.GitRepoTests.TestValidationAndCleanup(Boolean ignoreCase) in GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs:line 116
   at GVFS.FunctionalTests.Tests.GitCommands.GitRepoTests.TearDownForTest() in GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs:line 100

Failed : GVFS.FunctionalTests.Tests.GitCommands.StatusTests(False).WriteWithoutClose()
reset --hard -q HEAD Errors Lines
reset --hard -q HEAD Errors Lines counts do not match. was: 2 expected: 0
Extra: error: unable to unlink old 'Readme.md': Resource temporarily unavailable
Extra: fatal: Could not reset index file to revision 'HEAD'.
   at GVFS.Tests.Should.EnumerableShouldExtensions.ShouldMatchInOrder[T](IEnumerable`1 group, IEnumerable`1 expectedValues, Func`3 equals, String message) in GVFS/GVFS.Tests/Should/EnumerableShouldExtensions.cs:line 119
   at GVFS.FunctionalTests.Tools.GitHelpers.ErrorsShouldMatch(String command, ProcessResult expectedResult, ProcessResult actualResult) in GVFS/GVFS.FunctionalTests/Tools/GitHelpers.cs:line 190
   at GVFS.FunctionalTests.Tests.GitCommands.GitRepoTests.RunGitCommand(String command, Boolean ignoreErrors, Boolean checkStatus) in GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs:line 194
   at GVFS.FunctionalTests.Tests.GitCommands.GitRepoTests.TestValidationAndCleanup(Boolean ignoreCase) in GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs:line 115
   at GVFS.FunctionalTests.Tests.GitCommands.GitRepoTests.TearDownForTest() in GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitRepoTests.cs:line 100

[chrisd8088]

Another issue which we have identified as likely stemming from the use of flock(2) for exclusive locking of inodes while checking their projection status is a significant slowdown when executing on a multi-core system.

Because the FUSE getattr() file operation, in particular, is called repeatedly while the Linux VFS traverses each path in each syscall, and because libprojfs invokes flock(2) on each call to getattr() in order to check the projection status of the parent directory (in the call to project_dir()), this results, we believe, in significant contention and effective serialization of parallel file operations.

Unfortunately, while we could switch to using LOCK_SH instead of LOCK_EX for the common fast-path case where the directory has already been projected, we can't know that in advance, and there is no race-free way to upgrade to an exclusive lock in the case that we need to perform a projection. It might be acceptable to have a race here, though; if, when we do acquire the exclusive lock, the projection has already happened, we can simply unlock and return.

An internal locking system which could be combined with the FUSE low-level API, so that locks were held just in memory in custom per-inode structures, would probably yield the most benefit, however, as it would solve both types of contention issues -- with clients such as .NET that also use flock(), and between threads when calling getattr() and other FUSE file operations.