All of lore.kernel.org
 help / color / mirror / Atom feed
* Pull resquest review to 2.6.36-rc5
@ 2010-09-25  1:01 Gustavo F. Padovan
  2010-09-25  1:01 ` [PATCH 1/6] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth

Hi all,

These are the patches we have queued for the Bluetooth subsystem intended to be
pushed into 2.6.36. We are bit late already, please make any comment as soon as
possible then  we can hurry and pull those to wireless-2.6 on Monday maybe.


The following changes since commit b30a3f6257ed2105259b404d419b4964e363928c:

  Linux 2.6.36-rc5 (2010-09-20 16:56:53 -0700)

are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git master

Andrei Emeltchenko (1):
      Bluetooth: fix MTU L2CAP configuration parameter

Gustavo F. Padovan (4):
      Bluetooth: Simplify L2CAP Streaming mode sending
      Bluetooth: Fix inconsistent lock state with RFCOMM
      Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state"
      Bluetooth: Fix deadlock in the ERTM logic

Mat Martineau (1):
      Bluetooth: Only enable for L2CAP FCS for ERTM or streaming

 include/net/bluetooth/bluetooth.h |   11 +++++++
 net/bluetooth/l2cap.c             |   57 +++++++++++++++---------------------
 net/bluetooth/rfcomm/sock.c       |    4 ++
 3 files changed, 39 insertions(+), 33 deletions(-)



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming
  2010-09-25  1:01 Pull resquest review to 2.6.36-rc5 Gustavo F. Padovan
@ 2010-09-25  1:01 ` Gustavo F. Padovan
  2010-09-25  1:01   ` [PATCH 2/6] Bluetooth: fix MTU L2CAP configuration parameter Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Mat Martineau

From: Mat Martineau <mathewm@codeaurora.org>

This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16
with L2CAP modes other than ERTM or streaming.  At present, this only
affects the FCS value shown with getsockopt() for basic mode.

Signed-off-by: Mat Martineau <mathewm@codeaurora.org>
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index fadf26b..c784703 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3071,6 +3071,17 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
 	return 0;
 }
 
+static inline void set_default_fcs(struct l2cap_pinfo *pi)
+{
+	/* FCS is enabled only in ERTM or streaming mode, if one or both
+	 * sides request it.
+	 */
+	if (pi->mode != L2CAP_MODE_ERTM && pi->mode != L2CAP_MODE_STREAMING)
+		pi->fcs = L2CAP_FCS_NONE;
+	else if (!(pi->conf_state & L2CAP_CONF_NO_FCS_RECV))
+		pi->fcs = L2CAP_FCS_CRC16;
+}
+
 static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u16 cmd_len, u8 *data)
 {
 	struct l2cap_conf_req *req = (struct l2cap_conf_req *) data;
@@ -3135,9 +3146,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 		goto unlock;
 
 	if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
-		if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
-		    l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
-			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
+		set_default_fcs(l2cap_pi(sk));
 
 		sk->sk_state = BT_CONNECTED;
 
@@ -3225,9 +3234,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 	l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE;
 
 	if (l2cap_pi(sk)->conf_state & L2CAP_CONF_OUTPUT_DONE) {
-		if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
-		    l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
-			l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
+		set_default_fcs(l2cap_pi(sk));
 
 		sk->sk_state = BT_CONNECTED;
 		l2cap_pi(sk)->next_tx_seq = 0;
-- 
1.7.2.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/6] Bluetooth: fix MTU L2CAP configuration parameter
  2010-09-25  1:01 ` [PATCH 1/6] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming Gustavo F. Padovan
@ 2010-09-25  1:01   ` Gustavo F. Padovan
  2010-09-25  1:01     ` [PATCH 3/6] Bluetooth: Simplify L2CAP Streaming mode sending Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Andrei Emeltchenko

From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>

When receiving L2CAP negative configuration response with respect
to MTU parameter we modify wrong field. MTU here means proposed
value of MTU that the remote device intends to transmit. So for local
L2CAP socket it is pi->imtu.

Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Acked-by: Ville Tervo <ville.tervo@nokia.com>
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c784703..9fad312 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2771,10 +2771,10 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data,
 		case L2CAP_CONF_MTU:
 			if (val < L2CAP_DEFAULT_MIN_MTU) {
 				*result = L2CAP_CONF_UNACCEPT;
-				pi->omtu = L2CAP_DEFAULT_MIN_MTU;
+				pi->imtu = L2CAP_DEFAULT_MIN_MTU;
 			} else
-				pi->omtu = val;
-			l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->omtu);
+				pi->imtu = val;
+			l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu);
 			break;
 
 		case L2CAP_CONF_FLUSH_TO:
-- 
1.7.2.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/6] Bluetooth: Simplify L2CAP Streaming mode sending
  2010-09-25  1:01   ` [PATCH 2/6] Bluetooth: fix MTU L2CAP configuration parameter Gustavo F. Padovan
@ 2010-09-25  1:01     ` Gustavo F. Padovan
  2010-09-25  1:01       ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Gustavo F. Padovan
  0 siblings, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

As we don't have any error control on the Streaming mode, i.e., we don't
need to keep a copy of the skb for later resending we don't need to
call skb_clone() on it.
Then we can go one further here, and dequeue the skb before sending it,
that also means we don't need to look to sk->sk_send_head anymore.

The patch saves memory and time when sending Streaming mode data, so
it is good to mainline.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |   24 +++++++-----------------
 1 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 9fad312..f2062aa 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1441,33 +1441,23 @@ static inline void l2cap_do_send(struct sock *sk, struct sk_buff *skb)
 
 static void l2cap_streaming_send(struct sock *sk)
 {
-	struct sk_buff *skb, *tx_skb;
+	struct sk_buff *skb;
 	struct l2cap_pinfo *pi = l2cap_pi(sk);
 	u16 control, fcs;
 
-	while ((skb = sk->sk_send_head)) {
-		tx_skb = skb_clone(skb, GFP_ATOMIC);
-
-		control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
+	while ((skb = skb_dequeue(TX_QUEUE(sk)))) {
+		control = get_unaligned_le16(skb->data + L2CAP_HDR_SIZE);
 		control |= pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT;
-		put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
+		put_unaligned_le16(control, skb->data + L2CAP_HDR_SIZE);
 
 		if (pi->fcs == L2CAP_FCS_CRC16) {
-			fcs = crc16(0, (u8 *)tx_skb->data, tx_skb->len - 2);
-			put_unaligned_le16(fcs, tx_skb->data + tx_skb->len - 2);
+			fcs = crc16(0, (u8 *)skb->data, skb->len - 2);
+			put_unaligned_le16(fcs, skb->data + skb->len - 2);
 		}
 
-		l2cap_do_send(sk, tx_skb);
+		l2cap_do_send(sk, skb);
 
 		pi->next_tx_seq = (pi->next_tx_seq + 1) % 64;
-
-		if (skb_queue_is_last(TX_QUEUE(sk), skb))
-			sk->sk_send_head = NULL;
-		else
-			sk->sk_send_head = skb_queue_next(TX_QUEUE(sk), skb);
-
-		skb = skb_dequeue(TX_QUEUE(sk));
-		kfree_skb(skb);
 	}
 }
 
-- 
1.7.2.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM
  2010-09-25  1:01     ` [PATCH 3/6] Bluetooth: Simplify L2CAP Streaming mode sending Gustavo F. Padovan
@ 2010-09-25  1:01       ` Gustavo F. Padovan
  2010-09-25  1:01         ` [PATCH 5/6] Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state" Gustavo F. Padovan
  2010-09-29  7:41         ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Yuri Kululin
  0 siblings, 2 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

When receiving a rfcomm connection with the old dund deamon a
inconsistent lock state happens. That's because interrupts were already
disabled by l2cap_conn_start() when rfcomm_sk_state_change() try to lock
the spin_lock.

As result we may have a inconsistent lock state for l2cap_conn_start()
after rfcomm_sk_state_change() calls bh_lock_sock() and disable interrupts
as well.

[ 2833.151999]
[ 2833.151999] =================================
[ 2833.151999] [ INFO: inconsistent lock state ]
[ 2833.151999] 2.6.36-rc3 #2
[ 2833.151999] ---------------------------------
[ 2833.151999] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[ 2833.151999] krfcommd/2306 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 2833.151999]  (slock-AF_BLUETOOTH){+.?...}, at: [<ffffffffa00bcb56>] rfcomm_sk_state_change+0x46/0x170 [rfcomm]
[ 2833.151999] {IN-SOFTIRQ-W} state was registered at:
[ 2833.151999]   [<ffffffff81094346>] __lock_acquire+0x5b6/0x1560
[ 2833.151999]   [<ffffffff8109534a>] lock_acquire+0x5a/0x70
[ 2833.151999]   [<ffffffff81392b6c>] _raw_spin_lock+0x2c/0x40
[ 2833.151999]   [<ffffffffa00a5092>] l2cap_conn_start+0x92/0x640 [l2cap]
[ 2833.151999]   [<ffffffffa00a6a3f>] l2cap_sig_channel+0x6bf/0x1320 [l2cap]
[ 2833.151999]   [<ffffffffa00a9173>] l2cap_recv_frame+0x133/0x770 [l2cap]
[ 2833.151999]   [<ffffffffa00a997b>] l2cap_recv_acldata+0x1cb/0x390 [l2cap]
[ 2833.151999]   [<ffffffffa000db4b>] hci_rx_task+0x2ab/0x450 [bluetooth]
[ 2833.151999]   [<ffffffff8106b22b>] tasklet_action+0xcb/0xe0
[ 2833.151999]   [<ffffffff8106b91e>] __do_softirq+0xae/0x150
[ 2833.151999]   [<ffffffff8102bc0c>] call_softirq+0x1c/0x30
[ 2833.151999]   [<ffffffff8102ddb5>] do_softirq+0x75/0xb0
[ 2833.151999]   [<ffffffff8106b56d>] irq_exit+0x8d/0xa0
[ 2833.151999]   [<ffffffff8104484b>] smp_apic_timer_interrupt+0x6b/0xa0
[ 2833.151999]   [<ffffffff8102b6d3>] apic_timer_interrupt+0x13/0x20
[ 2833.151999]   [<ffffffff81029dfa>] cpu_idle+0x5a/0xb0
[ 2833.151999]   [<ffffffff81381ded>] rest_init+0xad/0xc0
[ 2833.151999]   [<ffffffff817ebc4d>] start_kernel+0x2dd/0x2e8
[ 2833.151999]   [<ffffffff817eb2e6>] x86_64_start_reservations+0xf6/0xfa
[ 2833.151999]   [<ffffffff817eb3ce>] x86_64_start_kernel+0xe4/0xeb
[ 2833.151999] irq event stamp: 731
[ 2833.151999] hardirqs last  enabled at (731): [<ffffffff8106b762>] local_bh_enable_ip+0x82/0xe0
[ 2833.151999] hardirqs last disabled at (729): [<ffffffff8106b93e>] __do_softirq+0xce/0x150
[ 2833.151999] softirqs last  enabled at (730): [<ffffffff8106b96e>] __do_softirq+0xfe/0x150
[ 2833.151999] softirqs last disabled at (711): [<ffffffff8102bc0c>] call_softirq+0x1c/0x30
[ 2833.151999]
[ 2833.151999] other info that might help us debug this:
[ 2833.151999] 2 locks held by krfcommd/2306:
[ 2833.151999]  #0:  (rfcomm_mutex){+.+.+.}, at: [<ffffffffa00bb744>] rfcomm_run+0x174/0xb20 [rfcomm]
[ 2833.151999]  #1:  (&(&d->lock)->rlock){+.+...}, at: [<ffffffffa00b9223>] rfcomm_dlc_accept+0x53/0x100 [rfcomm]
[ 2833.151999]
[ 2833.151999] stack backtrace:
[ 2833.151999] Pid: 2306, comm: krfcommd Tainted: G        W   2.6.36-rc3 #2
[ 2833.151999] Call Trace:
[ 2833.151999]  [<ffffffff810928e1>] print_usage_bug+0x171/0x180
[ 2833.151999]  [<ffffffff810936c3>] mark_lock+0x333/0x400
[ 2833.151999]  [<ffffffff810943ca>] __lock_acquire+0x63a/0x1560
[ 2833.151999]  [<ffffffff810948b5>] ? __lock_acquire+0xb25/0x1560
[ 2833.151999]  [<ffffffff8109534a>] lock_acquire+0x5a/0x70
[ 2833.151999]  [<ffffffffa00bcb56>] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm]
[ 2833.151999]  [<ffffffff81392b6c>] _raw_spin_lock+0x2c/0x40
[ 2833.151999]  [<ffffffffa00bcb56>] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm]
[ 2833.151999]  [<ffffffffa00bcb56>] rfcomm_sk_state_change+0x46/0x170 [rfcomm]
[ 2833.151999]  [<ffffffffa00b9239>] rfcomm_dlc_accept+0x69/0x100 [rfcomm]
[ 2833.151999]  [<ffffffffa00b9a49>] rfcomm_check_accept+0x59/0xd0 [rfcomm]
[ 2833.151999]  [<ffffffffa00bacab>] rfcomm_recv_frame+0x9fb/0x1320 [rfcomm]
[ 2833.151999]  [<ffffffff813932bb>] ? _raw_spin_unlock_irqrestore+0x3b/0x60
[ 2833.151999]  [<ffffffff81093acd>] ? trace_hardirqs_on_caller+0x13d/0x180
[ 2833.151999]  [<ffffffff81093b1d>] ? trace_hardirqs_on+0xd/0x10
[ 2833.151999]  [<ffffffffa00bb7f1>] rfcomm_run+0x221/0xb20 [rfcomm]
[ 2833.151999]  [<ffffffff813905e7>] ? schedule+0x287/0x780
[ 2833.151999]  [<ffffffffa00bb5d0>] ? rfcomm_run+0x0/0xb20 [rfcomm]
[ 2833.151999]  [<ffffffff81081026>] kthread+0x96/0xa0
[ 2833.151999]  [<ffffffff8102bb14>] kernel_thread_helper+0x4/0x10
[ 2833.151999]  [<ffffffff813936bc>] ? restore_args+0x0/0x30
[ 2833.151999]  [<ffffffff81080f90>] ? kthread+0x0/0xa0
[ 2833.151999]  [<ffffffff8102bb10>] ? kernel_thread_helper+0x0/0x10

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/rfcomm/sock.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index 44a6232..194b3a0 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -82,11 +82,14 @@ static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb)
 static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
 {
 	struct sock *sk = d->owner, *parent;
+	unsigned long flags;
+
 	if (!sk)
 		return;
 
 	BT_DBG("dlc %p state %ld err %d", d, d->state, err);
 
+	local_irq_save(flags);
 	bh_lock_sock(sk);
 
 	if (err)
@@ -108,6 +111,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
 	}
 
 	bh_unlock_sock(sk);
+	local_irq_restore(flags);
 
 	if (parent && sock_flag(sk, SOCK_ZAPPED)) {
 		/* We have to drop DLC lock here, otherwise
-- 
1.7.2.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 5/6] Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state"
  2010-09-25  1:01       ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Gustavo F. Padovan
@ 2010-09-25  1:01         ` Gustavo F. Padovan
  2010-09-25  1:01           ` [PATCH 6/6] Bluetooth: Fix deadlock in the ERTM logic Gustavo F. Padovan
  2010-09-29  7:41         ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Yuri Kululin
  1 sibling, 1 reply; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

This reverts commit 8cb8e6f1684be13b51f8429b15f39c140326b327.

That commit introduced a regression with the Bluetooth Profile Tuning
Suite(PTS), Reverting this make sure that L2CAP is in a qualificable
state.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
 net/bluetooth/l2cap.c |    8 +-------
 1 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index f2062aa..44a8fb0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3089,14 +3089,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 	if (!sk)
 		return -ENOENT;
 
-	if (sk->sk_state != BT_CONFIG) {
-		struct l2cap_cmd_rej rej;
-
-		rej.reason = cpu_to_le16(0x0002);
-		l2cap_send_cmd(conn, cmd->ident, L2CAP_COMMAND_REJ,
-				sizeof(rej), &rej);
+	if (sk->sk_state == BT_DISCONN)
 		goto unlock;
-	}
 
 	/* Reject if config buffer is too small. */
 	len = cmd_len - sizeof(*req);
-- 
1.7.2.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 6/6] Bluetooth: Fix deadlock in the ERTM logic
  2010-09-25  1:01         ` [PATCH 5/6] Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state" Gustavo F. Padovan
@ 2010-09-25  1:01           ` Gustavo F. Padovan
  0 siblings, 0 replies; 8+ messages in thread
From: Gustavo F. Padovan @ 2010-09-25  1:01 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: Gustavo F. Padovan <padovan@profusion.mobi>

The Enhanced Retransmission Mode(ERTM) is a realiable mode of operation
of the Bluetooth L2CAP layer. Think on it like a simplified version of
TCP.
The problem we were facing here was a deadlock. ERTM uses a backlog
queue to queue incomimg packets while the user is helding the lock. At
some moment the sk_sndbuf can be exceeded and we can't alloc new skbs
then the code sleep with the lock to wait for memory, that stalls the
ERTM connection once we can't read the acknowledgements packets in the
backlog queue to free memory and make the allocation of outcoming skb
successful.

This patch actually affect all users of bt_skb_send_alloc(), i.e., all
L2CAP modes and SCO.

Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Signed-off-by: Ulisses Furquim <ulisses@profusion.mobi>
---
 include/net/bluetooth/bluetooth.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..118b69b 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -161,10 +161,21 @@ 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 (*err)
+		return NULL;
+
+	*err = sock_error(sk);
+	if (*err) {
+		kfree_skb(skb);
+		return NULL;
+	}
 
 	return skb;
 }
-- 
1.7.2.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM
  2010-09-25  1:01       ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Gustavo F. Padovan
  2010-09-25  1:01         ` [PATCH 5/6] Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state" Gustavo F. Padovan
@ 2010-09-29  7:41         ` Yuri Kululin
  1 sibling, 0 replies; 8+ messages in thread
From: Yuri Kululin @ 2010-09-29  7:41 UTC (permalink / raw)
  To: ext Gustavo F. Padovan; +Cc: linux-bluetooth, Gustavo F. Padovan

ext Gustavo F. Padovan wrote:
> From: Gustavo F. Padovan <padovan@profusion.mobi>
> 
> When receiving a rfcomm connection with the old dund deamon a
> inconsistent lock state happens. That's because interrupts were already
> disabled by l2cap_conn_start() when rfcomm_sk_state_change() try to lock
> the spin_lock.
> 
> As result we may have a inconsistent lock state for l2cap_conn_start()
> after rfcomm_sk_state_change() calls bh_lock_sock() and disable interrupts
> as well.
> 
> [ 2833.151999]
> [ 2833.151999] =================================
> [ 2833.151999] [ INFO: inconsistent lock state ]
> [ 2833.151999] 2.6.36-rc3 #2
> [ 2833.151999] ---------------------------------
> [ 2833.151999] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
> [ 2833.151999] krfcommd/2306 [HC0[0]:SC0[0]:HE1:SE1] takes:
> [ 2833.151999]  (slock-AF_BLUETOOTH){+.?...}, at: [<ffffffffa00bcb56>] rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999] {IN-SOFTIRQ-W} state was registered at:
> [ 2833.151999]   [<ffffffff81094346>] __lock_acquire+0x5b6/0x1560
> [ 2833.151999]   [<ffffffff8109534a>] lock_acquire+0x5a/0x70
> [ 2833.151999]   [<ffffffff81392b6c>] _raw_spin_lock+0x2c/0x40
> [ 2833.151999]   [<ffffffffa00a5092>] l2cap_conn_start+0x92/0x640 [l2cap]
> [ 2833.151999]   [<ffffffffa00a6a3f>] l2cap_sig_channel+0x6bf/0x1320 [l2cap]
> [ 2833.151999]   [<ffffffffa00a9173>] l2cap_recv_frame+0x133/0x770 [l2cap]
> [ 2833.151999]   [<ffffffffa00a997b>] l2cap_recv_acldata+0x1cb/0x390 [l2cap]
> [ 2833.151999]   [<ffffffffa000db4b>] hci_rx_task+0x2ab/0x450 [bluetooth]
> [ 2833.151999]   [<ffffffff8106b22b>] tasklet_action+0xcb/0xe0
> [ 2833.151999]   [<ffffffff8106b91e>] __do_softirq+0xae/0x150
> [ 2833.151999]   [<ffffffff8102bc0c>] call_softirq+0x1c/0x30
> [ 2833.151999]   [<ffffffff8102ddb5>] do_softirq+0x75/0xb0
> [ 2833.151999]   [<ffffffff8106b56d>] irq_exit+0x8d/0xa0
> [ 2833.151999]   [<ffffffff8104484b>] smp_apic_timer_interrupt+0x6b/0xa0
> [ 2833.151999]   [<ffffffff8102b6d3>] apic_timer_interrupt+0x13/0x20
> [ 2833.151999]   [<ffffffff81029dfa>] cpu_idle+0x5a/0xb0
> [ 2833.151999]   [<ffffffff81381ded>] rest_init+0xad/0xc0
> [ 2833.151999]   [<ffffffff817ebc4d>] start_kernel+0x2dd/0x2e8
> [ 2833.151999]   [<ffffffff817eb2e6>] x86_64_start_reservations+0xf6/0xfa
> [ 2833.151999]   [<ffffffff817eb3ce>] x86_64_start_kernel+0xe4/0xeb
> [ 2833.151999] irq event stamp: 731
> [ 2833.151999] hardirqs last  enabled at (731): [<ffffffff8106b762>] local_bh_enable_ip+0x82/0xe0
> [ 2833.151999] hardirqs last disabled at (729): [<ffffffff8106b93e>] __do_softirq+0xce/0x150
> [ 2833.151999] softirqs last  enabled at (730): [<ffffffff8106b96e>] __do_softirq+0xfe/0x150
> [ 2833.151999] softirqs last disabled at (711): [<ffffffff8102bc0c>] call_softirq+0x1c/0x30
> [ 2833.151999]
> [ 2833.151999] other info that might help us debug this:
> [ 2833.151999] 2 locks held by krfcommd/2306:
> [ 2833.151999]  #0:  (rfcomm_mutex){+.+.+.}, at: [<ffffffffa00bb744>] rfcomm_run+0x174/0xb20 [rfcomm]
> [ 2833.151999]  #1:  (&(&d->lock)->rlock){+.+...}, at: [<ffffffffa00b9223>] rfcomm_dlc_accept+0x53/0x100 [rfcomm]
> [ 2833.151999]
> [ 2833.151999] stack backtrace:
> [ 2833.151999] Pid: 2306, comm: krfcommd Tainted: G        W   2.6.36-rc3 #2
> [ 2833.151999] Call Trace:
> [ 2833.151999]  [<ffffffff810928e1>] print_usage_bug+0x171/0x180
> [ 2833.151999]  [<ffffffff810936c3>] mark_lock+0x333/0x400
> [ 2833.151999]  [<ffffffff810943ca>] __lock_acquire+0x63a/0x1560
> [ 2833.151999]  [<ffffffff810948b5>] ? __lock_acquire+0xb25/0x1560
> [ 2833.151999]  [<ffffffff8109534a>] lock_acquire+0x5a/0x70
> [ 2833.151999]  [<ffffffffa00bcb56>] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999]  [<ffffffff81392b6c>] _raw_spin_lock+0x2c/0x40
> [ 2833.151999]  [<ffffffffa00bcb56>] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999]  [<ffffffffa00bcb56>] rfcomm_sk_state_change+0x46/0x170 [rfcomm]
> [ 2833.151999]  [<ffffffffa00b9239>] rfcomm_dlc_accept+0x69/0x100 [rfcomm]
> [ 2833.151999]  [<ffffffffa00b9a49>] rfcomm_check_accept+0x59/0xd0 [rfcomm]
> [ 2833.151999]  [<ffffffffa00bacab>] rfcomm_recv_frame+0x9fb/0x1320 [rfcomm]
> [ 2833.151999]  [<ffffffff813932bb>] ? _raw_spin_unlock_irqrestore+0x3b/0x60
> [ 2833.151999]  [<ffffffff81093acd>] ? trace_hardirqs_on_caller+0x13d/0x180
> [ 2833.151999]  [<ffffffff81093b1d>] ? trace_hardirqs_on+0xd/0x10
> [ 2833.151999]  [<ffffffffa00bb7f1>] rfcomm_run+0x221/0xb20 [rfcomm]
> [ 2833.151999]  [<ffffffff813905e7>] ? schedule+0x287/0x780
> [ 2833.151999]  [<ffffffffa00bb5d0>] ? rfcomm_run+0x0/0xb20 [rfcomm]
> [ 2833.151999]  [<ffffffff81081026>] kthread+0x96/0xa0
> [ 2833.151999]  [<ffffffff8102bb14>] kernel_thread_helper+0x4/0x10
> [ 2833.151999]  [<ffffffff813936bc>] ? restore_args+0x0/0x30
> [ 2833.151999]  [<ffffffff81080f90>] ? kthread+0x0/0xa0
> [ 2833.151999]  [<ffffffff8102bb10>] ? kernel_thread_helper+0x0/0x10
> 
> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
> ---
>  net/bluetooth/rfcomm/sock.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
> index 44a6232..194b3a0 100644
> --- a/net/bluetooth/rfcomm/sock.c
> +++ b/net/bluetooth/rfcomm/sock.c
> @@ -82,11 +82,14 @@ static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb)
>  static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
>  {
>  	struct sock *sk = d->owner, *parent;
> +	unsigned long flags;
> +
>  	if (!sk)
>  		return;
>  
>  	BT_DBG("dlc %p state %ld err %d", d, d->state, err);
>  
> +	local_irq_save(flags);
>  	bh_lock_sock(sk);
>  
>  	if (err)
> @@ -108,6 +111,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err)
>  	}
>  
>  	bh_unlock_sock(sk);
> +	local_irq_restore(flags);
>  
>  	if (parent && sock_flag(sk, SOCK_ZAPPED)) {
>  		/* We have to drop DLC lock here, otherwise

I've faced with the similar warning and looks like this patch fixes the issue.

[ 2917.827178] =================================
[ 2917.833068] [ INFO: inconsistent lock state ]
[ 2917.837432] 2.6.32-09421-g65e7ba7 #54
[ 2917.841125] ---------------------------------
[ 2917.845520] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
[ 2917.851562] krfcommd/1516 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 2917.856903]  (slock-AF_BLUETOOTH){+.?...}, at: [<bf057b50>]
rfcomm_sk_state_change+0x78/0x160 [rfcomm]
[ 2917.866363] {IN-SOFTIRQ-W} state was registered at:
[ 2917.871276]   [<c008d33c>] mark_lock+0x298/0x630
[ 2917.875946]   [<c008ed3c>] __lock_acquire+0x5f4/0x175c
[ 2917.881134]   [<c008ff0c>] lock_acquire+0x68/0x7c
[ 2917.885864]   [<c036938c>] _spin_lock+0x48/0x58
[ 2917.890441]   [<bf025960>] l2cap_conn_start+0x80/0x388 [l2cap]
[ 2917.896362]   [<bf028f44>] l2cap_recv_frame+0x1c58/0x2fe0 [l2cap]
[ 2917.902526]   [<bf02a3cc>] l2cap_recv_acldata+0x100/0x350 [l2cap]
[ 2917.908691]   [<bf0035a8>] hci_rx_task+0x244/0x478 [bluetooth]
[ 2917.914642]   [<c006c4d0>] tasklet_action+0x78/0xd8
[ 2917.919555]   [<c006cc34>] __do_softirq+0xa8/0x154
[ 2917.924407]   [<c006cd40>] irq_exit+0x60/0xb4
[ 2917.928802]   [<c0030078>] asm_do_IRQ+0x78/0x90
[ 2917.933380]   [<c0030af0>] __irq_svc+0x50/0xbc
[ 2917.937866]   [<c0043c74>] omap3_enter_idle_bm+0x1d0/0x238
[ 2917.943389]   [<c029ee94>] cpuidle_idle_call+0xb4/0x114
[ 2917.948669]   [<c00320b0>] cpu_idle+0x58/0xac
[ 2917.953063]   [<c0360b18>] rest_init+0x70/0x84
[ 2917.957550]   [<c00089fc>] start_kernel+0x2b4/0x318
[ 2917.962493]   [<80008034>] 0x80008034
[ 2917.966186] irq event stamp: 312
[ 2917.969421] hardirqs last  enabled at (312): [<c03691ac>]
_spin_unlock_irqrestore+0x44/0x70
[ 2917.977844] hardirqs last disabled at (311): [<c036947c>]
_spin_lock_irqsave+0x24/0x68
[ 2917.985809] softirqs last  enabled at (261): [<c006ccc8>]
__do_softirq+0x13c/0x154
[ 2917.993438] softirqs last disabled at (244): [<c006cde8>]
do_softirq+0x54/0x78
[ 2918.000732]
[ 2918.000732] other info that might help us debug this:
[ 2918.007293] 2 locks held by krfcommd/1516:
[ 2918.011413]  #0:  (rfcomm_mutex){+.+.+.}, at: [<bf054df4>]
rfcomm_run+0x1f0/0xb00 [rfcomm]
[ 2918.019805]  #1:  (&d->lock){+.+...}, at: [<bf055220>]
rfcomm_run+0x61c/0xb00 [rfcomm]
[ 2918.027832]
[ 2918.027832] stack backtrace:
[ 2918.032226] Backtrace:
[ 2918.034729] [<c00348d0>] (dump_backtrace+0x0/0x110) from [<c036616c>]
(dump_stack+0x18/0x1c)
[ 2918.043212]  r7:dc8f6c00 r6:c0425252 r5:00000001 r4:00000001
[ 2918.048950] [<c0366154>] (dump_stack+0x0/0x1c) from [<c008d060>]
(print_usage_bug+0x178/0x1bc)
[ 2918.057617] [<c008cee8>] (print_usage_bug+0x0/0x1bc) from [<c008d408>]
(mark_lock+0x364/0x630)
[ 2918.066284] [<c008d0a4>] (mark_lock+0x0/0x630) from [<c008edcc>]
(__lock_acquire+0x684/0x175c)
[ 2918.074951] [<c008e748>] (__lock_acquire+0x0/0x175c) from [<c008ff0c>]
(lock_acquire+0x68/0x7c)
[ 2918.083709] [<c008fea4>] (lock_acquire+0x0/0x7c) from [<c036938c>]
(_spin_lock+0x48/0x58)
[ 2918.091949]  r7:dba9402c r6:dba5c3c0 r5:dba9402c r4:bf057b50
[ 2918.097717] [<c0369344>] (_spin_lock+0x0/0x58) from [<bf057b50>]
(rfcomm_sk_state_change+0x78/0x160 [rfcomm])
[ 2918.107696]  r5:dba94000 r4:00000000
[ 2918.111358] [<bf057ad8>] (rfcomm_sk_state_change+0x0/0x160 [rfcomm]) from
[<bf055238>] (rfcomm_run+0x634/0xb00 [rfcomm])
[ 2918.122283]  r7:dba5c450 r6:dba5d6c0 r5:dba5c3c0 r4:dba5c430
[ 2918.128051] [<bf054c04>] (rfcomm_run+0x0/0xb00 [rfcomm]) from [<c007cc10>]
(kthread+0x88/0x90)
[ 2918.136749] [<c007cb88>] (kthread+0x0/0x90) from [<c006a86c>]
(do_exit+0x0/0x678)
[ 2918.144256]  r7:00000000 r6:00000000 r5:00000000 r4:00000000

Thanks,
Yuri K.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-09-29  7:41 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-25  1:01 Pull resquest review to 2.6.36-rc5 Gustavo F. Padovan
2010-09-25  1:01 ` [PATCH 1/6] Bluetooth: Only enable for L2CAP FCS for ERTM or streaming Gustavo F. Padovan
2010-09-25  1:01   ` [PATCH 2/6] Bluetooth: fix MTU L2CAP configuration parameter Gustavo F. Padovan
2010-09-25  1:01     ` [PATCH 3/6] Bluetooth: Simplify L2CAP Streaming mode sending Gustavo F. Padovan
2010-09-25  1:01       ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Gustavo F. Padovan
2010-09-25  1:01         ` [PATCH 5/6] Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state" Gustavo F. Padovan
2010-09-25  1:01           ` [PATCH 6/6] Bluetooth: Fix deadlock in the ERTM logic Gustavo F. Padovan
2010-09-29  7:41         ` [PATCH 4/6] Bluetooth: Fix inconsistent lock state with RFCOMM Yuri Kululin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.