Message ID | 20100928224941.GA19409@vigoh |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
From: "Gustavo F. Padovan" <padovan@profusion.mobi> Date: Tue, 28 Sep 2010 19:49:41 -0300 > Actually sk_stream_wait_memory is another point why it's safe to release > the lock and block waiting for memory. We've been doing that safely in > protocols like TCP, SCTP and DCCP for a long time. Do you notice what TCP does when sk_stream_wait_memory() returns? It reloads all volatile state that might have changed in the socket while the lock was dropped. For example, TCP will reload the current MSS that can change asynchronously while we don't have the socket lock. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Dave, * David Miller <davem@davemloft.net> [2010-09-30 17:26:57 -0700]: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > Date: Tue, 28 Sep 2010 19:49:41 -0300 > > > Actually sk_stream_wait_memory is another point why it's safe to release > > the lock and block waiting for memory. We've been doing that safely in > > protocols like TCP, SCTP and DCCP for a long time. > > Do you notice what TCP does when sk_stream_wait_memory() returns? > > It reloads all volatile state that might have changed in the socket > while the lock was dropped. > > For example, TCP will reload the current MSS that can change > asynchronously while we don't have the socket lock. I got your point. And what I tried to say in the last e-mail is that ERTM doesn't have such volatile states that need to restore after get the lock back. The others code path it affect are very simple and also doesn't have such problem. So we are safe against asynchronous changes. We obvious have volatiles states, but the code paths where bt_skb_send_alloc() is used doesn't rely on that states. I'm seeing no problem on release the lock, alloc memory, and lock it again.
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 27a902d..e8d64ba 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -161,12 +161,30 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk, unsigned long l { struct sk_buff *skb; + release_sock(sk); if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) { skb_reserve(skb, BT_SKB_RESERVE); bt_cb(skb)->incoming = 0; } + lock_sock(sk); + + if (!skb && *err) + return NULL; + + *err = sock_error(sk); + if (*err) + goto out; + + if (sk->sk_shutdown) { + *err = ECONNRESET; + goto out; + } return skb; + +out: + kfree_skb(skb); + return NULL; } int bt_err(__u16 code);