Skip to contentSkip to conversation

Bluetooth RFComm: using it locks up the machine

Regression In review Mark Lord 4 participants 7 messages Mar 4, 2007 – Mar 5, 2007

Subsystems (3)
Bluetooth drivers Driver core Bluetooth
lore
Share & tools
lore

Mark Lord

author
#0 lore raw

Any attempt to open/use a bluetooth rfcomm device locks up scheduling completely on my machine.

Interrupts (ping, alt-sysrq) seem to be alive, but nothing else.

This was working fine in 2.6.20, broken now in 2.6.21-rc2-git*

Mar  4 09:43:52 silvy kernel: Bluetooth: L2CAP ver 2.8
Mar  4 09:43:52 silvy kernel: Bluetooth: L2CAP socket layer initialized
Mar  4 09:43:52 silvy kernel: Bluetooth: RFCOMM socket layer initialized
Mar  4 09:43:52 silvy kernel: Bluetooth: RFCOMM TTY layer initialized
Mar  4 09:43:52 silvy kernel: Bluetooth: RFCOMM ver 1.8

More info attached.

Cheers

Bus 002 Device 002: ID 413c:8103 Dell Computer Corp. Wireless 350 Bluetooth Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 224 Wireless
bDeviceSubClass 1 Radio Frequency
bDeviceProtocol 1 Bluetooth
bMaxPacketSize0 64
idVendor 0x413c Dell Computer Corp.
idProduct 0x8103 Wireless 350 Bluetooth
bcdDevice 16.57

  iManufacturer           0 
  iProduct                0 
  iSerial                 0 
  bNumConfigurations      1

Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 193
bNumInterfaces 3
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xe0
Self Powered
Remote Wakeup
MaxPower 0mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0000 1x 0 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0000 1x 0 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 1
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0009 1x 9 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0009 1x 9 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 2
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0011 1x 17 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0011 1x 17 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 3
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0019 1x 25 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0019 1x 25 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 4
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0021 1x 33 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0021 1x 33 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 5
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0031 1x 49 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0031 1x 49 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 2
bAlternateSetting 0
bNumEndpoints 0
bInterfaceClass 254 Application Specific Interface
bInterfaceSubClass 1 Device Firmware Update
bInterfaceProtocol 0
iInterface 0
Device Status: 0x0001
Self Powered

author
#1 lore raw
Quoted 6 lines · Mark Lord

Any attempt to open/use a bluetooth rfcomm device locks up scheduling completely on my machine.

Interrupts (ping, alt-sysrq) seem to be alive, but nothing else.

This was working fine in 2.6.20, broken now in 2.6.21-rc2-git*

Further info: Reverting this change (below) fixes it:

| author	Marcel Holtmann <[email protected]>
| 	 Sat, 17 Feb 2007 22:58:57 +0000 (23:58 +0100)
| committer	David S. Miller <[email protected]>
| 	 Mon, 26 Feb 2007 19:42:41 +0000 (11:42 -0800)
| commit	c1a3313698895d8ad4760f98642007bf236af2e8
| tree	337a876f727061362b6a169f8759849c105b8f7a	tree | snapshot
| parent	f5ffd4620aba9e55656483ae1ef5c79ba81f5403	commit | diff
| | [Bluetooth] Make use of device_move() for RFCOMM TTY devices
| | In the case of bound RFCOMM TTY devices the parent is not available
| before its usage. So when opening a RFCOMM TTY device, move it to
| the corresponding ACL device as a child. When closing the device,
| move it back to the virtual device tree.
| Signed-off-by: Marcel Holtmann <[email protected]>

Specifically, I reverted these changes, below, to fix it:

net/bluetooth/rfcomm/tty.c +0 -0
@@ -74,6 +74,8 @@

	wait_queue_head_t       wait;
	struct tasklet_struct   wakeup_task;

+ struct device *tty_dev;
+

	atomic_t 		wmem_alloc;
};

@@ -261,7 +263,7 @@

		return err;
	}

- tty_register_device(rfcomm_tty_driver, dev->id, rfcomm_get_device(dev));
+ dev->tty_dev = tty_register_device(rfcomm_tty_driver, dev->id, NULL);

	return dev->id;
}
@@ -630,6 +632,9 @@
	set_current_state(TASK_RUNNING);
	remove_wait_queue(&dev->wait, &wait);

+ if (err == 0)
+ device_move(dev->tty_dev, rfcomm_get_device(dev));
+

	return err;
}

@@ -642,6 +647,8 @@

	BT_DBG("tty %p dev %p dlc %p opened %d", tty, dev, dev->dlc, dev->opened);

	if (--dev->opened == 0) {
+		device_move(dev->tty_dev, NULL);
+
		/* Close DLC and dettach TTY */
		rfcomm_dlc_close(dev->dlc, 0);
maintainer
#2 lore raw

Hi Mark,

Quoted 25 lines
Quoted 6 lines

Any attempt to open/use a bluetooth rfcomm device locks up scheduling completely on my machine.

Interrupts (ping, alt-sysrq) seem to be alive, but nothing else.

This was working fine in 2.6.20, broken now in 2.6.21-rc2-git*

Further info: Reverting this change (below) fixes it:

| author	Marcel Holtmann <[email protected]>
| 	 Sat, 17 Feb 2007 22:58:57 +0000 (23:58 +0100)
| committer	David S. Miller <[email protected]>
| 	 Mon, 26 Feb 2007 19:42:41 +0000 (11:42 -0800)
| commit	c1a3313698895d8ad4760f98642007bf236af2e8
| tree	337a876f727061362b6a169f8759849c105b8f7a	tree | snapshot
| parent	f5ffd4620aba9e55656483ae1ef5c79ba81f5403	commit | diff
| 
| [Bluetooth] Make use of device_move() for RFCOMM TTY devices
| 
| In the case of bound RFCOMM TTY devices the parent is not available
| before its usage. So when opening a RFCOMM TTY device, move it to
| the corresponding ACL device as a child. When closing the device,
| move it back to the virtual device tree.
| Signed-off-by: Marcel Holtmann <[email protected]>
Quoted 1 line

Specifically, I reverted these changes, below, to fix it:

please post these information to the Linux kernel mailing list, because I think that must be an issue with the device_move() API. I tested this successfully with a Quad G5 and it was working as expected.

Regards

Marcel

author
#3 lore raw
In reply to Mark Lord
Quoted 10 lines · Mark Lord
Quoted 6 lines · Mark Lord

Any attempt to open/use a bluetooth rfcomm device locks up scheduling completely on my machine.

Interrupts (ping, alt-sysrq) seem to be alive, but nothing else.

This was working fine in 2.6.20, broken now in 2.6.21-rc2-git*

Further info: Reverting this change (below) fixes it:

Quoted 13 lines
| author    Marcel Holtmann <[email protected]>
|      Sat, 17 Feb 2007 22:58:57 +0000 (23:58 +0100)
| committer    David S. Miller <[email protected]>
|      Mon, 26 Feb 2007 19:42:41 +0000 (11:42 -0800)
| commit    c1a3313698895d8ad4760f98642007bf236af2e8
| tree    337a876f727061362b6a169f8759849c105b8f7a    tree | snapshot
| parent    f5ffd4620aba9e55656483ae1ef5c79ba81f5403    commit | diff
| | [Bluetooth] Make use of device_move() for RFCOMM TTY devices
| | In the case of bound RFCOMM TTY devices the parent is not available
| before its usage. So when opening a RFCOMM TTY device, move it to
| the corresponding ACL device as a child. When closing the device,
| move it back to the virtual device tree.
| Signed-off-by: Marcel Holtmann <[email protected]>

..

The lockup is happening inside the call to device_move(), specifically in sysfs_move_dir() it loops here forever:

...
again:

       mutex_lock(&old_parent_dentry->d_inode->i_mutex);
       if (!mutex_trylock(&new_parent_dentry->d_inode->i_mutex)) {
               mutex_unlock(&old_parent_dentry->d_inode->i_mutex);
               goto again;
       }

...

Screen-shot (photograph) from alt-sysrq-P is here:
http://rtr.ca/recent/rfcomm_pc.jpg

Cheers

author
#4 lore raw

PATCH 1/1 Fix 2.6.21 rfcomm lockups (2.6.21 regression)

Quoted 6 lines · Mark Lord

Any attempt to open/use a bluetooth rfcomm device locks up scheduling completely on my machine.

Interrupts (ping, alt-sysrq) seem to be alive, but nothing else.

This was working fine in 2.6.20, broken now in 2.6.21-rc2-git*

Further info: Reverting this change (below) fixes it:

| author    Marcel Holtmann <[email protected]>
|      Sat, 17 Feb 2007 22:58:57 +0000 (23:58 +0100)
| committer    David S. Miller <[email protected]>
|      Mon, 26 Feb 2007 19:42:41 +0000 (11:42 -0800)
| commit    c1a3313698895d8ad4760f98642007bf236af2e8
| tree    337a876f727061362b6a169f8759849c105b8f7a    tree | snapshot
| parent    f5ffd4620aba9e55656483ae1ef5c79ba81f5403    commit | diff
| | [Bluetooth] Make use of device_move() for RFCOMM TTY devices
| | In the case of bound RFCOMM TTY devices the parent is not available
| before its usage. So when opening a RFCOMM TTY device, move it to
| the corresponding ACL device as a child. When closing the device,
| move it back to the virtual device tree.
| Signed-off-by: Marcel Holtmann <[email protected]>

The simplest fix for this bug is to prevent sysfs_move_dir() from self-deadlocking when (old_parent == new_parent).

This patch prevents total system lockup when using rfcomm devices.

Signed-off-by Mark Lord <[email protected]>

---

fs/sysfs/dir.c +0 -0
@@ -431,6 +431,8 @@

	new_parent_dentry = new_parent ?
		new_parent->dentry : sysfs_mount->mnt_sb->s_root;

+ if (old_parent_dentry->d_inode == new_parent_dentry->d_inode)
+ return 0; /* nothing to move */
again:

	mutex_lock(&old_parent_dentry->d_inode->i_mutex);
	if (!mutex_trylock(&new_parent_dentry->d_inode->i_mutex)) {
Acked-by on v1 1/1
#5 lore raw

Re: [PATCH] Fix 2.6.21 rfcomm lockups (2.6.21 regression)

Quoted 10 lines · Mark Lord

The simplest fix for this bug is to prevent sysfs_move_dir() from self-deadlocking when (old_parent == new_parent).

This patch prevents total system lockup when using rfcomm devices.

Signed-off-by Mark Lord <[email protected]>

---

fs/sysfs/dir.c line 438

Quoted 8 lines
fs/sysfs/dir.c +2 -0
@@ -431,6 +431,8 @@
 	new_parent_dentry = new_parent ?
 		new_parent->dentry : sysfs_mount->mnt_sb->s_root;
 
+	if (old_parent_dentry->d_inode == new_parent_dentry->d_inode)
+		return 0;	/* nothing to move */
 again:
 	mutex_lock(&old_parent_dentry->d_inode->i_mutex);
 	if (!mutex_trylock(&new_parent_dentry->d_inode->i_mutex)) {

Hm, never thought that someone might call the moving functions with old_parent == new_parent, sorry about that. It should be safe to return success here, we add only a bit of overhead with some driver model juggling and emitting a not needed uevent.

Acked-by Cornelia Huck <[email protected]>
#6 lore raw

Re: [PATCH] Fix 2.6.21 rfcomm lockups (2.6.21 regression)

In reply to Mark Lord
Quoted 1 line · Mark Lord

This patch prevents total system lockup when using rfcomm devices.

I acknowledge that this patch fixes lockup for me too.

When debugging this, I also came across a different bug (spotted by lockdep). Is the patch below applicable?

From: Jiri Kosina <[email protected]>

[Bluetooth] Fix socket locking in hci_sock_dev_event()

hci_sock_dev_event() uses bh_lock_sock() to lock the socket lock. This is not deadlock-safe against locking of the same socket lock in l2cap_connect_cfm() from softirq context. In addition to that, hci_sock_dev_event() doesn't seem to be called from softirq context, so it is safe to use lock_sock()/release_sock() instead.

The lockdep warning can be triggered on my T42p simply by switching the Bluetooth off by the keyboard button.

=================================
[ INFO: inconsistent lock state ]
2.6.21-rc2 #4
---------------------------------
inconsistent {in-softirq-W} -> {softirq-on-W} usage.
khubd/156 [HC0[0]:SC0[0]:HE1:SE1] takes:
(slock-AF_BLUETOOTH){-+..}, at: [<e0ca5520>] hci_sock_dev_event+0xa8/0xc5 [bluetooth]
{in-softirq-W} state was registered at:

  [<c012d1db>] mark_lock+0x59/0x414
  [<e0cef688>] l2cap_connect_cfm+0x4e/0x11f [l2cap]
  [<c012dfd7>] __lock_acquire+0x3e5/0xb99
  [<e0cef688>] l2cap_connect_cfm+0x4e/0x11f [l2cap]
  [<c012e7f2>] lock_acquire+0x67/0x81
  [<e0cef688>] l2cap_connect_cfm+0x4e/0x11f [l2cap]
  [<c036ee72>] _spin_lock+0x29/0x34
  [<e0cef688>] l2cap_connect_cfm+0x4e/0x11f [l2cap]
  [<e0cef688>] l2cap_connect_cfm+0x4e/0x11f [l2cap]
  [<e0ca17c3>] hci_send_cmd+0x126/0x14f [bluetooth]
  [<e0ca4ce4>] hci_event_packet+0x729/0xebd [bluetooth]
  [<e0ca205b>] hci_rx_task+0x2a/0x20f [bluetooth]
  [<e0ca209d>] hci_rx_task+0x6c/0x20f [bluetooth]
  [<c012d7be>] trace_hardirqs_on+0x10d/0x14e
  [<c011ac85>] tasklet_action+0x3d/0x68
  [<c011abba>] __do_softirq+0x41/0x92
  [<c011ac32>] do_softirq+0x27/0x3d
  [<c0105134>] do_IRQ+0x7b/0x8f
  [<c0103dec>] common_interrupt+0x24/0x34
  [<c0103df6>] common_interrupt+0x2e/0x34
  [<c0248e65>] acpi_processor_idle+0x1b3/0x34a
  [<c0248e68>] acpi_processor_idle+0x1b6/0x34a
  [<c010232b>] cpu_idle+0x39/0x4e
  [<c04bab0c>] start_kernel+0x372/0x37a
  [<c04ba42b>] unknown_bootoption+0x0/0x202
  [<ffffffff>] 0xffffffff
Signed-off-by Jiri Kosina <[email protected]>
---
 net/bluetooth/hci_sock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
net/bluetooth/hci_sock.c +2 -2
index f928d2b..71f5cfb 100644
@@ -656,7 +656,7 @@ static int hci_sock_dev_event(struct not
 		/* Detach sockets from device */
 		read_lock(&hci_sk_list.lock);
 		sk_for_each(sk, node, &hci_sk_list.head) {
-			bh_lock_sock(sk);
+			lock_sock(sk);
 			if (hci_pi(sk)->hdev == hdev) {
 				hci_pi(sk)->hdev = NULL;
 				sk->sk_err = EPIPE;

@@ -665,7 +665,7 @@ static int hci_sock_dev_event(struct not
 
 				hci_dev_put(hdev);
 			}
-			bh_unlock_sock(sk);
+			release_sock(sk);
 		}
 		read_unlock(&hci_sk_list.lock);
 	}