From fc225c3f5d1b6aa6f99c5c300af4605e4923ce79 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sat, 6 Apr 2013 20:28:38 +0200 Subject: Bluetooth: remove unneeded hci_conn_hold/put_device() hci_conn_hold/put_device() is used to control when hci_conn->dev is no longer needed and can be deleted from the system. Lets first look how they are currently used throughout the code (excluding HIDP!). All code that uses hci_conn_hold_device() looks like this: ... hci_conn_hold_device(); hci_conn_add_sysfs(); ... On the other side, hci_conn_put_device() is exclusively used in hci_conn_del(). So, considering that hci_conn_del() must not be called twice (which would fail horribly), we know that hci_conn_put_device() is only called _once_ (which is in hci_conn_del()). On the other hand, hci_conn_add_sysfs() must not be called twice, either (it would call device_add twice, which breaks the device, see drivers/base/core.c). So we know that hci_conn_hold_device() is also called only once (it's only called directly before hci_conn_add_sysfs()). So hold and put are known to be called only once. That means we can safely remove them and directly call hci_conn_del_sysfs() in hci_conn_del(). But there is one issue left: HIDP also uses hci_conn_hold/put_device(). However, this case can be ignored and simply removed as it is totally broken. The issue is, the only thing HIDP delays with hci_conn_hold_device() is the removal of the hci_conn->dev from sysfs. But, the hci_conn device has no mechanism to get notified when its own parent (hci_dev) gets removed from sysfs. hci_dev_hold/put() does _not_ control when it is removed but only when the device object is created and destroyed. And hci_dev calls hci_conn_flush_*() when it removes itself from sysfs, which itself causes hci_conn_del() to be called, but it does _not_ cause hci_conn_del_sysfs() to be called, which is wrong. Hence, we fix it to call hci_conn_del_sysfs() in hci_conn_del(). This guarantees that a hci_conn object is removed from sysfs _before_ its parent hci_dev is removed. The changes to HIDP look scary, wrong and broken. However, if you look at the HIDP session management, you will notice they're already broken in the exact _same_ way (ever tried "unplugging" HIDP devices? Breaks _all_ the time). So this patch only makes HIDP look _scary_ and _obviously broken_. It does not break HIDP itself, it already is! See later patches in this series which fix HIDP to use proper session-management. Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo Padovan --- net/bluetooth/hci_conn.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) (limited to 'net/bluetooth/hci_conn.c') diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index b1a02ce39a20..6b5b8e77cf0b 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -410,8 +410,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst) if (hdev->notify) hdev->notify(hdev, HCI_NOTIFY_CONN_ADD); - atomic_set(&conn->devref, 0); - hci_conn_init_sysfs(conn); return conn; @@ -460,7 +458,7 @@ int hci_conn_del(struct hci_conn *conn) skb_queue_purge(&conn->data_q); - hci_conn_put_device(conn); + hci_conn_del_sysfs(conn); hci_dev_put(hdev); @@ -847,19 +845,6 @@ void hci_conn_check_pending(struct hci_dev *hdev) hci_dev_unlock(hdev); } -void hci_conn_hold_device(struct hci_conn *conn) -{ - atomic_inc(&conn->devref); -} -EXPORT_SYMBOL(hci_conn_hold_device); - -void hci_conn_put_device(struct hci_conn *conn) -{ - if (atomic_dec_and_test(&conn->devref)) - hci_conn_del_sysfs(conn); -} -EXPORT_SYMBOL(hci_conn_put_device); - int hci_get_conn_list(void __user *arg) { struct hci_conn *c; -- cgit v1.2.3