Re: [PATCH 8/8] tcp_splice: Exit forwarding earlier when stalled read side

On Thu, 4 Jun 2026 15:26:36 +1000 David Gibson wrote:

On Thu, Jun 04, 2026 at 06:41:47AM +0200, Stefano Brivio wrote:

...

On Thu, 28 May 2026 15:02:13 +1000 David Gibson wrote:

...

At the end of our loop we have a conditional 'break' that exits if we're at EOF on the read side and have nothing left in the pipe. This doesn't depend on anything write-side, so we can move it earlier, avoiding an unnecessary write side splice in this case.

Furthermore, there's also nothing to be done write side if we've hit EAGAIN on the read side and the pipe is empty, so exit early for that case as well.

Signed-off-by: David Gibson --- tcp_splice.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tcp_splice.c b/tcp_splice.c index 565596d3..623ca926 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -497,9 +497,17 @@ static int tcp_splice_forward(struct ctx *c,

flow_trace(conn, "%zi from read-side call", readlen);

- if (!readlen) { - conn_event(conn, FIN_RCVD(fromsidei)); - } else if (readlen > 0) { + if (readlen <= 0) { + if (!readlen) /* EOF */ + conn_event(conn, FIN_RCVD(fromsidei)); + + /* We're either blocked or at EOF on the read side, and + * there's nothing in the pipe so there's nothing to do + * write side either. + */ + if (!conn->pending[fromsidei]) + break; + } else { conn->pending[fromsidei] += readlen;

if (readlen >= (long)c->tcp.pipe_size * 90 / 100) @@ -530,10 +538,6 @@ static int tcp_splice_forward(struct ctx *c, break;

conn->pending[fromsidei] -= written; - - if (conn->events & FIN_RCVD(fromsidei) && - !conn->pending[fromsidei]) - break;

The rest of the series looks good to me and I'm running tests now before pushing, but I can't convince myself of the correctness of this change.

The first part makes sense as an additional condition to exit the loop and avoid an additional splice() call that would just return EAGAIN.

But this one is a different condition because it happens to check conn->pending[fromsidei] right after we subtracted 'written' from it, and we know we have no input data anymore, so it avoids a useless (although I think harmless) read-side splice() call in the next iteration of the loop, doesn't it?

Good point. If there was data in the pipe before the read-splice(), then we get EOF, then we empty the pipe with the write-splice() we know we're done and don't need to read-splice() again.

I'm also pretty sure it's harmless, as long as repeated read-splice()s after EOF return either EOF again or EAGAIN, and I can't really see it doing anything else.

Given that, and the fact you're already testing, I guess it makes more sense to do a fixup on top, rather than respinning?

But the commit message of this one would still be misleading because we're not actually "moving" the condition, and the fix-up would just add back these four lines (I guess?), so I think it would be more practical if you could send a new version of 8/8 only (you can base it on 7/8). -- Stefano