netdev - Re: [PATCH net-next 1/2] net/tcp: optimise locking for blocking splice

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 23 May 2023 15:52:06 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Pavel Begunkov <asml.silence@...il.com>, netdev@...r.kernel.org, 
 edumazet@...gle.com, davem@...emloft.net, dsahern@...nel.org,
 kuba@...nel.org
Subject: Re: [PATCH net-next 1/2] net/tcp: optimise locking for blocking
 splice

On Fri, 2023-05-19 at 14:33 +0100, Pavel Begunkov wrote:
> Even when tcp_splice_read() reads all it was asked for, for blocking
> sockets it'll release and immediately regrab the socket lock, loop
> around and break on the while check.
> 
> Check tss.len right after we adjust it, and return if we're done.
> That saves us one release_sock(); lock_sock(); pair per successful
> blocking splice read.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@...il.com>
> ---
>  net/ipv4/tcp.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 4d6392c16b7a..bf7627f37e69 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -789,13 +789,15 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
>  	 */
>  	if (unlikely(*ppos))
>  		return -ESPIPE;
> +	if (unlikely(!tss.len))
> +		return 0;
>  
>  	ret = spliced = 0;
>  
>  	lock_sock(sk);
>  
>  	timeo = sock_rcvtimeo(sk, sock->file->f_flags & O_NONBLOCK);
> -	while (tss.len) {
> +	while (true) {
>  		ret = __tcp_splice_read(sk, &tss);
>  		if (ret < 0)
>  			break;
> @@ -835,10 +837,10 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
>  			}
>  			continue;
>  		}
> -		tss.len -= ret;
>  		spliced += ret;
> +		tss.len -= ret;

The patch LGTM. The only minor thing that I note is that the above
chunk is not needed. Perhaps avoiding unneeded delta could be worthy.

Cheers,

Paolo