On Mon, 23 Apr 2007, Jiri Kosina wrote:
> > BUG: sleeping function called from invalid context at net/core/sock.c:1523
> > in_atomic():1, irqs_disabled():0
> > 1 lock held by khubd/180:
> > #0: (old_style_rw_init#2){-.-?}, at: [<f88c5816>]
> > hci_sock_dev_event+0x42/0xc5 [bluetooth]
[...]
> OK, this probably started happening since b40df5743. Before that commit,
> hci_sock_dev_event() used bh_lock_sock() to lock the corresponding
> struct sock. This was obviously buggy - not deadlock safe against
> l2cap_connect_cfm() from softirq context. This however introduced
> another problem - hci_sock_dev_event() is now obviously being triggered
> (for HCI_DEV_UNREG event, when suspending) in atomic context with
> preemption disabled. This is what lock_sock_nested() complains about, as
> it is allowed to sleep inside __lock_sock(), waiting for the lock owner.
I guess the patch below is a proper fix. Marcel, does this look okay to
you?
From: Jiri Kosina <[EMAIL PROTECTED]>
Bluetooth: postpone hci_dev unregistration
Commit b40df57 substituted bh_lock_sock() in hci_sock_dev_event() for
lock_sock() when unregistering HCI device, in order to prevent deadlock
against locking in l2cap_connect_cfm() from softirq context.
This however introduces another problem - hci_sock_dev_event() for
HCI_DEV_UNREG can also be triggered in atomic context, in which calling
lock_sock() is not safe as it could sleep.
This patch moves the detaching of sockets from hci_device into workqueue,
so that lock_sock() can be used safely. This requires movement of
deallocation of hci_dev - deallocating device just after
hci_unregister_dev() would be too soon, as it could happen before the
workqueue has been run.
Signed-off-by: Jiri Kosina <[EMAIL PROTECTED]>
---
drivers/bluetooth/bfusb.c | 2 -
drivers/bluetooth/bluecard_cs.c | 2 -
drivers/bluetooth/bpa10x.c | 2 -
drivers/bluetooth/bt3c_cs.c | 2 -
drivers/bluetooth/btuart_cs.c | 2 -
drivers/bluetooth/dtl1_cs.c | 2 -
drivers/bluetooth/hci_ldisc.c | 1 -
drivers/bluetooth/hci_usb.c | 2 -
drivers/bluetooth/hci_vhci.c | 2 -
include/net/bluetooth/hci_core.h | 3 ++
net/bluetooth/hci_core.c | 9 +++++++
net/bluetooth/hci_sock.c | 44 ++++++++++++++++++++-----------------
12 files changed, 36 insertions(+), 37 deletions(-)
diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index 4c766f3..db6809e 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -762,8 +762,6 @@ static void bfusb_disconnect(struct usb_
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
-
- hci_free_dev(hdev);
}
static struct usb_driver bfusb_driver = {
diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
index acfb6a4..1184113 100644
--- a/drivers/bluetooth/bluecard_cs.c
+++ b/drivers/bluetooth/bluecard_cs.c
@@ -851,8 +851,6 @@ static int bluecard_close(bluecard_info_
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
- hci_free_dev(hdev);
-
return 0;
}
diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index 9fca651..7dfaa95 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -613,8 +613,6 @@ static void bpa10x_disconnect(struct usb
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
-
- hci_free_dev(hdev);
}
static struct usb_driver bpa10x_driver = {
diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
index 18b0f39..6ab7b56 100644
--- a/drivers/bluetooth/bt3c_cs.c
+++ b/drivers/bluetooth/bt3c_cs.c
@@ -640,8 +640,6 @@ static int bt3c_close(bt3c_info_t *info)
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
- hci_free_dev(hdev);
-
return 0;
}
diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
index c1bce75..93ca675 100644
--- a/drivers/bluetooth/btuart_cs.c
+++ b/drivers/bluetooth/btuart_cs.c
@@ -570,8 +570,6 @@ static int btuart_close(btuart_info_t *i
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
- hci_free_dev(hdev);
-
return 0;
}
diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
index 459aa97..4fc7c02 100644
--- a/drivers/bluetooth/dtl1_cs.c
+++ b/drivers/bluetooth/dtl1_cs.c
@@ -552,8 +552,6 @@ static int dtl1_close(dtl1_info_t *info)
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
- hci_free_dev(hdev);
-
return 0;
}
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 0f4203b..4c4d555 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -312,7 +312,6 @@ static void hci_uart_tty_close(struct tt
if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) {
hu->proto->close(hu);
hci_unregister_dev(hdev);
- hci_free_dev(hdev);
}
}
}
diff --git a/drivers/bluetooth/hci_usb.c b/drivers/bluetooth/hci_usb.c
index 406af57..e9e0183 100644
--- a/drivers/bluetooth/hci_usb.c
+++ b/drivers/bluetooth/hci_usb.c
@@ -1069,8 +1069,6 @@ static void hci_usb_disconnect(struct us
if (hci_unregister_dev(hdev) < 0)
BT_ERR("Can't unregister HCI device %s", hdev->name);
-
- hci_free_dev(hdev);
}
static int hci_usb_suspend(struct usb_interface *intf, pm_message_t message)
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index b71a5cc..e5a3a8c 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -308,8 +308,6 @@ static int vhci_release(struct inode *in
BT_ERR("Can't unregister HCI device %s", hdev->name);
}
- hci_free_dev(hdev);
-
file->private_data = NULL;
return 0;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index c0fc396..a0a0a15 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -132,6 +132,8 @@ struct hci_dev {
struct module *owner;
+ struct work_struct hci_dev_unreg_work;
+
int (*open)(struct hci_dev *hdev);
int (*close)(struct hci_dev *hdev);
int (*flush)(struct hci_dev *hdev);
@@ -622,6 +624,7 @@ void hci_si_event(struct hci_dev *hdev,
/* ----- HCI Sockets ----- */
void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
+void hci_sock_detach(struct hci_dev *hdev);
/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4917919..54a50ee 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -795,6 +795,14 @@ int hci_get_dev_info(void __user *arg)
return err;
}
+void hci_dev_unreg(struct work_struct *work)
+{
+ struct hci_dev *hdev =
+ container_of(work, struct hci_dev, hci_dev_unreg_work);
+ hci_sock_detach(hdev);
+ hci_free_dev(hdev);
+}
+
/* ---- Interface to HCI drivers ---- */
/* Alloc HCI device */
@@ -807,6 +815,7 @@ struct hci_dev *hci_alloc_dev(void)
return NULL;
skb_queue_head_init(&hdev->driver_init);
+ INIT_WORK(&hdev->hci_dev_unreg_work, hci_dev_unreg);
return hdev;
}
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 71f5cfb..fb59408 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -141,6 +141,28 @@ void hci_send_to_sock(struct hci_dev *hd
read_unlock(&hci_sk_list.lock);
}
+void hci_sock_detach(struct hci_dev *hdev)
+{
+ struct sock *sk;
+ struct hlist_node *node;
+
+ /* Detach sockets from device */
+ read_lock(&hci_sk_list.lock);
+ sk_for_each(sk, node, &hci_sk_list.head) {
+ lock_sock(sk);
+ if (hci_pi(sk)->hdev == hdev) {
+ hci_pi(sk)->hdev = NULL;
+ sk->sk_err = EPIPE;
+ sk->sk_state = BT_OPEN;
+ sk->sk_state_change(sk);
+
+ hci_dev_put(hdev);
+ }
+ release_sock(sk);
+ }
+ read_unlock(&hci_sk_list.lock);
+}
+
static int hci_sock_release(struct socket *sock)
{
struct sock *sk = sock->sk;
@@ -649,26 +671,8 @@ static int hci_sock_dev_event(struct not
ev.dev_id = hdev->id;
hci_si_event(NULL, HCI_EV_SI_DEVICE, sizeof(ev), &ev);
- if (event == HCI_DEV_UNREG) {
- struct sock *sk;
- struct hlist_node *node;
-
- /* Detach sockets from device */
- read_lock(&hci_sk_list.lock);
- sk_for_each(sk, node, &hci_sk_list.head) {
- lock_sock(sk);
- if (hci_pi(sk)->hdev == hdev) {
- hci_pi(sk)->hdev = NULL;
- sk->sk_err = EPIPE;
- sk->sk_state = BT_OPEN;
- sk->sk_state_change(sk);
-
- hci_dev_put(hdev);
- }
- release_sock(sk);
- }
- read_unlock(&hci_sk_list.lock);
- }
+ if (event == HCI_DEV_UNREG)
+ schedule_work(&hdev->hci_dev_unreg_work);
return NOTIFY_DONE;
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
