net: Fix percpu counters deadlock
authorHerbert Xu <herbert@gondor.apana.org.au>
Tue, 30 Dec 2008 07:04:08 +0000 (23:04 -0800)
committerDavid S. Miller <davem@davemloft.net>
Tue, 30 Dec 2008 07:04:08 +0000 (23:04 -0800)
When we converted the protocol atomic counters such as the orphan
count and the total socket count deadlocks were introduced due to
the mismatch in BH status of the spots that used the percpu counter
operations.

Based on the diagnosis and patch by Peter Zijlstra, this patch
fixes these issues by disabling BH where we may be in process
context.

Reported-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Tested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/dccp/proto.c
net/ipv4/inet_connection_sock.c
net/ipv4/proc.c
net/ipv4/tcp.c
net/ipv4/tcp_ipv4.c
net/ipv6/tcp_ipv6.c

index d5c2bacb713c6055ffb9a3ca9cb8975ebb907a9a..1747ccae8e8d098f50232253e7be60a40867848f 100644 (file)
@@ -964,7 +964,6 @@ adjudge_to_death:
        state = sk->sk_state;
        sock_hold(sk);
        sock_orphan(sk);
-       percpu_counter_inc(sk->sk_prot->orphan_count);
 
        /*
         * It is the last release_sock in its life. It will remove backlog.
@@ -978,6 +977,8 @@ adjudge_to_death:
        bh_lock_sock(sk);
        WARN_ON(sock_owned_by_user(sk));
 
+       percpu_counter_inc(sk->sk_prot->orphan_count);
+
        /* Have we already been destroyed by a softirq or backlog? */
        if (state != DCCP_CLOSED && sk->sk_state == DCCP_CLOSED)
                goto out;
index c7cda1ca8e6571232d1cf22c576a2c998983a08a..f26ab38680de00a5b77c303e34774a078022d499 100644 (file)
@@ -633,8 +633,6 @@ void inet_csk_listen_stop(struct sock *sk)
 
                acc_req = req->dl_next;
 
-               percpu_counter_inc(sk->sk_prot->orphan_count);
-
                local_bh_disable();
                bh_lock_sock(child);
                WARN_ON(sock_owned_by_user(child));
@@ -644,6 +642,8 @@ void inet_csk_listen_stop(struct sock *sk)
 
                sock_orphan(child);
 
+               percpu_counter_inc(sk->sk_prot->orphan_count);
+
                inet_csk_destroy_sock(child);
 
                bh_unlock_sock(child);
index 614958b7c27695aa6ed2a60822dd759b64648214..eb62e58bff7922b0e62d8f883da79fc0a8bee53b 100644 (file)
@@ -38,6 +38,7 @@
 #include <net/tcp.h>
 #include <net/udp.h>
 #include <net/udplite.h>
+#include <linux/bottom_half.h>
 #include <linux/inetdevice.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 static int sockstat_seq_show(struct seq_file *seq, void *v)
 {
        struct net *net = seq->private;
+       int orphans, sockets;
+
+       local_bh_disable();
+       orphans = percpu_counter_sum_positive(&tcp_orphan_count),
+       sockets = percpu_counter_sum_positive(&tcp_sockets_allocated),
+       local_bh_enable();
 
        socket_seq_show(seq);
        seq_printf(seq, "TCP: inuse %d orphan %d tw %d alloc %d mem %d\n",
-                  sock_prot_inuse_get(net, &tcp_prot),
-                  (int)percpu_counter_sum_positive(&tcp_orphan_count),
-                  tcp_death_row.tw_count,
-                  (int)percpu_counter_sum_positive(&tcp_sockets_allocated),
+                  sock_prot_inuse_get(net, &tcp_prot), orphans,
+                  tcp_death_row.tw_count, sockets,
                   atomic_read(&tcp_memory_allocated));
        seq_printf(seq, "UDP: inuse %d mem %d\n",
                   sock_prot_inuse_get(net, &udp_prot),
index 1f3d52946b3b84f604385fb5542390bb6b593ce1..f28acf11fc67d419bc0ccab57510c8bcedee1c3e 100644 (file)
@@ -1836,7 +1836,6 @@ adjudge_to_death:
        state = sk->sk_state;
        sock_hold(sk);
        sock_orphan(sk);
-       percpu_counter_inc(sk->sk_prot->orphan_count);
 
        /* It is the last release_sock in its life. It will remove backlog. */
        release_sock(sk);
@@ -1849,6 +1848,8 @@ adjudge_to_death:
        bh_lock_sock(sk);
        WARN_ON(sock_owned_by_user(sk));
 
+       percpu_counter_inc(sk->sk_prot->orphan_count);
+
        /* Have we already been destroyed by a softirq or backlog? */
        if (state != TCP_CLOSE && sk->sk_state == TCP_CLOSE)
                goto out;
index 10172487921b944d3f82b1d527b4674a9af449a4..9d839fa9331e5845c39b22d07d9d3a93f39634d3 100644 (file)
@@ -51,6 +51,7 @@
  */
 
 
+#include <linux/bottom_half.h>
 #include <linux/types.h>
 #include <linux/fcntl.h>
 #include <linux/module.h>
@@ -1797,7 +1798,9 @@ static int tcp_v4_init_sock(struct sock *sk)
        sk->sk_sndbuf = sysctl_tcp_wmem[1];
        sk->sk_rcvbuf = sysctl_tcp_rmem[1];
 
+       local_bh_disable();
        percpu_counter_inc(&tcp_sockets_allocated);
+       local_bh_enable();
 
        return 0;
 }
index 8702b06cb60a9dbe6b1d7490bd30656ed79e141c..e8b8337a83107d8a2ccafeac7a6539c1a7621d38 100644 (file)
@@ -23,6 +23,7 @@
  *      2 of the License, or (at your option) any later version.
  */
 
+#include <linux/bottom_half.h>
 #include <linux/module.h>
 #include <linux/errno.h>
 #include <linux/types.h>
@@ -1830,7 +1831,9 @@ static int tcp_v6_init_sock(struct sock *sk)
        sk->sk_sndbuf = sysctl_tcp_wmem[1];
        sk->sk_rcvbuf = sysctl_tcp_rmem[1];
 
+       local_bh_disable();
        percpu_counter_inc(&tcp_sockets_allocated);
+       local_bh_enable();
 
        return 0;
 }