From 2f32114bd356c2124d75fb1879ff230fc1f0ca1b Mon Sep 17 00:00:00 2001 From: Jakob Unterwurzacher Date: Sun, 24 Jan 2016 13:08:08 +0100 Subject: [PATCH] Add per-inode write mutex At the moment, FUSE writes to a single file are serialized by the kernel. However, it is unclear if this is guaranteed behaviour or may change in the future. This patch adds our own per-inode write lock to rule out races regardless of kernel behavoir. --- pathfs_frontend/file.go | 26 ++++++-------- pathfs_frontend/write_lock.go | 66 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 15 deletions(-) create mode 100644 pathfs_frontend/write_lock.go diff --git a/pathfs_frontend/file.go b/pathfs_frontend/file.go index 4639157..522b9c1 100644 --- a/pathfs_frontend/file.go +++ b/pathfs_frontend/file.go @@ -43,6 +43,7 @@ type file struct { func NewFile(fd *os.File, writeOnly bool, cfs *cryptfs.CryptFS) nodefs.File { var st syscall.Stat_t syscall.Fstat(int(fd.Fd()), &st) + wlock.register(st.Ino) return &file{ fd: fd, @@ -59,20 +60,6 @@ func (f *file) InnerFile() nodefs.File { func (f *file) SetInode(n *nodefs.Inode) { } -// Ensure that all modifications to the file contents are serialized and no -// reads happen concurrently. -// -// This prevents several races: -// * getFileId vs Truncate -// * zeroPad vs Read -// * RMW vs Write -func (f *file) wlock() { -} -func (f *file) rlock() { -} -func (f *file) unlock() { -} - // readHeader - load the file header from disk // // Returns io.EOF if the file is empty @@ -252,6 +239,7 @@ func (f *file) doWrite(data []byte, off int64) (uint32, fuse.Status) { cryptfs.Debug.Printf("len(oldData)=%d len(blockData)=%d", len(oldData), len(blockData)) } + // Encrypt blockOffset, blockLen := b.CiphertextRange() blockData = f.cfs.EncryptBlock(blockData, b.BlockNo, f.header.Id) cryptfs.Debug.Printf("ino%d: Writing %d bytes to block #%d", @@ -271,6 +259,7 @@ func (f *file) doWrite(data []byte, off int64) (uint32, fuse.Status) { f.fdLock.Lock() _, err = f.fd.WriteAt(blockData, int64(blockOffset)) f.fdLock.Unlock() + if err != nil { cryptfs.Warn.Printf("doWrite: Write failed: %s", err.Error()) status = fuse.ToStatus(err) @@ -284,6 +273,8 @@ func (f *file) doWrite(data []byte, off int64) (uint32, fuse.Status) { // Write - FUSE call func (f *file) Write(data []byte, off int64) (uint32, fuse.Status) { cryptfs.Debug.Printf("ino%d: FUSE Write: offset=%d length=%d", f.ino, off, len(data)) + wlock.lock(f.ino) + defer wlock.unlock(f.ino) fi, err := f.fd.Stat() if err != nil { @@ -306,6 +297,7 @@ func (f *file) Release() { f.fdLock.Lock() f.fd.Close() f.fdLock.Unlock() + wlock.unregister(f.ino) } // Flush - FUSE call @@ -333,7 +325,11 @@ func (f *file) Fsync(flags int) (code fuse.Status) { return r } +// Truncate - FUSE call func (f *file) Truncate(newSize uint64) fuse.Status { + wlock.lock(f.ino) + defer wlock.unlock(f.ino) + // Common case first: Truncate to zero if newSize == 0 { f.fdLock.Lock() @@ -343,7 +339,7 @@ func (f *file) Truncate(newSize uint64) fuse.Status { cryptfs.Warn.Printf("Ftruncate(fd, 0) returned error: %v", err) return fuse.ToStatus(err) } - // A truncate to zero kills the file header + // Truncate to zero kills the file header f.header = nil return fuse.OK } diff --git a/pathfs_frontend/write_lock.go b/pathfs_frontend/write_lock.go new file mode 100644 index 0000000..0704eb6 --- /dev/null +++ b/pathfs_frontend/write_lock.go @@ -0,0 +1,66 @@ +package pathfs_frontend + +import ( + "sync" +) + +func init() { + wlock.m = make(map[uint64]*refCntMutex) +} + +// wlock - serializes write accesses to each file (identified by inode number) +// Writing partial blocks means we have to do read-modify-write cycles. We +// really don't want concurrent writes there. +// Concurrent full-block writes could actually be allowed, but are not to +// keep the locking simple. +var wlock wlockMap + +// wlockMap - usage: +// 1) register +// 2) lock ... unlock ... +// 3) unregister +type wlockMap struct { + mapMutex sync.RWMutex + m map[uint64]*refCntMutex +} + +func (w *wlockMap) register(ino uint64) { + w.mapMutex.Lock() + r := w.m[ino] + if r == nil { + r = &refCntMutex{} + w.m[ino] = r + } + r.refCnt++ // this must happen inside the mapMutex lock + w.mapMutex.Unlock() +} + +func (w *wlockMap) unregister(ino uint64) { + w.mapMutex.Lock() + r := w.m[ino] + r.refCnt-- + if r.refCnt == 0 { + delete(w.m, ino) + } + w.mapMutex.Unlock() +} + +func (w *wlockMap) lock(ino uint64) { + w.mapMutex.RLock() + r := w.m[ino] + w.mapMutex.RUnlock() + r.Lock() // this can take a long time - execute outside the mapMutex lock +} + +func (w *wlockMap) unlock(ino uint64) { + w.mapMutex.RLock() + r := w.m[ino] + w.mapMutex.RUnlock() + r.Unlock() +} + +// refCntMutex - mutex with reference count +type refCntMutex struct { + sync.Mutex + refCnt int +}