From 36363fb4b864ae44cfaa9e52cd7719a807c02f26 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Wed, 19 Dec 2018 13:58:03 -0300 Subject: [PATCH 1/2] rename and document `File`'s node lock --- fd.go | 4 ++-- file.go | 19 ++++++++++--------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/fd.go b/fd.go index 45a7158..53275c4 100644 --- a/fd.go +++ b/fd.go @@ -140,13 +140,13 @@ func (fi *fileDescriptor) flushUp(fullsync bool) error { return err } - fi.inode.nodelk.Lock() + fi.inode.nodeLock.Lock() fi.inode.node = nd // TODO: Create a `SetNode` method. name := fi.inode.name parent := fi.inode.parent // TODO: Can the parent be modified? Do we need to do this inside the lock? - fi.inode.nodelk.Unlock() + fi.inode.nodeLock.Unlock() // TODO: Maybe all this logic should happen in `File`. return parent.closeChild(name, nd, fullsync) diff --git a/file.go b/file.go index 2c69125..963428a 100644 --- a/file.go +++ b/file.go @@ -28,8 +28,9 @@ type File struct { // of a particular sub-DAG that abstract an upper layer's entity. node ipld.Node - // TODO: Rename. - nodelk sync.Mutex + // Lock around the `node` that represents this file, necessary because + // there may be many `FileDescriptor`s operating on this `File`. + nodeLock sync.Mutex RawLeaves bool } @@ -58,9 +59,9 @@ const ( ) func (fi *File) Open(flags int, sync bool) (FileDescriptor, error) { - fi.nodelk.Lock() + fi.nodeLock.Lock() node := fi.node - fi.nodelk.Unlock() + fi.nodeLock.Unlock() // TODO: Move this `switch` logic outside (maybe even // to another package, this seems like a job of UnixFS), @@ -118,8 +119,8 @@ func (fi *File) Open(flags int, sync bool) (FileDescriptor, error) { // pretty much the same thing as here, we should at least call // that function and wrap the `ErrNotUnixfs` with an MFS text. func (fi *File) Size() (int64, error) { - fi.nodelk.Lock() - defer fi.nodelk.Unlock() + fi.nodeLock.Lock() + defer fi.nodeLock.Unlock() switch nd := fi.node.(type) { case *dag.ProtoNode: fsn, err := ft.FSNodeFromBytes(nd.Data()) @@ -135,10 +136,10 @@ func (fi *File) Size() (int64, error) { } // GetNode returns the dag node associated with this file -// TODO: Use this method and do not access the `nodelk` directly anywhere else. +// TODO: Use this method and do not access the `nodeLock` directly anywhere else. func (fi *File) GetNode() (ipld.Node, error) { - fi.nodelk.Lock() - defer fi.nodelk.Unlock() + fi.nodeLock.Lock() + defer fi.nodeLock.Unlock() return fi.node, nil } From 00b7d5c5388270eb1865b8c9e3500320597e2f34 Mon Sep 17 00:00:00 2001 From: Lucas Molas Date: Wed, 19 Dec 2018 14:04:35 -0300 Subject: [PATCH 2/2] use RW lock for the `File`'s node This allows us to, e.g., get the size, etc. in parallel. --- file.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/file.go b/file.go index 963428a..f75723a 100644 --- a/file.go +++ b/file.go @@ -30,7 +30,7 @@ type File struct { // Lock around the `node` that represents this file, necessary because // there may be many `FileDescriptor`s operating on this `File`. - nodeLock sync.Mutex + nodeLock sync.RWMutex RawLeaves bool } @@ -59,9 +59,9 @@ const ( ) func (fi *File) Open(flags int, sync bool) (FileDescriptor, error) { - fi.nodeLock.Lock() + fi.nodeLock.RLock() node := fi.node - fi.nodeLock.Unlock() + fi.nodeLock.RUnlock() // TODO: Move this `switch` logic outside (maybe even // to another package, this seems like a job of UnixFS), @@ -119,8 +119,8 @@ func (fi *File) Open(flags int, sync bool) (FileDescriptor, error) { // pretty much the same thing as here, we should at least call // that function and wrap the `ErrNotUnixfs` with an MFS text. func (fi *File) Size() (int64, error) { - fi.nodeLock.Lock() - defer fi.nodeLock.Unlock() + fi.nodeLock.RLock() + defer fi.nodeLock.RUnlock() switch nd := fi.node.(type) { case *dag.ProtoNode: fsn, err := ft.FSNodeFromBytes(nd.Data()) @@ -138,8 +138,8 @@ func (fi *File) Size() (int64, error) { // GetNode returns the dag node associated with this file // TODO: Use this method and do not access the `nodeLock` directly anywhere else. func (fi *File) GetNode() (ipld.Node, error) { - fi.nodeLock.Lock() - defer fi.nodeLock.Unlock() + fi.nodeLock.RLock() + defer fi.nodeLock.RUnlock() return fi.node, nil }