tcp_splice: Remove questionable "optimisation" of pending bytes tracking

We have a special path that avoids updating conn->pending when the amounts
read and written are equal.  This has a conceptual complexity cost, in
particular, it means that conn->pending[] is not accurate to its normal
meaning for a section of the loop body.

conn->pending[] shares a cacheline with conn->pipe[] and conn->s[], so it's
almost certainly cache-hot.  It's questionable that avoiding the update
of pending even outweighs the extra conditional branch, let alone saves
anything of significance.  Remove it.

This allows us to move the updates to conn->pending closer to the actual
splice() calls, making it easier to reason about its value.  It also lets
us move the conn->pending updates so they can piggy back on existing tests
rather than needing a conditional expression to avoid clobbering it when
splice() returns -1 (EAGAIN).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

1 files changed, 4 insertions, 7 deletions

diff --git a/tcp_splice.c b/tcp_splice.c
index 5f41258..565596d 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -500,6 +500,8 @@ static int tcp_splice_forward(struct ctx *c,
 		if (!readlen) {
 			conn_event(conn, FIN_RCVD(fromsidei));
 		} else if (readlen > 0) {
+			conn->pending[fromsidei] += readlen;
+
 			if (readlen >= (long)c->tcp.pipe_size * 90 / 100)
 				more = SPLICE_F_MORE;
 
@@ -524,16 +526,11 @@ static int tcp_splice_forward(struct ctx *c,
 		flow_trace(conn, "%zi from write-side call (passed %zi)",
 			   written, c->tcp.pipe_size);
 
-		/* Most common case: skip updating count of pending bytes */
-		if (readlen > 0 && readlen == written)
-			continue;
-
-		conn->pending[fromsidei] += readlen > 0 ? readlen : 0;
-		conn->pending[fromsidei] -= written > 0 ? written : 0;
-
 		if (written < 0)
 			break;
 
+		conn->pending[fromsidei] -= written;
+
 		if (conn->events & FIN_RCVD(fromsidei) &&
 		    !conn->pending[fromsidei])
 			break;