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
---
tcp_splice.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/tcp_splice.c b/tcp_splice.c
index 5f412584..565596d3 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;
--
2.54.0