From 253f180c691e762ea94291f9a24eb532bf6d1ee7 Mon Sep 17 00:00:00 2001
From: Howard Zhang <howard.zhang@arm.com>
Date: Thu, 25 Mar 2021 17:14:38 +0800
Subject: Fix comments error

Signed-off-by: Howard Zhang <howard.zhang@arm.com>
---
 pkg/sentry/platform/kvm/machine.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'pkg/sentry/platform')

diff --git a/pkg/sentry/platform/kvm/machine.go b/pkg/sentry/platform/kvm/machine.go
index 0e4cf01e1..5d586f257 100644
--- a/pkg/sentry/platform/kvm/machine.go
+++ b/pkg/sentry/platform/kvm/machine.go
@@ -436,7 +436,7 @@ func (m *machine) Get() *vCPU {
 			}
 
 			// The vCPU is not be able to transition to
-			// vCPUGuest|vCPUUser or to vCPUUser because that
+			// vCPUGuest|vCPUWaiter or to vCPUUser because that
 			// transition requires holding the machine mutex, as we
 			// do now. There is no path to register a waiter on
 			// just the vCPUReady state.
-- 
cgit v1.2.3


From da6ddd1df8ef9684d548ec17e7f0e51f5427211b Mon Sep 17 00:00:00 2001
From: Ayush Ranjan <ayushranjan@google.com>
Date: Mon, 29 Mar 2021 10:50:40 -0700
Subject: [perf] Reduce contention in ptrace.threadPool.lookupOrCreate().

lookupOrCreate is called from subprocess.switchToApp() and subprocess.syscall().
lookupOrCreate() looks for a thread already created for the current TID. If a
thread exists (common case), it returns immediately. Otherwise it creates a new
one.

This change switches to using a sync.RWMutex. The initial thread existence
lookup is now done only with the read lock. So multiple successful lookups can
occur concurrently. Only when a new thread is created will it acquire the lock
for writing and update the map (which is not the common case).

Discovered in mutex profiles from the various ptrace benchmarks.
Example: https://gvisor.dev/profile/gvisor-buildkite/fd14bfad-b30f-44dc-859b-80ebac50beb4/843827db-da50-4dc9-a2ea-ecf734dde2d5/tmp/profile/ptrace/BenchmarkFio/operation.write/blockSize.4K/filesystem.tmpfs/benchmarks/fio/mutex.pprof/flamegraph
PiperOrigin-RevId: 365612094
---
 pkg/sentry/platform/ptrace/subprocess.go | 56 +++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 22 deletions(-)

(limited to 'pkg/sentry/platform')

diff --git a/pkg/sentry/platform/ptrace/subprocess.go b/pkg/sentry/platform/ptrace/subprocess.go
index acccbfe2e..d2284487a 100644
--- a/pkg/sentry/platform/ptrace/subprocess.go
+++ b/pkg/sentry/platform/ptrace/subprocess.go
@@ -69,7 +69,7 @@ type thread struct {
 // threadPool is a collection of threads.
 type threadPool struct {
 	// mu protects below.
-	mu sync.Mutex
+	mu sync.RWMutex
 
 	// threads is the collection of threads.
 	//
@@ -85,30 +85,42 @@ type threadPool struct {
 //
 // Precondition: the runtime OS thread must be locked.
 func (tp *threadPool) lookupOrCreate(currentTID int32, newThread func() *thread) *thread {
-	tp.mu.Lock()
+	// The overwhelming common case is that the thread is already created.
+	// Optimistically attempt the lookup by only locking for reading.
+	tp.mu.RLock()
 	t, ok := tp.threads[currentTID]
-	if !ok {
-		// Before creating a new thread, see if we can find a thread
-		// whose system tid has disappeared.
-		//
-		// TODO(b/77216482): Other parts of this package depend on
-		// threads never exiting.
-		for origTID, t := range tp.threads {
-			// Signal zero is an easy existence check.
-			if err := unix.Tgkill(unix.Getpid(), int(origTID), 0); err != nil {
-				// This thread has been abandoned; reuse it.
-				delete(tp.threads, origTID)
-				tp.threads[currentTID] = t
-				tp.mu.Unlock()
-				return t
-			}
-		}
+	tp.mu.RUnlock()
+	if ok {
+		return t
+	}
 
-		// Create a new thread.
-		t = newThread()
-		tp.threads[currentTID] = t
+	tp.mu.Lock()
+	defer tp.mu.Unlock()
+
+	// Another goroutine might have created the thread for currentTID in between
+	// mu.RUnlock() and mu.Lock().
+	if t, ok = tp.threads[currentTID]; ok {
+		return t
+	}
+
+	// Before creating a new thread, see if we can find a thread
+	// whose system tid has disappeared.
+	//
+	// TODO(b/77216482): Other parts of this package depend on
+	// threads never exiting.
+	for origTID, t := range tp.threads {
+		// Signal zero is an easy existence check.
+		if err := unix.Tgkill(unix.Getpid(), int(origTID), 0); err != nil {
+			// This thread has been abandoned; reuse it.
+			delete(tp.threads, origTID)
+			tp.threads[currentTID] = t
+			return t
+		}
 	}
-	tp.mu.Unlock()
+
+	// Create a new thread.
+	t = newThread()
+	tp.threads[currentTID] = t
 	return t
 }
 
-- 
cgit v1.2.3