From df888905abb3855627ea460047b9201f5de5a117 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 05:54:41 +0000 Subject: [PATCH 01/17] Set InitWritebackCache. --- connection.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/connection.go b/connection.go index 7dfd29f..5ae345d 100644 --- a/connection.go +++ b/connection.go @@ -165,6 +165,9 @@ func (c *Connection) Init() (err error) { // Tell the kernel not to use pitifully small 4 KiB writes. initOp.Flags |= fusekernel.InitBigWrites + // TODO(jacobsa): Add comments motivating this. + initOp.Flags |= fusekernel.InitWritebackCache + c.Reply(ctx, nil) return } From de030d4a356012d80d32274bc4f1167a1a2712c4 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 05:58:35 +0000 Subject: [PATCH 02/17] Improved setattr debug logging. --- debug.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/debug.go b/debug.go index 41bcb13..06b12c8 100644 --- a/debug.go +++ b/debug.go @@ -57,6 +57,23 @@ func describeRequest(op interface{}) (s string) { addComponent("parent %d", typed.Parent) addComponent("name %q", typed.Name) + case *fuseops.SetInodeAttributesOp: + if typed.Size != nil { + addComponent("size %d", *typed.Size) + } + + if typed.Mode != nil { + addComponent("mode %v", *typed.Mode) + } + + if typed.Atime != nil { + addComponent("atime %v", *typed.Atime) + } + + if typed.Mtime != nil { + addComponent("mtime %v", *typed.Mtime) + } + case *fuseops.ReadFileOp: addComponent("handle %d", typed.Handle) addComponent("offset %d", typed.Offset) From 9c33490a6c40a83414f60e28606f5f8a49cf5cf6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:04:41 +0000 Subject: [PATCH 03/17] Removed clocks from memfs.go. --- samples/memfs/memfs.go | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/samples/memfs/memfs.go b/samples/memfs/memfs.go index b2499ae..e850573 100644 --- a/samples/memfs/memfs.go +++ b/samples/memfs/memfs.go @@ -26,7 +26,6 @@ import ( "github.com/jacobsa/fuse/fuseops" "github.com/jacobsa/fuse/fuseutil" "github.com/jacobsa/syncutil" - "github.com/jacobsa/timeutil" ) type memFS struct { @@ -36,12 +35,6 @@ type memFS struct { uid uint32 gid uint32 - ///////////////////////// - // Dependencies - ///////////////////////// - - clock timeutil.Clock - ///////////////////////// // Mutable state ///////////////////////// @@ -74,11 +67,9 @@ type memFS struct { // default_permissions option. func NewMemFS( uid uint32, - gid uint32, - clock timeutil.Clock) fuse.Server { + gid uint32) fuse.Server { // Set up the basic struct. fs := &memFS{ - clock: clock, inodes: make([]*inode, fuseops.RootInodeID+1), uid: uid, gid: gid, @@ -91,7 +82,7 @@ func NewMemFS( Gid: gid, } - fs.inodes[fuseops.RootInodeID] = newInode(clock, rootAttrs) + fs.inodes[fuseops.RootInodeID] = newInode(rootAttrs) // Set up invariant checking. fs.mu = syncutil.NewInvariantMutex(fs.checkInvariants) @@ -165,7 +156,7 @@ func (fs *memFS) getInodeOrDie(id fuseops.InodeID) (inode *inode) { func (fs *memFS) allocateInode( attrs fuseops.InodeAttributes) (id fuseops.InodeID, inode *inode) { // Create the inode. - inode = newInode(fs.clock, attrs) + inode = newInode(attrs) // Re-use a free ID if possible. Otherwise mint a new one. numFree := len(fs.freeInodes) @@ -216,7 +207,7 @@ func (fs *memFS) LookUpInode( // We don't spontaneously mutate, so the kernel can cache as long as it wants // (since it also handles invalidation). - op.Entry.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + op.Entry.AttributesExpiration = time.Now().Add(365 * 24 * time.Hour) op.Entry.EntryExpiration = op.Entry.EntryExpiration return @@ -236,7 +227,7 @@ func (fs *memFS) GetInodeAttributes( // We don't spontaneously mutate, so the kernel can cache as long as it wants // (since it also handles invalidation). - op.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + op.AttributesExpiration = time.Now().Add(365 * 24 * time.Hour) return } @@ -258,7 +249,7 @@ func (fs *memFS) SetInodeAttributes( // We don't spontaneously mutate, so the kernel can cache as long as it wants // (since it also handles invalidation). - op.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + op.AttributesExpiration = time.Now().Add(365 * 24 * time.Hour) return } @@ -300,7 +291,7 @@ func (fs *memFS) MkDir( // We don't spontaneously mutate, so the kernel can cache as long as it wants // (since it also handles invalidation). - op.Entry.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + op.Entry.AttributesExpiration = time.Now().Add(365 * 24 * time.Hour) op.Entry.EntryExpiration = op.Entry.EntryExpiration return @@ -324,7 +315,7 @@ func (fs *memFS) CreateFile( } // Set up attributes from the child. - now := fs.clock.Now() + now := time.Now() childAttrs := fuseops.InodeAttributes{ Nlink: 1, Mode: op.Mode, @@ -348,7 +339,7 @@ func (fs *memFS) CreateFile( // We don't spontaneously mutate, so the kernel can cache as long as it wants // (since it also handles invalidation). - op.Entry.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + op.Entry.AttributesExpiration = time.Now().Add(365 * 24 * time.Hour) op.Entry.EntryExpiration = op.Entry.EntryExpiration // We have nothing interesting to put in the Handle field. @@ -374,7 +365,7 @@ func (fs *memFS) CreateSymlink( } // Set up attributes from the child. - now := fs.clock.Now() + now := time.Now() childAttrs := fuseops.InodeAttributes{ Nlink: 1, Mode: 0444 | os.ModeSymlink, @@ -401,7 +392,7 @@ func (fs *memFS) CreateSymlink( // We don't spontaneously mutate, so the kernel can cache as long as it wants // (since it also handles invalidation). - op.Entry.AttributesExpiration = fs.clock.Now().Add(365 * 24 * time.Hour) + op.Entry.AttributesExpiration = time.Now().Add(365 * 24 * time.Hour) op.Entry.EntryExpiration = op.Entry.EntryExpiration return From eecd0fb9647e0f6b8d26c01a16e0eab2892ac416 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:05:17 +0000 Subject: [PATCH 04/17] Fixed inode.go. --- samples/memfs/inode.go | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/samples/memfs/inode.go b/samples/memfs/inode.go index ea6c172..3f98b9f 100644 --- a/samples/memfs/inode.go +++ b/samples/memfs/inode.go @@ -22,19 +22,12 @@ import ( "github.com/jacobsa/fuse/fuseops" "github.com/jacobsa/fuse/fuseutil" - "github.com/jacobsa/timeutil" ) // Common attributes for files and directories. // // External synchronization is required. type inode struct { - ///////////////////////// - // Dependencies - ///////////////////////// - - clock timeutil.Clock - ///////////////////////// // Mutable state ///////////////////////// @@ -79,16 +72,14 @@ type inode struct { // Create a new inode with the supplied attributes, which need not contain // time-related information (the inode object will take care of that). func newInode( - clock timeutil.Clock, attrs fuseops.InodeAttributes) (in *inode) { // Update time info. - now := clock.Now() + now := time.Now() attrs.Mtime = now attrs.Crtime = now // Create the object. in = &inode{ - clock: clock, attrs: attrs, } @@ -226,7 +217,7 @@ func (in *inode) AddChild( var index int // Update the modification time. - in.attrs.Mtime = in.clock.Now() + in.attrs.Mtime = time.Now() // No matter where we place the entry, make sure it has the correct Offset // field. @@ -260,7 +251,7 @@ func (in *inode) AddChild( // REQUIRES: An entry for the given name exists. func (in *inode) RemoveChild(name string) { // Update the modification time. - in.attrs.Mtime = in.clock.Now() + in.attrs.Mtime = time.Now() // Find the entry. i, ok := in.findChild(name) @@ -334,7 +325,7 @@ func (in *inode) WriteAt(p []byte, off int64) (n int, err error) { } // Update the modification time. - in.attrs.Mtime = in.clock.Now() + in.attrs.Mtime = time.Now() // Ensure that the contents slice is long enough. newLen := int(off) + len(p) @@ -361,7 +352,7 @@ func (in *inode) SetAttributes( mode *os.FileMode, mtime *time.Time) { // Update the modification time. - in.attrs.Mtime = in.clock.Now() + in.attrs.Mtime = time.Now() // Truncate? if size != nil { From a6dd3d21479e6809266349c333a167ccb7b1bcad Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:06:40 +0000 Subject: [PATCH 05/17] Fixed memfs_test.go. --- samples/memfs/memfs_test.go | 61 +++++++------------------------------ 1 file changed, 11 insertions(+), 50 deletions(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 10c3f36..f28529b 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -89,7 +89,7 @@ type MemFSTest struct { func init() { RegisterTestSuite(&MemFSTest{}) } func (t *MemFSTest) SetUp(ti *TestInfo) { - t.Server = memfs.NewMemFS(currentUid(), currentGid(), &t.Clock) + t.Server = memfs.NewMemFS(currentUid(), currentGid()) t.SampleTest.SetUp(ti) } @@ -112,17 +112,11 @@ func (t *MemFSTest) Mkdir_OneLevel() { dirName := path.Join(t.Dir, "dir") - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Create a directory within the root. - createTime := t.Clock.Now() + createTime := time.Now() err = os.Mkdir(dirName, 0754) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Stat the directory. fi, err = os.Stat(dirName) stat = fi.Sys().(*syscall.Stat_t) @@ -174,17 +168,11 @@ func (t *MemFSTest) Mkdir_TwoLevels() { err = os.Mkdir(path.Join(t.Dir, "parent"), 0700) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Create a child of that directory. - createTime := t.Clock.Now() + createTime := time.Now() err = os.Mkdir(path.Join(t.Dir, "parent/dir"), 0754) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Stat the directory. fi, err = os.Stat(path.Join(t.Dir, "parent/dir")) stat = fi.Sys().(*syscall.Stat_t) @@ -290,13 +278,10 @@ func (t *MemFSTest) CreateNewFile_InRoot() { fileName := path.Join(t.Dir, "foo") const contents = "Hello\x00world" - createTime := t.Clock.Now() + createTime := time.Now() err = ioutil.WriteFile(fileName, []byte(contents), 0400) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Stat it. fi, err = os.Stat(fileName) stat = fi.Sys().(*syscall.Stat_t) @@ -335,13 +320,10 @@ func (t *MemFSTest) CreateNewFile_InSubDir() { fileName := path.Join(dirName, "foo") const contents = "Hello\x00world" - createTime := t.Clock.Now() + createTime := time.Now() err = ioutil.WriteFile(fileName, []byte(contents), 0400) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Stat it. fi, err = os.Stat(fileName) stat = fi.Sys().(*syscall.Stat_t) @@ -375,26 +357,20 @@ func (t *MemFSTest) ModifyExistingFile_InRoot() { // Write a file. fileName := path.Join(t.Dir, "foo") - createTime := t.Clock.Now() + createTime := time.Now() err = ioutil.WriteFile(fileName, []byte("Hello, world!"), 0600) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Open the file and modify it. f, err := os.OpenFile(fileName, os.O_WRONLY, 0400) t.ToClose = append(t.ToClose, f) AssertEq(nil, err) - modifyTime := t.Clock.Now() + modifyTime := time.Now() n, err = f.WriteAt([]byte("H"), 0) AssertEq(nil, err) AssertEq(1, n) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Stat the file. fi, err = os.Stat(fileName) stat = fi.Sys().(*syscall.Stat_t) @@ -433,26 +409,20 @@ func (t *MemFSTest) ModifyExistingFile_InSubDir() { // Write a file. fileName := path.Join(dirName, "foo") - createTime := t.Clock.Now() + createTime := time.Now() err = ioutil.WriteFile(fileName, []byte("Hello, world!"), 0600) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Open the file and modify it. f, err := os.OpenFile(fileName, os.O_WRONLY, 0400) t.ToClose = append(t.ToClose, f) AssertEq(nil, err) - modifyTime := t.Clock.Now() + modifyTime := time.Now() n, err = f.WriteAt([]byte("H"), 0) AssertEq(nil, err) AssertEq(1, n) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Stat the file. fi, err = os.Stat(fileName) stat = fi.Sys().(*syscall.Stat_t) @@ -574,17 +544,11 @@ func (t *MemFSTest) Rmdir_Empty() { err = os.MkdirAll(path.Join(t.Dir, "foo/bar"), 0754) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Remove the leaf. - rmTime := t.Clock.Now() + rmTime := time.Now() err = os.Remove(path.Join(t.Dir, "foo/bar")) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // There should be nothing left in the parent. entries, err = fusetesting.ReadDirPicky(path.Join(t.Dir, "foo")) @@ -618,13 +582,10 @@ func (t *MemFSTest) Rmdir_OpenedForReading() { var err error // Create a directory. - createTime := t.Clock.Now() + createTime := time.Now() err = os.Mkdir(path.Join(t.Dir, "dir"), 0700) AssertEq(nil, err) - // Simulate time advancing. - t.Clock.AdvanceTime(time.Second) - // Open the directory for reading. f, err := os.Open(path.Join(t.Dir, "dir")) defer func() { From d6e247cc46f8c83e6a4858d4a1feec8bc6d14c2f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:19:14 +0000 Subject: [PATCH 06/17] Fixed several mtime assertions. --- fusetesting/stat.go | 26 +++++++++++++++----------- samples/memfs/memfs_test.go | 15 +++++++++------ 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/fusetesting/stat.go b/fusetesting/stat.go index c084303..c8bdbc1 100644 --- a/fusetesting/stat.go +++ b/fusetesting/stat.go @@ -28,28 +28,32 @@ import ( // also that it matches. func MtimeIs(expected time.Time) oglematchers.Matcher { return oglematchers.NewMatcher( - func(c interface{}) error { return mtimeIs(c, expected) }, + func(c interface{}) error { return mtimeIsWithin(c, expected, 0) }, fmt.Sprintf("mtime is %v", expected)) } -func mtimeIs(c interface{}, expected time.Time) error { +// Like MtimeIs, but allows for a tolerance. +func MtimeIsWithin(expected time.Time, d time.Duration) oglematchers.Matcher { + return oglematchers.NewMatcher( + func(c interface{}) error { return mtimeIsWithin(c, expected, d) }, + fmt.Sprintf("mtime is within %v of %v", d, expected)) +} + +func mtimeIsWithin(c interface{}, expected time.Time, d time.Duration) error { fi, ok := c.(os.FileInfo) if !ok { return fmt.Errorf("which is of type %v", reflect.TypeOf(c)) } // Check ModTime(). - if fi.ModTime() != expected { - d := fi.ModTime().Sub(expected) - return fmt.Errorf("which has mtime %v, off by %v", fi.ModTime(), d) + diff := fi.ModTime().Sub(expected) + absDiff := diff + if absDiff < 0 { + absDiff = -absDiff } - // Check Sys(). - if sysMtime, ok := extractMtime(fi.Sys()); ok { - if sysMtime != expected { - d := sysMtime.Sub(expected) - return fmt.Errorf("which has Sys() mtime %v, off by %v", sysMtime, d) - } + if !(absDiff < d) { + return fmt.Errorf("which has mtime %v, off by %v", fi.ModTime(), diff) } return nil diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index f28529b..3906d12 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -35,6 +35,9 @@ import ( func TestMemFS(t *testing.T) { RunTests(t) } +// TODO(jacobsa): Comments. +const timeSlop = 5 * time.Millisecond + //////////////////////////////////////////////////////////////////////// // Helpers //////////////////////////////////////////////////////////////////////// @@ -125,7 +128,7 @@ func (t *MemFSTest) Mkdir_OneLevel() { ExpectEq("dir", fi.Name()) ExpectEq(0, fi.Size()) ExpectEq(os.ModeDir|applyUmask(0754), fi.Mode()) - ExpectThat(fi, fusetesting.MtimeIs(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectThat(fi, fusetesting.BirthtimeIs(createTime)) ExpectTrue(fi.IsDir()) @@ -181,7 +184,7 @@ func (t *MemFSTest) Mkdir_TwoLevels() { ExpectEq("dir", fi.Name()) ExpectEq(0, fi.Size()) ExpectEq(os.ModeDir|applyUmask(0754), fi.Mode()) - ExpectThat(fi, fusetesting.MtimeIs(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectThat(fi, fusetesting.BirthtimeIs(createTime)) ExpectTrue(fi.IsDir()) @@ -290,7 +293,7 @@ func (t *MemFSTest) CreateNewFile_InRoot() { ExpectEq("foo", fi.Name()) ExpectEq(len(contents), fi.Size()) ExpectEq(applyUmask(0400), fi.Mode()) - ExpectThat(fi, fusetesting.MtimeIs(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectThat(fi, fusetesting.BirthtimeIs(createTime)) ExpectFalse(fi.IsDir()) @@ -332,7 +335,7 @@ func (t *MemFSTest) CreateNewFile_InSubDir() { ExpectEq("foo", fi.Name()) ExpectEq(len(contents), fi.Size()) ExpectEq(applyUmask(0400), fi.Mode()) - ExpectThat(fi, fusetesting.MtimeIs(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectThat(fi, fusetesting.BirthtimeIs(createTime)) ExpectFalse(fi.IsDir()) @@ -379,7 +382,7 @@ func (t *MemFSTest) ModifyExistingFile_InRoot() { ExpectEq("foo", fi.Name()) ExpectEq(len("Hello, world!"), fi.Size()) ExpectEq(applyUmask(0600), fi.Mode()) - ExpectThat(fi, fusetesting.MtimeIs(modifyTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(modifyTime, timeSlop)) ExpectThat(fi, fusetesting.BirthtimeIs(createTime)) ExpectFalse(fi.IsDir()) @@ -431,7 +434,7 @@ func (t *MemFSTest) ModifyExistingFile_InSubDir() { ExpectEq("foo", fi.Name()) ExpectEq(len("Hello, world!"), fi.Size()) ExpectEq(applyUmask(0600), fi.Mode()) - ExpectThat(fi, fusetesting.MtimeIs(modifyTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(modifyTime, timeSlop)) ExpectThat(fi, fusetesting.BirthtimeIs(createTime)) ExpectFalse(fi.IsDir()) From 0bc289ec1bd41dde46f916d607d6ae35ff61ad8d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:20:30 +0000 Subject: [PATCH 07/17] Fixed some more mtime assertions. --- samples/memfs/memfs_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index 3906d12..cc32073 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -142,7 +142,7 @@ func (t *MemFSTest) Mkdir_OneLevel() { fi, err = os.Stat(t.Dir) AssertEq(nil, err) - ExpectEq(0, fi.ModTime().Sub(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) // Read the directory. entries, err = fusetesting.ReadDirPicky(dirName) @@ -197,7 +197,7 @@ func (t *MemFSTest) Mkdir_TwoLevels() { // Check the parent's mtime. fi, err = os.Stat(path.Join(t.Dir, "parent")) AssertEq(nil, err) - ExpectEq(0, fi.ModTime().Sub(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) // Read the directory. entries, err = fusetesting.ReadDirPicky(path.Join(t.Dir, "parent/dir")) @@ -561,7 +561,7 @@ func (t *MemFSTest) Rmdir_Empty() { // Check the parent's mtime. fi, err := os.Stat(path.Join(t.Dir, "foo")) AssertEq(nil, err) - ExpectEq(0, fi.ModTime().Sub(rmTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(rmTime, timeSlop)) // Remove the parent. err = os.Remove(path.Join(t.Dir, "foo")) @@ -619,7 +619,7 @@ func (t *MemFSTest) Rmdir_OpenedForReading() { fi, err := f.Stat() ExpectEq("dir", fi.Name()) - ExpectEq(0, fi.ModTime().Sub(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectEq(0, fi.Sys().(*syscall.Stat_t).Nlink) // Attempt to read from the directory. This shouldn't see any junk from the @@ -1018,7 +1018,7 @@ func (t *MemFSTest) Chtimes() { // Stat it. fi, err := os.Stat(fileName) AssertEq(nil, err) - ExpectEq(0, fi.ModTime().Sub(expectedMtime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(expectedMtime, timeSlop)) } func (t *MemFSTest) ReadDirWhileModifying() { From eaa0177b80daf4c82ffae826983c5d3d245f6c5b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:26:46 +0000 Subject: [PATCH 08/17] Clean up mappings even on assertion failure. --- samples/flushfs/flush_fs_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/samples/flushfs/flush_fs_test.go b/samples/flushfs/flush_fs_test.go index 278f0ba..e787998 100644 --- a/samples/flushfs/flush_fs_test.go +++ b/samples/flushfs/flush_fs_test.go @@ -590,6 +590,8 @@ func (t *NoErrorsTest) Mmap_NoMsync_MunmapBeforeClose() { syscall.MAP_SHARED) AssertEq(nil, err) + defer syscall.Munmap(data) + AssertEq("taco", string(data)) // Modify the contents. @@ -638,6 +640,8 @@ func (t *NoErrorsTest) Mmap_NoMsync_CloseBeforeMunmap() { syscall.MAP_SHARED) AssertEq(nil, err) + defer syscall.Munmap(data) + AssertEq("taco", string(data)) // Close the file. We should see a flush. @@ -682,6 +686,8 @@ func (t *NoErrorsTest) Mmap_WithMsync_MunmapBeforeClose() { syscall.MAP_SHARED) AssertEq(nil, err) + defer syscall.Munmap(data) + AssertEq("taco", string(data)) // Modify the contents. @@ -738,6 +744,8 @@ func (t *NoErrorsTest) Mmap_WithMsync_CloseBeforeMunmap() { syscall.MAP_SHARED) AssertEq(nil, err) + defer syscall.Munmap(data) + AssertEq("taco", string(data)) // Close the file. We should see a flush. @@ -941,6 +949,7 @@ func (t *FsyncErrorTest) Msync() { syscall.MAP_SHARED) AssertEq(nil, err) + defer syscall.Munmap(data) // msync the mapping. err = msync(data) From fee78b6548cc16884cb6783cc6d21f987ede43ea Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:21:45 +0000 Subject: [PATCH 09/17] Added comments for timeSlop. --- samples/memfs/memfs_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index cc32073..b1f5181 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -35,7 +35,9 @@ import ( func TestMemFS(t *testing.T) { RunTests(t) } -// TODO(jacobsa): Comments. +// The radius we use for "expect mtime is within"-style assertions. We can't +// share a synchronized clock with the ultimate source of mtimes because with +// writeback caching enabled the kernel manufactures them based on wall time. const timeSlop = 5 * time.Millisecond //////////////////////////////////////////////////////////////////////// From e69b8a8c37eedad34cbe5358bb259e1348641df5 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Tue, 11 Aug 2015 06:29:52 +0000 Subject: [PATCH 10/17] Implement setattr; otherwise closing after writing fails. --- samples/flushfs/flush_fs.go | 48 ++++++++++++++++++++++++++----------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/samples/flushfs/flush_fs.go b/samples/flushfs/flush_fs.go index 7dea888..2963519 100644 --- a/samples/flushfs/flush_fs.go +++ b/samples/flushfs/flush_fs.go @@ -90,6 +90,29 @@ func (fs *flushFS) barAttributes() fuseops.InodeAttributes { } } +// LOCKS_REQUIRED(fs.mu) +func (fs *flushFS) getAttributes(id fuseops.InodeID) ( + attrs fuseops.InodeAttributes, + err error) { + switch id { + case fuseops.RootInodeID: + attrs = fs.rootAttributes() + return + + case fooID: + attrs = fs.fooAttributes() + return + + case barID: + attrs = fs.barAttributes() + return + + default: + err = fuse.ENOENT + return + } +} + //////////////////////////////////////////////////////////////////////// // FileSystem methods //////////////////////////////////////////////////////////////////////// @@ -134,23 +157,20 @@ func (fs *flushFS) GetInodeAttributes( fs.mu.Lock() defer fs.mu.Unlock() - switch op.Inode { - case fuseops.RootInodeID: - op.Attributes = fs.rootAttributes() - return + op.Attributes, err = fs.getAttributes(op.Inode) + return +} - case fooID: - op.Attributes = fs.fooAttributes() - return +func (fs *flushFS) SetInodeAttributes( + ctx context.Context, + op *fuseops.SetInodeAttributesOp) (err error) { + fs.mu.Lock() + defer fs.mu.Unlock() - case barID: - op.Attributes = fs.barAttributes() - return + // Ignore any changes and simply return existing attributes. + op.Attributes, err = fs.getAttributes(op.Inode) - default: - err = fuse.ENOENT - return - } + return } func (fs *flushFS) OpenFile( From ed6f72d0787059eb18600292ca3ea133ba51830d Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 00:48:00 +0000 Subject: [PATCH 11/17] Updated a TODO. --- connection.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/connection.go b/connection.go index 5ae345d..60aa997 100644 --- a/connection.go +++ b/connection.go @@ -165,7 +165,11 @@ func (c *Connection) Init() (err error) { // Tell the kernel not to use pitifully small 4 KiB writes. initOp.Flags |= fusekernel.InitBigWrites - // TODO(jacobsa): Add comments motivating this. + // TODO(jacobsa): Make this opt out and discuss benefits and caveats: + // * Write performance may be better (cf. http://thread.gmane.org/gmane.comp.file-systems.fuse.devel/13923) + // * (Discuss what writeback caching even means) + // * File systems need to implement setattr for dealing with kernel's stored time (find code reference) + // * File systems no longer "own" mtime; kernel will cache it even if no writes (cf. http://thread.gmane.org/gmane.comp.file-systems.fuse.devel/14808) initOp.Flags |= fusekernel.InitWritebackCache c.Reply(ctx, nil) From 842af25013a32af1edcd4ff73bef5b30307dc1f6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 02:29:23 +0000 Subject: [PATCH 12/17] Added and documented MountConfig.DisableWritebackCaching. --- mount_config.go | 65 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/mount_config.go b/mount_config.go index dd76561..7855a26 100644 --- a/mount_config.go +++ b/mount_config.go @@ -48,6 +48,71 @@ type MountConfig struct { // performed. DebugLogger *log.Logger + // Linux only. + // + // By default on Linux we allow the kernel to perform writeback caching + // (cf. http://goo.gl/LdZzo1): + // + // * When the user calls write(2), the kernel sticks the user's data into + // its page cache. Only later does it call through to the file system, + // potentially after coalescing multiple small user writes. + // + // * The file system may receive multiple write ops from the kernel + // concurrently if there is a lot of page cache data to flush. + // + // * Write performance may be significantly improved due to the user and + // the kernel not waiting for serial round trips to the file system. This + // is especially true if the user makes tiny writes. + // + // * close(2) (and anything else calling f_op->flush) causes all dirty + // pages to be written out before it proceeds to send a FlushFileOp + // (cf. https://goo.gl/TMrY6X). + // + // * Similarly, close(2) causes the kernel to send a setattr request + // filling in the mtime if any dirty pages were flushed, since the time + // at which the pages were written to the file system can't be trusted. + // + // * close(2) (and anything else calling f_op->flush) writes out all dirty + // pages, then sends a setattr request with an appropriate mtime for + // those writes if there were any, and only then proceeds to send a flush + // + // Code walk: + // + // * (https://goo.gl/zTIZQ9) fuse_flush calls write_inode_now before + // calling the file system. The latter eventually calls into + // __writeback_single_inode. + // + // * (https://goo.gl/L7Z2w5) __writeback_single_inode calls + // do_writepages, which writes out any dirty pages. + // + // * (https://goo.gl/DOPgla) __writeback_single_inode later calls + // write_inode, which calls into the superblock op struct's write_inode + // member. For fuse, this is fuse_write_inode + // (cf. https://goo.gl/eDSKOX). + // + // * (https://goo.gl/PbkGA1) fuse_write_inode calls fuse_flush_times. + // + // * (https://goo.gl/ig8x9V) fuse_flush_times sends a setttr request + // for setting the inode's mtime. + // + // However, this brings along some caveats: + // + // * The file system must handle SetInodeAttributesOp or close(2) will fail, + // due to the call chain into fuse_flush_times listed above. + // + // * The kernel caches mtime and ctime regardless of whether the file + // system tells it to do so, disregarding the result of further getattr + // requests (cf. https://goo.gl/3ZZMUw, https://goo.gl/7WtQUp). It + // appears this may be true of the file size, too. Writeback caching may + // therefore not be suitable for file systems where these attributes can + // spontaneously change for reasons the kernel doesn't observe. See + // http://goo.gl/V5WQCN for more discussion. + // + // Setting DisableWritebackCaching disables this behavior. Instead the file + // system is called one or more times for each write(2), and the user's + // syscall doesn't return until the file system returns. + DisableWritebackCaching bool + // OS X only. // // Normally on OS X we mount with the novncache option From f40a24c8d5d923c4a142d1082f7305047319c54e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 02:32:39 +0000 Subject: [PATCH 13/17] Pay attention to DisableWritebackCaching. --- connection.go | 20 ++++++++------------ mount.go | 8 ++++---- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/connection.go b/connection.go index 60aa997..0d7e13a 100644 --- a/connection.go +++ b/connection.go @@ -57,6 +57,7 @@ const maxReadahead = 1 << 20 // A connection to the fuse kernel process. type Connection struct { + cfg MountConfig debugLogger *log.Logger errorLogger *log.Logger @@ -65,9 +66,6 @@ type Connection struct { dev *os.File protocol fusekernel.Protocol - // The context from which all op contexts inherit. - parentCtx context.Context - mu sync.Mutex // A map from fuse "unique" request ID (*not* the op ID for logging used @@ -94,15 +92,15 @@ type opState struct { // // The loggers may be nil. func newConnection( - parentCtx context.Context, + cfg MountConfig, debugLogger *log.Logger, errorLogger *log.Logger, dev *os.File) (c *Connection, err error) { c = &Connection{ + cfg: cfg, debugLogger: debugLogger, errorLogger: errorLogger, dev: dev, - parentCtx: parentCtx, cancelFuncs: make(map[uint64]func()), } @@ -165,12 +163,10 @@ func (c *Connection) Init() (err error) { // Tell the kernel not to use pitifully small 4 KiB writes. initOp.Flags |= fusekernel.InitBigWrites - // TODO(jacobsa): Make this opt out and discuss benefits and caveats: - // * Write performance may be better (cf. http://thread.gmane.org/gmane.comp.file-systems.fuse.devel/13923) - // * (Discuss what writeback caching even means) - // * File systems need to implement setattr for dealing with kernel's stored time (find code reference) - // * File systems no longer "own" mtime; kernel will cache it even if no writes (cf. http://thread.gmane.org/gmane.comp.file-systems.fuse.devel/14808) - initOp.Flags |= fusekernel.InitWritebackCache + // Enable writeback caching if the user hasn't asked us not to. + if !c.cfg.DisableWritebackCaching { + initOp.Flags |= fusekernel.InitWritebackCache + } c.Reply(ctx, nil) return @@ -234,7 +230,7 @@ func (c *Connection) beginOp( opCode uint32, fuseID uint64) (ctx context.Context) { // Start with the parent context. - ctx = c.parentCtx + ctx = c.cfg.OpContext // Set up a cancellation function. // diff --git a/mount.go b/mount.go index 7f9c553..6227617 100644 --- a/mount.go +++ b/mount.go @@ -67,14 +67,14 @@ func Mount( } // Choose a parent context for ops. - opContext := config.OpContext - if opContext == nil { - opContext = context.Background() + cfgCopy := *config + if cfgCopy.OpContext == nil { + cfgCopy.OpContext = context.Background() } // Create a Connection object wrapping the device. connection, err := newConnection( - opContext, + cfgCopy, config.DebugLogger, config.ErrorLogger, dev) From 2e6836708e809c8a85a6eae337cc106ea3142b32 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 02:34:02 +0000 Subject: [PATCH 14/17] Increased the slop in memfs_test. --- samples/memfs/memfs_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/samples/memfs/memfs_test.go b/samples/memfs/memfs_test.go index b1f5181..8ef5544 100644 --- a/samples/memfs/memfs_test.go +++ b/samples/memfs/memfs_test.go @@ -38,7 +38,7 @@ func TestMemFS(t *testing.T) { RunTests(t) } // The radius we use for "expect mtime is within"-style assertions. We can't // share a synchronized clock with the ultimate source of mtimes because with // writeback caching enabled the kernel manufactures them based on wall time. -const timeSlop = 5 * time.Millisecond +const timeSlop = 25 * time.Millisecond //////////////////////////////////////////////////////////////////////// // Helpers From 182c96da073772c78367bd64f99a9266ec1b713e Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 02:35:41 +0000 Subject: [PATCH 15/17] Fixed cachingfs. --- samples/cachingfs/caching_fs_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/samples/cachingfs/caching_fs_test.go b/samples/cachingfs/caching_fs_test.go index 19cdf39..7e733a3 100644 --- a/samples/cachingfs/caching_fs_test.go +++ b/samples/cachingfs/caching_fs_test.go @@ -53,6 +53,10 @@ func (t *cachingFSTest) setUp( getattrTimeout time.Duration) { var err error + // We assert things about whether or not mtimes are cached, but writeback + // caching causes them to always be cached. Turn it off. + t.MountConfig.DisableWritebackCaching = true + // Create the file system. t.fs, err = cachingfs.NewCachingFS(lookupEntryTimeout, getattrTimeout) AssertEq(nil, err) From 308cf97f7e742ef21bb5c98e4d2c3f4cf4cbe18c Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 02:36:11 +0000 Subject: [PATCH 16/17] Described what happens on OS X. --- mount_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount_config.go b/mount_config.go index 7855a26..61f805b 100644 --- a/mount_config.go +++ b/mount_config.go @@ -48,7 +48,7 @@ type MountConfig struct { // performed. DebugLogger *log.Logger - // Linux only. + // Linux only. OS X always behaves as if writeback caching is disabled. // // By default on Linux we allow the kernel to perform writeback caching // (cf. http://goo.gl/LdZzo1): From 4f74c87df8f3bd14ca2f13bb93dad2e5ec13b1bd Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 12:40:31 +1000 Subject: [PATCH 17/17] Updated a redundant code walk. --- fuseops/ops.go | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/fuseops/ops.go b/fuseops/ops.go index f786474..5da1b5e 100644 --- a/fuseops/ops.go +++ b/fuseops/ops.go @@ -509,17 +509,8 @@ type ReadFileOp struct { // // Note that the kernel *will* ensure that writes are received and acknowledged // by the file system before sending a FlushFileOp when closing the file -// descriptor to which they were written: -// -// * (http://goo.gl/PheZjf) fuse_flush calls write_inode_now, which appears -// to start a writeback in the background (it talks about a "flusher -// thread"). -// -// * (http://goo.gl/1IiepM) fuse_flush then calls fuse_sync_writes, which -// "[waits] for all pending writepages on the inode to finish". -// -// * (http://goo.gl/zzvxWv) Only then does fuse_flush finally send the -// flush request. +// descriptor to which they were written. Cf. the notes on +// fuse.MountConfig.DisableWritebackCaching. // // (See also http://goo.gl/ocdTdM, fuse-devel thread "Fuse guarantees on // concurrent requests".)