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

[Stefano Brivio <sbrivio@redhat.com>]

On Thu, 28 May 2026 15:02:13 +1000
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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?


>  	}
>  
>  	/* We need write-side wakeups if and only if we have data in the pipe to

-- 
Stefano