diff --git a/pkg/p9/handlers.go b/pkg/p9/handlers.go index b73cfdd81..9aa853398 100644 --- a/pkg/p9/handlers.go +++ b/pkg/p9/handlers.go @@ -1249,11 +1249,6 @@ func doWalk(cs *connState, ref *fidRef, names []string, getattr bool) (qids []QI file: sf, mode: ref.mode, pathNode: ref.pathNode, - - // For the clone case, the cloned fid must - // preserve the deleted property of the - // original FID. - deleted: ref.deleted, } if !ref.isRoot() { if !newRef.isDeleted() { diff --git a/pkg/p9/path_tree.go b/pkg/p9/path_tree.go index 72ef53313..14f4e1d68 100644 --- a/pkg/p9/path_tree.go +++ b/pkg/p9/path_tree.go @@ -33,10 +33,15 @@ import ( type pathNode struct { // opMu synchronizes high-level, sematic operations, such as the // simultaneous creation and deletion of a file. - // - // opMu does not directly protect any fields in pathNode. opMu sync.RWMutex + // deleted indicates that the backing file has been deleted. We stop many + // operations at the API level if they are incompatible with a file that has + // already been unlinked. deleted is protected by opMu. However, it may be + // changed without opMu if this node is deleted as part of an entire subtree + // on unlink. So deleted must only be accessed/mutated using atomics. + deleted uint32 + // childMu protects the fields below. childMu sync.RWMutex @@ -211,7 +216,9 @@ func (p *pathNode) removeWithName(name string, fn func(ref *fidRef)) *pathNode { for ref := range m { delete(m, ref) delete(p.childRefNames, ref) - fn(ref) + if fn != nil { + fn(ref) + } } } diff --git a/pkg/p9/server.go b/pkg/p9/server.go index e7d129f9d..4866743cc 100644 --- a/pkg/p9/server.go +++ b/pkg/p9/server.go @@ -179,11 +179,6 @@ type fidRef struct { // isRoot should be used to check for root over looking at parent // directly. parent *fidRef - - // deleted indicates that the backing file has been deleted. We stop - // many operations at the API level if they are incompatible with a - // file that has already been unlinked. - deleted uint32 } // IncRef increases the references on a fid. @@ -211,8 +206,11 @@ func (f *fidRef) DecRef() { } // isDeleted returns true if this fidRef has been deleted. +// +// Precondition: this must be called via safelyRead, safelyWrite or +// safelyGlobal. func (f *fidRef) isDeleted() bool { - return atomic.LoadUint32(&f.deleted) != 0 + return atomic.LoadUint32(&f.pathNode.deleted) != 0 } // isRoot indicates whether this is a root fid. @@ -232,10 +230,7 @@ func (f *fidRef) maybeParent() *fidRef { // // Precondition: this must be called via safelyWrite or safelyGlobal. func notifyDelete(pn *pathNode) { - // Call on all local references. - pn.forEachChildRef(func(ref *fidRef, _ string) { - atomic.StoreUint32(&ref.deleted, 1) - }) + atomic.StoreUint32(&pn.deleted, 1) // Call on all subtrees. pn.forEachChildNode(func(pn *pathNode) { @@ -247,11 +242,7 @@ func notifyDelete(pn *pathNode) { // // Precondition: this must be called via safelyWrite or safelyGlobal. func (f *fidRef) markChildDeleted(name string) { - origPathNode := f.pathNode.removeWithName(name, func(ref *fidRef) { - atomic.StoreUint32(&ref.deleted, 1) - }) - - if origPathNode != nil { + if origPathNode := f.pathNode.removeWithName(name, nil); origPathNode != nil { // Mark all children as deleted. notifyDelete(origPathNode) }