net: fix a lockdep splat
We have for each socket : One spinlock (sk_slock.slock) One rwlock (sk_callback_lock) Possible scenarios are : (A) (this is used in net/sunrpc/xprtsock.c) read_lock(&sk->sk_callback_lock) (without blocking BH) <BH> spin_lock(&sk->sk_slock.slock); ... read_lock(&sk->sk_callback_lock); ... (B) write_lock_bh(&sk->sk_callback_lock) stuff write_unlock_bh(&sk->sk_callback_lock) (C) spin_lock_bh(&sk->sk_slock) ... write_lock_bh(&sk->sk_callback_lock) stuff write_unlock_bh(&sk->sk_callback_lock) spin_unlock_bh(&sk->sk_slock) This (C) case conflicts with (A) : CPU1 [A] CPU2 [C] read_lock(callback_lock) <BH> spin_lock_bh(slock) <wait to spin_lock(slock)> <wait to write_lock_bh(callback_lock)> We have one problematic (C) use case in inet_csk_listen_stop() : local_bh_disable(); bh_lock_sock(child); // spin_lock_bh(&sk->sk_slock) WARN_ON(sock_owned_by_user(child)); ... sock_orphan(child); // write_lock_bh(&sk->sk_callback_lock) lockdep is not happy with this, as reported by Tetsuo Handa It seems only way to deal with this is to use read_lock_bh(callbacklock) everywhere. Thanks to Jarek for pointing a bug in my first attempt and suggesting this solution. Reported-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Tested-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> CC: Jarek Poplawski <jarkao2@gmail.com> Tested-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
committed by
David S. Miller
parent
605c82bab5
commit
f064af1e50
@@ -800,7 +800,7 @@ static void xs_udp_data_ready(struct sock *sk, int len)
|
||||
u32 _xid;
|
||||
__be32 *xp;
|
||||
|
||||
read_lock(&sk->sk_callback_lock);
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
dprintk("RPC: xs_udp_data_ready...\n");
|
||||
if (!(xprt = xprt_from_sock(sk)))
|
||||
goto out;
|
||||
@@ -852,7 +852,7 @@ static void xs_udp_data_ready(struct sock *sk, int len)
|
||||
dropit:
|
||||
skb_free_datagram(sk, skb);
|
||||
out:
|
||||
read_unlock(&sk->sk_callback_lock);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
static inline void xs_tcp_read_fraghdr(struct rpc_xprt *xprt, struct xdr_skb_reader *desc)
|
||||
@@ -1229,7 +1229,7 @@ static void xs_tcp_data_ready(struct sock *sk, int bytes)
|
||||
|
||||
dprintk("RPC: xs_tcp_data_ready...\n");
|
||||
|
||||
read_lock(&sk->sk_callback_lock);
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
if (!(xprt = xprt_from_sock(sk)))
|
||||
goto out;
|
||||
if (xprt->shutdown)
|
||||
@@ -1248,7 +1248,7 @@ static void xs_tcp_data_ready(struct sock *sk, int bytes)
|
||||
read = tcp_read_sock(sk, &rd_desc, xs_tcp_data_recv);
|
||||
} while (read > 0);
|
||||
out:
|
||||
read_unlock(&sk->sk_callback_lock);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
/*
|
||||
@@ -1301,7 +1301,7 @@ static void xs_tcp_state_change(struct sock *sk)
|
||||
{
|
||||
struct rpc_xprt *xprt;
|
||||
|
||||
read_lock(&sk->sk_callback_lock);
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
if (!(xprt = xprt_from_sock(sk)))
|
||||
goto out;
|
||||
dprintk("RPC: xs_tcp_state_change client %p...\n", xprt);
|
||||
@@ -1313,7 +1313,7 @@ static void xs_tcp_state_change(struct sock *sk)
|
||||
|
||||
switch (sk->sk_state) {
|
||||
case TCP_ESTABLISHED:
|
||||
spin_lock_bh(&xprt->transport_lock);
|
||||
spin_lock(&xprt->transport_lock);
|
||||
if (!xprt_test_and_set_connected(xprt)) {
|
||||
struct sock_xprt *transport = container_of(xprt,
|
||||
struct sock_xprt, xprt);
|
||||
@@ -1327,7 +1327,7 @@ static void xs_tcp_state_change(struct sock *sk)
|
||||
|
||||
xprt_wake_pending_tasks(xprt, -EAGAIN);
|
||||
}
|
||||
spin_unlock_bh(&xprt->transport_lock);
|
||||
spin_unlock(&xprt->transport_lock);
|
||||
break;
|
||||
case TCP_FIN_WAIT1:
|
||||
/* The client initiated a shutdown of the socket */
|
||||
@@ -1365,7 +1365,7 @@ static void xs_tcp_state_change(struct sock *sk)
|
||||
xs_sock_mark_closed(xprt);
|
||||
}
|
||||
out:
|
||||
read_unlock(&sk->sk_callback_lock);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1376,7 +1376,7 @@ static void xs_error_report(struct sock *sk)
|
||||
{
|
||||
struct rpc_xprt *xprt;
|
||||
|
||||
read_lock(&sk->sk_callback_lock);
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
if (!(xprt = xprt_from_sock(sk)))
|
||||
goto out;
|
||||
dprintk("RPC: %s client %p...\n"
|
||||
@@ -1384,7 +1384,7 @@ static void xs_error_report(struct sock *sk)
|
||||
__func__, xprt, sk->sk_err);
|
||||
xprt_wake_pending_tasks(xprt, -EAGAIN);
|
||||
out:
|
||||
read_unlock(&sk->sk_callback_lock);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
static void xs_write_space(struct sock *sk)
|
||||
@@ -1416,13 +1416,13 @@ static void xs_write_space(struct sock *sk)
|
||||
*/
|
||||
static void xs_udp_write_space(struct sock *sk)
|
||||
{
|
||||
read_lock(&sk->sk_callback_lock);
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
|
||||
/* from net/core/sock.c:sock_def_write_space */
|
||||
if (sock_writeable(sk))
|
||||
xs_write_space(sk);
|
||||
|
||||
read_unlock(&sk->sk_callback_lock);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1437,13 +1437,13 @@ static void xs_udp_write_space(struct sock *sk)
|
||||
*/
|
||||
static void xs_tcp_write_space(struct sock *sk)
|
||||
{
|
||||
read_lock(&sk->sk_callback_lock);
|
||||
read_lock_bh(&sk->sk_callback_lock);
|
||||
|
||||
/* from net/core/stream.c:sk_stream_write_space */
|
||||
if (sk_stream_wspace(sk) >= sk_stream_min_wspace(sk))
|
||||
xs_write_space(sk);
|
||||
|
||||
read_unlock(&sk->sk_callback_lock);
|
||||
read_unlock_bh(&sk->sk_callback_lock);
|
||||
}
|
||||
|
||||
static void xs_udp_do_set_buffer_size(struct rpc_xprt *xprt)
|
||||
|
Reference in New Issue
Block a user