Re: [PATCH v2 8/8] tcp_splice: Improve EOF and read stall exit conditions

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 3020 bytes --]

On Fri, Jun 05, 2026 at 12:59:42PM +0200, Stefano Brivio wrote:
> On Fri,  5 Jun 2026 10:34:16 +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 makes
> > sense: at EOF there's nothing left to do read-side and with nothing in the
> > pipe there's nothing to do write side either.
> > 
> > The same is true if the read side hit an EAGAIN and the pipe is empty:
> > there's nothing we can do (for now) read side, and with an empty pipe
> > nothing write side either.  So, generalise the condition to exit on either
> > EOF or EAGAIN read side.
> > 
> > Furthermore, if the read side is at EOF or EAGAIN and there's already
> > nothing in the pipe before the write-side splice(), then that write side
> > splice() can't accomplish anything, so exit the loop early in that case
> > avoiding a harmless but unnecessary write-splice().
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  tcp_splice.c | 20 +++++++++++++++-----
> >  1 file changed, 15 insertions(+), 5 deletions(-)
> > 
> > Changes in v2:
> >  * Duplicate rather than move the test, it's valuable in both places.
> >  * Make comments and commit message clearer
> > 
> > diff --git a/tcp_splice.c b/tcp_splice.c
> > index 565596d3..1e3c7749 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)
> > @@ -531,9 +539,11 @@ static int tcp_splice_forward(struct ctx *c,
> >  
> >  		conn->pending[fromsidei] -= written;
> >  
> > -		if (conn->events & FIN_RCVD(fromsidei) &&
> > -		    !conn->pending[fromsidei])
> > +		if (!conn->pending[fromsidei] && readlen <= 0) {
> > +			/* Read side is EOF or EAGAIN, and we emptied the pipe.
> > +			 * No more we can do for now, */
> 
> Changed "now," to "now.", added empty comment line, applied.

Oops, thanks.

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

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]