summaryrefslogtreecommitdiffstats
path: root/net/bluetooth/hidp
diff options
context:
space:
mode:
authorDavid Herrmann <dh.herrmann@gmail.com>2013-04-06 20:28:38 +0200
committerGustavo Padovan <gustavo.padovan@collabora.co.uk>2013-04-17 02:38:36 -0300
commitfc225c3f5d1b6aa6f99c5c300af4605e4923ce79 (patch)
tree15abcbd5495d423c3634af6b518e1f5ff2c22f84 /net/bluetooth/hidp
parent93796fa6f21411dab2ce7ba4fd7fd4d4ed4aca2e (diff)
downloadlinux-fc225c3f5d1b6aa6f99c5c300af4605e4923ce79.tar.bz2
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 <dh.herrmann@gmail.com> Acked-by: Marcel Holtmann <marcel@holtmann.org> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
Diffstat (limited to 'net/bluetooth/hidp')
-rw-r--r--net/bluetooth/hidp/core.c20
1 files changed, 3 insertions, 17 deletions
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 4ab82cb3eac3..9734136d6431 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -73,18 +73,6 @@ static struct hidp_session *__hidp_get_session(bdaddr_t *bdaddr)
return NULL;
}
-static void __hidp_link_session(struct hidp_session *session)
-{
- list_add(&session->list, &hidp_session_list);
-}
-
-static void __hidp_unlink_session(struct hidp_session *session)
-{
- hci_conn_put_device(session->conn);
-
- list_del(&session->list);
-}
-
static void __hidp_copy_session(struct hidp_session *session, struct hidp_conninfo *ci)
{
memset(ci, 0, sizeof(*ci));
@@ -760,7 +748,7 @@ static int hidp_session(void *arg)
fput(session->ctrl_sock->file);
- __hidp_unlink_session(session);
+ list_del(&session->list);
up_write(&hidp_session_sem);
@@ -783,8 +771,6 @@ static struct hci_conn *hidp_get_connection(struct hidp_session *session)
hci_dev_lock(hdev);
conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst);
- if (conn)
- hci_conn_hold_device(conn);
hci_dev_unlock(hdev);
hci_dev_put(hdev);
@@ -1026,7 +1012,7 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
session->flags = req->flags & (1 << HIDP_BLUETOOTH_VENDOR_ID);
session->idle_to = req->idle_to;
- __hidp_link_session(session);
+ list_add(&session->list, &hidp_session_list);
if (req->rd_size > 0) {
err = hidp_setup_hid(session, req);
@@ -1106,7 +1092,7 @@ unlink:
session->rd_data = NULL;
purge:
- __hidp_unlink_session(session);
+ list_del(&session->list);
skb_queue_purge(&session->ctrl_transmit);
skb_queue_purge(&session->intr_transmit);