From 41c1b78b2b38fe1d26b2244293f7fa66ad53768d Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Thu, 15 Sep 2022 14:58:48 -0700 Subject: [PATCH] Avoid busy work in TCP processor. TCP processor is woken up on unlock if the lock is held by a user syscall goroutine. In such instances there is no need to wake up the processor from the dispatcher or requeue an endpoint that may have more pending endpoints in the processor loop. The former causes a useless processor wakeup which spins needlessly and the latter keeps the current processor spinning till the lock is released. PiperOrigin-RevId: 474664843 --- pkg/tcpip/transport/tcp/dispatcher.go | 12 +++++++++--- pkg/tcpip/transport/tcp/endpoint.go | 6 ++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/tcpip/transport/tcp/dispatcher.go b/pkg/tcpip/transport/tcp/dispatcher.go index 8b604cbf1..6e4ab5b86 100644 --- a/pkg/tcpip/transport/tcp/dispatcher.go +++ b/pkg/tcpip/transport/tcp/dispatcher.go @@ -326,9 +326,10 @@ func (p *processor) start(wg *sync.WaitGroup) { default: panic(fmt.Sprintf("unexpected tcp state in processor: %v", state)) } - // If there are more segments to process then + // If there are more segments to process and the + // endpoint lock is not held by user then // requeue this endpoint for processing. - if !ep.segmentQueue.empty() { + if !ep.segmentQueue.empty() && !ep.isOwnedByUser() { p.epQ.enqueue(ep) } } @@ -443,7 +444,12 @@ func (d *dispatcher) queuePacket(stackEP stack.TransportEndpoint, id stack.Trans return } - d.selectProcessor(id).queueEndpoint(ep) + // Only wakeup the processor if endpoint lock is not held by a user + // goroutine as endpoint.UnlockUser will wake up the processor if the + // segment queue is not empty. + if !ep.isOwnedByUser() { + d.selectProcessor(id).queueEndpoint(ep) + } } // selectProcessor uses a hash of the transport endpoint ID to queue the diff --git a/pkg/tcpip/transport/tcp/endpoint.go b/pkg/tcpip/transport/tcp/endpoint.go index e0ffc7b95..03916d63c 100644 --- a/pkg/tcpip/transport/tcp/endpoint.go +++ b/pkg/tcpip/transport/tcp/endpoint.go @@ -605,6 +605,12 @@ func calculateAdvertisedMSS(userMSS uint16, r *stack.Route) uint16 { return maxMSS } +// isOwnedByUser() returns true if the endpoint lock is currently +// held by a user(syscall) goroutine. +func (e *endpoint) isOwnedByUser() bool { + return e.ownedByUser.Load() == 1 +} + // LockUser tries to lock e.mu and if it fails it will check if the lock is held // by another syscall goroutine. If yes, then it will goto sleep waiting for the // lock to be released, if not then it will spin till it acquires the lock or