• David Herrmann's avatar
    Bluetooth: remove unneeded hci_conn_hold/put_device() · fc225c3f
    David Herrmann authored
    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: default avatarDavid Herrmann <dh.herrmann@gmail.com>
    Acked-by: default avatarMarcel Holtmann <marcel@holtmann.org>
    Signed-off-by: default avatarGustavo Padovan <gustavo.padovan@collabora.co.uk>
    fc225c3f
core.c 29.1 KB