Skip to content

Commit

Permalink
fs: track most recent parent
Browse files Browse the repository at this point in the history
Introduce the new struct inodeParents that wraps a map
and one special slot for the most recent parent. Unit tests
included.

Because the map is lazily initialized, we should save some
memory on the common single-parent case (= file with no hard links)
compared to before.

Benchmarking with gocryptfs shows no discernible change
in performance. fsstress testing with gocryptfs shows no issues.

TestStaleHardlinks the previous commit passes now.

Change-Id: I8d69093abc906addde751a9e70dbd78a3a61371a
  • Loading branch information
rfjakob authored and hanwen committed Apr 23, 2021
1 parent c318613 commit a90e1f4
Show file tree
Hide file tree
Showing 3 changed files with 180 additions and 41 deletions.
65 changes: 24 additions & 41 deletions fs/inode.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ import (
"github.com/hanwen/go-fuse/v2/fuse"
)

type parentData struct {
name string
parent *Inode
}

// StableAttr holds immutable attributes of a object in the filesystem.
type StableAttr struct {
// Each Inode has a type, which does not change over the
Expand Down Expand Up @@ -105,7 +100,7 @@ type Inode struct {

// Parents of this Inode. Can be more than one due to hard links.
// When you change this, you MUST increment changeCounter.
parents map[parentData]struct{}
parents inodeParents
}

func (n *Inode) IsDir() bool {
Expand All @@ -126,7 +121,6 @@ func initInode(n *Inode, ops InodeEmbedder, attr StableAttr, bridge *rawBridge,
n.bridge = bridge
n.persistent = persistent
n.nodeId = nodeId
n.parents = make(map[parentData]struct{})
if attr.Mode == fuse.S_IFDIR {
n.children = make(map[string]*Inode)
}
Expand Down Expand Up @@ -265,7 +259,7 @@ func unlockNodes(ns ...*Inode) {
func (n *Inode) Forgotten() bool {
n.mu.Lock()
defer n.mu.Unlock()
return n.lookupCount == 0 && len(n.parents) == 0 && !n.persistent
return n.lookupCount == 0 && n.parents.count() == 0 && !n.persistent
}

// Operations returns the object implementing the file system
Expand All @@ -283,26 +277,16 @@ func (n *Inode) Path(root *Inode) string {
var segments []string
p := n
for p != nil && p != root {
var pd parentData

// We don't try to take all locks at the same time, because
// the caller won't use the "path" string under lock anyway.
found := false
p.mu.Lock()
// Select an arbitrary parent
for pd = range p.parents {
found = true
break
}
// Get last known parent
pd := p.parents.get()
p.mu.Unlock()
if found == false {
if pd == nil {
p = nil
break
}
if pd.parent == nil {
break
}

segments = append(segments, pd.name)
p = pd.parent
}
Expand Down Expand Up @@ -340,11 +324,9 @@ func (iparent *Inode) setEntry(name string, ichild *Inode) {
// Directories cannot have more than one parent. Clear the map.
// This special-case is neccessary because ichild may still have a
// parent that was forgotten (i.e. removed from bridge.inoMap).
for i := range ichild.parents {
delete(ichild.parents, i)
}
ichild.parents.clear()
}
ichild.parents[newParent] = struct{}{}
ichild.parents.add(newParent)
iparent.children[name] = ichild
ichild.changeCounter++
iparent.changeCounter++
Expand Down Expand Up @@ -421,7 +403,7 @@ retry:
parents = parents[:0]
nChange := n.changeCounter
live = n.lookupCount > 0 || len(n.children) > 0 || n.persistent
for p := range n.parents {
for _, p := range n.parents.all() {
parents = append(parents, p)
lockme = append(lockme, p.parent)
}
Expand All @@ -447,7 +429,7 @@ retry:
delete(p.parent.children, p.name)
p.parent.changeCounter++
}
n.parents = map[parentData]struct{}{}
n.parents.clear()
n.changeCounter++

if n.lookupCount != 0 {
Expand Down Expand Up @@ -488,7 +470,7 @@ retry:
parentCounter := n.changeCounter
if !ok {
n.children[name] = ch
ch.parents[parentData{name, n}] = struct{}{}
ch.parents.add(parentData{name, n})
n.changeCounter++
ch.changeCounter++
unlockNode2(n, ch)
Expand All @@ -506,9 +488,9 @@ retry:
continue retry
}

delete(prev.parents, parentData{name, n})
prev.parents.delete(parentData{name, n})
n.children[name] = ch
ch.parents[parentData{name, n}] = struct{}{}
ch.parents.add(parentData{name, n})
n.changeCounter++
ch.changeCounter++
prev.changeCounter++
Expand All @@ -534,10 +516,11 @@ func (n *Inode) Children() map[string]*Inode {
func (n *Inode) Parent() (string, *Inode) {
n.mu.Lock()
defer n.mu.Unlock()
for k := range n.parents {
return k.name, k.parent
p := n.parents.get()
if p == nil {
return "", nil
}
return "", nil
return p.name, p.parent
}

// RmAllChildren recursively drops a tree, forgetting all persistent
Expand Down Expand Up @@ -589,7 +572,7 @@ retry:
for _, nm := range names {
ch := n.children[nm]
delete(n.children, nm)
delete(ch.parents, parentData{nm, n})
ch.parents.delete(parentData{nm, n})

ch.changeCounter++
}
Expand Down Expand Up @@ -640,7 +623,7 @@ retry:

if oldChild != nil {
delete(n.children, old)
delete(oldChild.parents, parentData{old, n})
oldChild.parents.delete(parentData{old, n})
n.changeCounter++
oldChild.changeCounter++
}
Expand All @@ -649,7 +632,7 @@ retry:
// This can cause the child to be slated for
// removal; see below
delete(newParent.children, newName)
delete(destChild.parents, parentData{newName, newParent})
destChild.parents.delete(parentData{newName, newParent})
destChild.changeCounter++
newParent.changeCounter++
}
Expand All @@ -658,7 +641,7 @@ retry:
newParent.children[newName] = oldChild
newParent.changeCounter++

oldChild.parents[parentData{newName, newParent}] = struct{}{}
oldChild.parents.add(parentData{newName, newParent})
oldChild.changeCounter++
}

Expand Down Expand Up @@ -698,14 +681,14 @@ retry:
// Detach
if oldChild != nil {
delete(oldParent.children, oldName)
delete(oldChild.parents, parentData{oldName, oldParent})
oldChild.parents.delete(parentData{oldName, oldParent})
oldParent.changeCounter++
oldChild.changeCounter++
}

if destChild != nil {
delete(newParent.children, newName)
delete(destChild.parents, parentData{newName, newParent})
destChild.parents.delete(parentData{newName, newParent})
destChild.changeCounter++
newParent.changeCounter++
}
Expand All @@ -715,15 +698,15 @@ retry:
newParent.children[newName] = oldChild
newParent.changeCounter++

oldChild.parents[parentData{newName, newParent}] = struct{}{}
oldChild.parents.add(parentData{newName, newParent})
oldChild.changeCounter++
}

if destChild != nil {
oldParent.children[oldName] = oldChild
oldParent.changeCounter++

destChild.parents[parentData{oldName, oldParent}] = struct{}{}
destChild.parents.add(parentData{oldName, oldParent})
destChild.changeCounter++
}
unlockNodes(oldParent, newParent, oldChild, destChild)
Expand Down
101 changes: 101 additions & 0 deletions fs/inode_parents.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Copyright 2021 the Go-FUSE Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package fs

// inodeParents stores zero or more parents of an Inode,
// remembering which one is the most recent.
//
// No internal locking: the caller is responsible for preventing
// concurrent access.
type inodeParents struct {
// newest is the most-recently add()'ed parent.
// nil when we don't have any parents.
newest *parentData
// other are parents in addition to the newest.
// nil or empty when we have <= 1 parents.
other map[parentData]struct{}
}

// add adds a parent to the store.
func (p *inodeParents) add(n parentData) {
// one and only parent
if p.newest == nil {
p.newest = &n
}
// already known as `newest`
if *p.newest == n {
return
}
// old `newest` gets displaced into `other`
if p.other == nil {
p.other = make(map[parentData]struct{})
}
p.other[*p.newest] = struct{}{}
// new parent becomes `newest` (possibly moving up from `other`)
delete(p.other, n)
p.newest = &n
}

// get returns the most recent parent
// or nil if there is no parent at all.
func (p *inodeParents) get() *parentData {
return p.newest
}

// all returns all known parents
// or nil if there is no parent at all.
func (p *inodeParents) all() []parentData {
count := p.count()
if count == 0 {
return nil
}
out := make([]parentData, 0, count)
out = append(out, *p.newest)
for i := range p.other {
out = append(out, i)
}
return out
}

func (p *inodeParents) delete(n parentData) {
// We have zero parents, so we can't delete any.
if p.newest == nil {
return
}
// If it's not the `newest` it must be in `other` (or nowhere).
if *p.newest != n {
delete(p.other, n)
return
}
// We want to delete `newest`, but there is no other to replace it.
if len(p.other) == 0 {
p.newest = nil
return
}
// Move random entry from `other` over `newest`.
var i parentData
for i = range p.other {
p.newest = &i
break
}
delete(p.other, i)
}

func (p *inodeParents) clear() {
p.newest = nil
p.other = nil
}

func (p *inodeParents) count() int {
if p.newest == nil {
return 0
}
return 1 + len(p.other)
}

type parentData struct {
name string
parent *Inode
}
55 changes: 55 additions & 0 deletions fs/inode_parents_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package fs

import (
"testing"
)

func TestInodeParents(t *testing.T) {
var p inodeParents
var ino1, ino2, ino3 Inode

// empty store should be empty without panicing
if count := p.count(); count != 0 {
t.Error(count)
}
if p.all() != nil {
t.Error("empty store should return nil but did not")
}

// non-dupes should be stored
all := []parentData{
parentData{"foo", &ino1},
parentData{"foo2", &ino1},
parentData{"foo3", &ino1},
parentData{"foo", &ino2},
parentData{"foo", &ino3},
}
for i, v := range all {
p.add(v)
if count := p.count(); count != i+1 {
t.Errorf("want=%d have=%d", i+1, count)
}
last := p.get()
if *last != v {
t.Error("get did not give us last-known parent")
}
}

// adding dupes should not cause the count to increase, but
// must cause get() to return the most recently added dupe.
for _, v := range all {
p.add(v)
if count := p.count(); count != len(all) {
t.Errorf("want=%d have=%d", len(all), count)
}
last := p.get()
if *last != v {
t.Error("get did not give us last-known parent")
}
}

all2 := p.all()
if len(all) != len(all2) {
t.Errorf("want=%d have=%d", len(all), len(all2))
}
}

0 comments on commit a90e1f4

Please sign in to comment.