From: Geliang Tang <tanggeliang@kylinos.cn>

In the main loop of tcp_splice_read(), release_sock() is invoked to release
the socket lock, and then lock_sock() is immediately invoked to hold the
socket lock again.

It looks like these two calls are useless, so let's drop them.

Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
 net/ipv4/tcp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index c9eb94a6c41a..c6425f13afd0 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -866,8 +866,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 
 		if (!tss.len || !timeo)
 			break;
-		release_sock(sk);
-		lock_sock(sk);
 
 		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
 		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-- 
2.48.1

On Tue, 2 Sep 2025, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> In the main loop of tcp_splice_read(), release_sock() is invoked to release
> the socket lock, and then lock_sock() is immediately invoked to hold the
> socket lock again.
>
> It looks like these two calls are useless, so let's drop them.

Hi Geliang -

I think the intent here is to allow incoming packets to be processed from 
the backlog before __tcp_splice_read() is called on the next loop 
iteration, so there's a chance that more data can be processed in a single 
call to tcp_splice_read().

Was this lock reacquisition causing any issues?

Thanks,

Mat


>
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> net/ipv4/tcp.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index c9eb94a6c41a..c6425f13afd0 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -866,8 +866,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
>
> 		if (!tss.len || !timeo)
> 			break;
> -		release_sock(sk);
> -		lock_sock(sk);
>
> 		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
> 		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
> -- 
> 2.48.1
>
>
>

On Fri, 2025-09-12 at 16:34 -0700, Mat Martineau wrote:
> On Tue, 2 Sep 2025, Geliang Tang wrote:
> 
> > From: Geliang Tang <tanggeliang@kylinos.cn>
> > 
> > In the main loop of tcp_splice_read(), release_sock() is invoked to
> > release
> > the socket lock, and then lock_sock() is immediately invoked to
> > hold the
> > socket lock again.
> > 
> > It looks like these two calls are useless, so let's drop them.
> 
> Hi Geliang -
> 
> I think the intent here is to allow incoming packets to be processed
> from 
> the backlog before __tcp_splice_read() is called on the next loop 
> iteration, so there's a chance that more data can be processed in a
> single 
> call to tcp_splice_read().

Thanks Mat, I didn't understand the intent of this code before.

> 
> Was this lock reacquisition causing any issues?

No, I just dropped this patch in v11, and add this 'release and lock
again' code in mptcp_splice_read() too.

-Geliang

> 
> Thanks,
> 
> Mat
> 
> 
> > 
> > Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> > ---
> > net/ipv4/tcp.c | 2 --
> > 1 file changed, 2 deletions(-)
> > 
> > diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> > index c9eb94a6c41a..c6425f13afd0 100644
> > --- a/net/ipv4/tcp.c
> > +++ b/net/ipv4/tcp.c
> > @@ -866,8 +866,6 @@ ssize_t tcp_splice_read(struct socket *sock,
> > loff_t *ppos,
> > 
> > 		if (!tss.len || !timeo)
> > 			break;
> > -		release_sock(sk);
> > -		lock_sock(sk);
> > 
> > 		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
> > 		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
> > -- 
> > 2.48.1
> > 
> > 
> >