From 68a806268168c307d49b1dd6a2f06a2c7bb4eabb Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Mon, 23 Mar 2015 10:52:37 +1100 Subject: [PATCH 1/6] FlushFSTest.Mmap --- samples/flushfs/flush_fs_test.go | 38 +++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/samples/flushfs/flush_fs_test.go b/samples/flushfs/flush_fs_test.go index ebdc0f6..032ee96 100644 --- a/samples/flushfs/flush_fs_test.go +++ b/samples/flushfs/flush_fs_test.go @@ -570,7 +570,43 @@ func (t *FlushFSTest) Dup2_FlushError() { } func (t *FlushFSTest) Mmap() { - AssertTrue(false, "TODO") + var n int + var err error + + // Open the file. + t.f1, err = os.OpenFile(path.Join(t.Dir, "foo"), os.O_RDWR, 0) + AssertEq(nil, err) + + // Write some contents to the file. + n, err = t.f1.Write([]byte("taco")) + AssertEq(nil, err) + AssertEq(4, n) + + // mmap the file. + data, err := syscall.Mmap( + int(t.f1.Fd()), 0, 4, + syscall.PROT_READ|syscall.PROT_WRITE, + syscall.MAP_SHARED) + + AssertEq(nil, err) + AssertEq("taco", string(data)) + + // Close the file. We should see a flush. + err = t.f1.Close() + t.f1 = nil + AssertEq(nil, err) + + AssertThat(t.getFlushes(), ElementsAre("taco")) + AssertThat(t.getFsyncs(), ElementsAre()) + + // Modify then unmap. The unmapping should cause another flush. + data[0] = 'p' + + err = syscall.Munmap(data) + AssertEq(nil, err) + + ExpectThat(t.getFlushes(), ElementsAre("taco", "paco")) + ExpectThat(t.getFsyncs(), ElementsAre()) } func (t *FlushFSTest) Directory() { From 2728137e7de1a2e4b86e67aa111f62b3a623c116 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Mon, 23 Mar 2015 11:16:39 +1100 Subject: [PATCH 2/6] Worked around a deadlock in FlushFSTest.Mmap. --- samples/flushfs/flush_fs_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/samples/flushfs/flush_fs_test.go b/samples/flushfs/flush_fs_test.go index 032ee96..6452cbc 100644 --- a/samples/flushfs/flush_fs_test.go +++ b/samples/flushfs/flush_fs_test.go @@ -573,6 +573,19 @@ func (t *FlushFSTest) Mmap() { var n int var err error + // If we run this test with GOMAXPROCS=1 (the default), the program will + // deadlock for the reason described here: + // + // https://groups.google.com/d/msg/golang-nuts/11rdExWP6ac/TzwT6HBOb3wJ + // + // In summary, the goroutine reading from the mmap'd file is camping on a + // scheduler slot while it blocks on a page fault, and the goroutine handling + // fuse requests is waiting for the scheduler slot. + // + // So run with GOMAXPROCS=2. + old := runtime.GOMAXPROCS(2) + defer runtime.GOMAXPROCS(old) + // Open the file. t.f1, err = os.OpenFile(path.Join(t.Dir, "foo"), os.O_RDWR, 0) AssertEq(nil, err) From 83d4308afec488bdaf0e68ecb56b223601ba6edb Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Mon, 23 Mar 2015 11:39:02 +1100 Subject: [PATCH 3/6] Added a note about munmap not calling flush. --- file_system.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/file_system.go b/file_system.go index 8abcf0e..1cddaad 100644 --- a/file_system.go +++ b/file_system.go @@ -242,6 +242,10 @@ type FileSystem interface { // case of close(2), a flush error is returned to the user. For dup2(2), it // is not. // + // Note that one potentially significant case where this is *not* called is + // munmap(2). (Cf. http://goo.gl/7n1r9X, fuse-devel mailing list thread from + // Han-Wen Nienhuys on 2014-10-08.) + // // Because of cases like dup2(2), calls to FlushFile are not necessarily one // to one with calls to OpenFile. They should not be used for reference // counting, and the handle must remain valid even after the method is called From 7020914422c4180490e840afaee5df085e843cab Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Mon, 23 Mar 2015 11:40:03 +1100 Subject: [PATCH 4/6] FlushFSTest.Mmap_CloseBeforeMunmap --- samples/flushfs/flush_fs_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/samples/flushfs/flush_fs_test.go b/samples/flushfs/flush_fs_test.go index 6452cbc..47e9dbb 100644 --- a/samples/flushfs/flush_fs_test.go +++ b/samples/flushfs/flush_fs_test.go @@ -569,7 +569,11 @@ func (t *FlushFSTest) Dup2_FlushError() { ExpectEq(nil, err) } -func (t *FlushFSTest) Mmap() { +func (t *FlushFSTest) Mmap_MunmapBeforeClose() { + AssertTrue(false, "TODO") +} + +func (t *FlushFSTest) Mmap_CloseBeforeMunmap() { var n int var err error @@ -612,13 +616,14 @@ func (t *FlushFSTest) Mmap() { AssertThat(t.getFlushes(), ElementsAre("taco")) AssertThat(t.getFsyncs(), ElementsAre()) - // Modify then unmap. The unmapping should cause another flush. + // Modify then unmap. data[0] = 'p' err = syscall.Munmap(data) AssertEq(nil, err) - ExpectThat(t.getFlushes(), ElementsAre("taco", "paco")) + // munmap does not cause a flush. + ExpectThat(t.getFlushes(), ElementsAre("taco")) ExpectThat(t.getFsyncs(), ElementsAre()) } From 5908a93101bdce5fab70c28de0235584693ec6a8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Mon, 23 Mar 2015 11:40:36 +1100 Subject: [PATCH 5/6] FlushFSTest.Mmap_MunmapBeforeClose --- samples/flushfs/flush_fs_test.go | 52 +++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/samples/flushfs/flush_fs_test.go b/samples/flushfs/flush_fs_test.go index 47e9dbb..7cd3b2f 100644 --- a/samples/flushfs/flush_fs_test.go +++ b/samples/flushfs/flush_fs_test.go @@ -570,7 +570,57 @@ func (t *FlushFSTest) Dup2_FlushError() { } func (t *FlushFSTest) Mmap_MunmapBeforeClose() { - AssertTrue(false, "TODO") + var n int + var err error + + // If we run this test with GOMAXPROCS=1 (the default), the program will + // deadlock for the reason described here: + // + // https://groups.google.com/d/msg/golang-nuts/11rdExWP6ac/TzwT6HBOb3wJ + // + // In summary, the goroutine reading from the mmap'd file is camping on a + // scheduler slot while it blocks on a page fault, and the goroutine handling + // fuse requests is waiting for the scheduler slot. + // + // So run with GOMAXPROCS=2. + old := runtime.GOMAXPROCS(2) + defer runtime.GOMAXPROCS(old) + + // Open the file. + t.f1, err = os.OpenFile(path.Join(t.Dir, "foo"), os.O_RDWR, 0) + AssertEq(nil, err) + + // Write some contents to the file. + n, err = t.f1.Write([]byte("taco")) + AssertEq(nil, err) + AssertEq(4, n) + + // mmap the file. + data, err := syscall.Mmap( + int(t.f1.Fd()), 0, 4, + syscall.PROT_READ|syscall.PROT_WRITE, + syscall.MAP_SHARED) + + AssertEq(nil, err) + AssertEq("taco", string(data)) + + // Modify then unmap. + data[0] = 'p' + + err = syscall.Munmap(data) + AssertEq(nil, err) + + // munmap does not cause a flush. + ExpectThat(t.getFlushes(), ElementsAre()) + ExpectThat(t.getFsyncs(), ElementsAre()) + + // Close the file. We should see a flush. + err = t.f1.Close() + t.f1 = nil + AssertEq(nil, err) + + AssertThat(t.getFlushes(), ElementsAre("paco")) + AssertThat(t.getFsyncs(), ElementsAre()) } func (t *FlushFSTest) Mmap_CloseBeforeMunmap() { From 64d7bf8a69f92e33fcf2624db298dea1a4e9c7a8 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Mon, 23 Mar 2015 12:28:13 +1100 Subject: [PATCH 6/6] Worked around osxfuse/osxfuse#202. --- file_system.go | 4 +++- samples/flushfs/flush_fs_test.go | 12 +++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/file_system.go b/file_system.go index 1cddaad..a0253ad 100644 --- a/file_system.go +++ b/file_system.go @@ -244,7 +244,9 @@ type FileSystem interface { // // Note that one potentially significant case where this is *not* called is // munmap(2). (Cf. http://goo.gl/7n1r9X, fuse-devel mailing list thread from - // Han-Wen Nienhuys on 2014-10-08.) + // Han-Wen Nienhuys on 2014-10-08.) Even if users close(2) after writing to + // an mmap'd file, on OS X the contents are not immediately flushed (cf. + // https://github.com/osxfuse/osxfuse/issues/202). // // Because of cases like dup2(2), calls to FlushFile are not necessarily one // to one with calls to OpenFile. They should not be used for reference diff --git a/samples/flushfs/flush_fs_test.go b/samples/flushfs/flush_fs_test.go index 7cd3b2f..2421ede 100644 --- a/samples/flushfs/flush_fs_test.go +++ b/samples/flushfs/flush_fs_test.go @@ -614,13 +614,19 @@ func (t *FlushFSTest) Mmap_MunmapBeforeClose() { ExpectThat(t.getFlushes(), ElementsAre()) ExpectThat(t.getFsyncs(), ElementsAre()) - // Close the file. We should see a flush. + // Close the file. We should see a flush. On Darwin, this will contain out of + // date contents (cf. https://github.com/osxfuse/osxfuse/issues/202). err = t.f1.Close() t.f1 = nil AssertEq(nil, err) - AssertThat(t.getFlushes(), ElementsAre("paco")) - AssertThat(t.getFsyncs(), ElementsAre()) + if runtime.GOOS == "darwin" { + ExpectThat(t.getFlushes(), ElementsAre("taco")) + ExpectThat(t.getFsyncs(), ElementsAre()) + } else { + ExpectThat(t.getFlushes(), ElementsAre("paco")) + ExpectThat(t.getFsyncs(), ElementsAre()) + } } func (t *FlushFSTest) Mmap_CloseBeforeMunmap() {