[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL+tcoBoe+YA12ZEn+-R2r4YYR3by1VsnN6GGmQoAtOPiej+BA@mail.gmail.com>
Date: Thu, 15 Aug 2024 18:48:16 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: sunyiqi <sunyiqixm@...il.com>
Cc: edumazet@...gle.com, davem@...emloft.net, dsahern@...nel.org,
kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: remove release/lock_sock in tcp_splice_read
On Thu, Aug 15, 2024 at 5:46 PM sunyiqi <sunyiqixm@...il.com> wrote:
>
> When enters tcp_splice_read, tcp_splice_read will call lock_sock
> for sk in order to prevent other threads acquiring sk and release it
> before return.
>
> But in while(tss.len) loop, it releases and re-locks sk, give the other
> thread a small window to lock the sk.
>
> As a result, release/lock_sock in the while loop in tcp_splice_read may
> cause race condition.
>
> Fixes: 9c55e01c0cc8 ("[TCP]: Splice receive support.")
> Signed-off-by: sunyiqi <sunyiqixm@...il.com>
It's more of an optimization instead of a BUG, no?
I don't consider it as a bug, unless you can prove it... Let me ask
what kind of race issues could re-lock cause?
I think holding the socket lock too long is not a good idea because
releasing the lock can give others breathing (having the chance to
finish their own stuff). Please see release_sock().
Thanks,
Jason
> ---
> net/ipv4/tcp.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index e03a342c9162..7a2ce0e2e5be 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -856,8 +856,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.34.1
>
>
Powered by blists - more mailing lists