Re: [fixed] [patch] Re: [bug] stuck localhost TCP connections, v2.6.26-rc3+

Joe Perches
2008-06-03T23:57:25Z

On Wed, 2008-06-04 at 02:22 +0300, Ilpo Järvinen wrote:
> But here's somewhat more likely explanation... Only compile tested...
> It probably needs some commenting from people who understand locking
> variants & details (I don't).
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index c9454f0..d21d2b9 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4562,6 +4562,7 @@ static int tcp_defer_accept_check(struct sock *sk)
> struct tcp_sock *tp = tcp_sk(sk);
>
> if (tp->defer_tcp_accept.request) {
> + struct sock *listen_sk = tp->defer_tcp_accept.listen_sk;

Not commenting on the locking, but I think the code
would be clearer if tp->defer_tcp_accept was used in
a temporary instead.

Perhaps:

net/ipv4/tcp_input.c | 33 ++++++++++++++++-----------------
1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b54d9d3..f846e11 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4535,18 +4535,18 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, struct tcphdr *th)
static int tcp_defer_accept_check(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
-
- if (tp->defer_tcp_accept.request) {
- int queued_data = tp->rcv_nxt - tp->copied_seq;
- int hasfin = !skb_queue_empty(&sk->sk_receive_queue) ?
+ struct tcp_deferred_accept_info *tdai = &tp->defer_tcp_accept;
+ if (tdai->request) {
+ int queued_data = tp->rcv_nxt - tp->copied_seq;
+ int hasfin = !skb_queue_empty(&sk->sk_receive_queue) ?
tcp_hdr((struct sk_buff *)
sk->sk_receive_queue.prev)->fin : 0;

if (queued_data && hasfin)
queued_data--;

- if (queued_data &&
- tp->defer_tcp_accept.listen_sk->sk_state == TCP_LISTEN) {
+ bh_lock_sock(tdai->listen_sk);
+ if (queued_data && tdai->listen_sk->sk_state == TCP_LISTEN) {
if (sock_flag(sk, SOCK_KEEPOPEN)) {
inet_csk_reset_keepalive_timer(sk,
keepalive_time_when(tp));
@@ -4554,23 +4554,22 @@ static int tcp_defer_accept_check(struct sock *sk)
inet_csk_delete_keepalive_timer(sk);
}

- inet_csk_reqsk_queue_add(
- tp->defer_tcp_accept.listen_sk,
- tp->defer_tcp_accept.request,
- sk);
+ inet_csk_reqsk_queue_add(tdai->listen_sk,
+ tdai->request,
+ sk);

- tp->defer_tcp_accept.listen_sk->sk_data_ready(
- tp->defer_tcp_accept.listen_sk, 0);
+ tdai->listen_sk->sk_data_ready(tdai->listen_sk, 0);

- sock_put(tp->defer_tcp_accept.listen_sk);
+ sock_put(tdai->listen_sk);
sock_put(sk);
- tp->defer_tcp_accept.listen_sk = NULL;
- tp->defer_tcp_accept.request = NULL;
- } else if (hasfin ||
- tp->defer_tcp_accept.listen_sk->sk_state != TCP_LISTEN) {
+ tdai->listen_sk = NULL;
+ tdai->request = NULL;
+ } else if (hasfin || tdai->listen_sk->sk_state != TCP_LISTEN) {
+ bh_unlock_sock(tdai->listen_sk);
tcp_reset(sk);
return -1;
}
+ bh_unlock_sock(tdai->listen_sk);
}
return 0;
}