diff options
author | Jamie Liu <jamieliu@google.com> | 2020-08-07 18:32:03 -0700 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-08-07 18:41:48 -0700 |
commit | 343661770aa96efe3b539a82addff13df235413f (patch) | |
tree | c394c500c71ce4c1676cb315343d39c701ece4ad /pkg/sentry/fsimpl | |
parent | 8f6d576afe2e9e3fea7d792fd0b0d3f426b8d1b4 (diff) |
Don't hold gofer.filesystem.renameMu during dentry destruction.
PiperOrigin-RevId: 325546629
Diffstat (limited to 'pkg/sentry/fsimpl')
-rw-r--r-- | pkg/sentry/fsimpl/gofer/gofer.go | 20 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/gofer/gofer_test.go | 2 |
2 files changed, 17 insertions, 5 deletions
diff --git a/pkg/sentry/fsimpl/gofer/gofer.go b/pkg/sentry/fsimpl/gofer/gofer.go index 4ac8dd81d..63e589859 100644 --- a/pkg/sentry/fsimpl/gofer/gofer.go +++ b/pkg/sentry/fsimpl/gofer/gofer.go @@ -1158,7 +1158,8 @@ func (d *dentry) OnZeroWatches(ctx context.Context) { // operation. One of the calls may destroy the dentry, so subsequent calls will // do nothing. // -// Preconditions: d.fs.renameMu must be locked for writing. +// Preconditions: d.fs.renameMu must be locked for writing; it may be +// temporarily unlocked. func (d *dentry) checkCachingLocked(ctx context.Context) { // Dentries with a non-zero reference count must be retained. (The only way // to obtain a reference on a dentry with zero references is via path @@ -1238,11 +1239,13 @@ func (d *dentry) checkCachingLocked(ctx context.Context) { } } -// destroyLocked destroys the dentry. It may flushes dirty pages from cache, -// close p9 file and remove reference on parent dentry. +// destroyLocked destroys the dentry. // -// Preconditions: d.fs.renameMu must be locked for writing. d.refs == 0. d is -// not a child dentry. +// Preconditions: +// * d.fs.renameMu must be locked for writing; it may be temporarily unlocked. +// * d.refs == 0. +// * d.parent.children[d.name] != d, i.e. d is not reachable by path traversal +// from its former parent dentry. func (d *dentry) destroyLocked(ctx context.Context) { switch atomic.LoadInt64(&d.refs) { case 0: @@ -1254,6 +1257,10 @@ func (d *dentry) destroyLocked(ctx context.Context) { panic("dentry.destroyLocked() called with references on the dentry") } + // Allow the following to proceed without renameMu locked to improve + // scalability. + d.fs.renameMu.Unlock() + mf := d.fs.mfp.MemoryFile() d.handleMu.Lock() d.dataMu.Lock() @@ -1315,6 +1322,9 @@ func (d *dentry) destroyLocked(ctx context.Context) { delete(d.fs.syncableDentries, d) d.fs.syncMu.Unlock() } + + d.fs.renameMu.Lock() + // Drop the reference held by d on its parent without recursively locking // d.fs.renameMu. if d.parent != nil { diff --git a/pkg/sentry/fsimpl/gofer/gofer_test.go b/pkg/sentry/fsimpl/gofer/gofer_test.go index 36cca3625..bfe75dfe4 100644 --- a/pkg/sentry/fsimpl/gofer/gofer_test.go +++ b/pkg/sentry/fsimpl/gofer/gofer_test.go @@ -52,6 +52,8 @@ func TestDestroyIdempotent(t *testing.T) { } parent.cacheNewChildLocked(child, "child") + fs.renameMu.Lock() + defer fs.renameMu.Unlock() child.checkCachingLocked(ctx) if got := atomic.LoadInt64(&child.refs); got != -1 { t.Fatalf("child.refs=%d, want: -1", got) |