* [PATCH 00/18] Cleanup HCI reference counts
@ 2012-01-07 14:47 David Herrmann
2012-01-07 14:47 ` [PATCH 01/18] Bluetooth: Make hci-destruct callback optional David Herrmann
` (19 more replies)
0 siblings, 20 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
Hi
This is the same as my previous patchset but split into sequential patches. The
first patch makes the destruct-cb optional so the patches 2-7 can remove the
empty callbacks.
Patches 8-14 make the remaining drivers drop the destruct-callback and directly
free the driver data.
Patches 15-18 then remove the (now unused!) destruct callback and then properly
fixes the reference counts (same patches as already posted).
I hope the first 14 patches make clear that half of our drivers already work
without the destruct callback and the other half can be safely converted into
not using it. In fact, the drivers that use it introduce some memleaks which are
fixed by simply dropping the callback and freeing everything right away.
I've added detailed commit messages to each driver. If a single step is unclear,
please point me to it.
By the way, this fixes several bugs we currently have, including some mem-leaks
as described in the commits and the fact that hci_free_dev() currently
*directly* destroys the hci object regardless of any previous calls to
hci_hold_dev/hci_put_dev(). This is because the hci->refcnt counter is in no way
linked to the lifetime of the hci object. See hci_sysfs the release_host
callback which actually frees the hci object. It is in no way triggered by
hci->refcnt.
This patchset fixes all this and introduces proper reference counting.
Cheers
David
David Herrmann (18):
Bluetooth: Make hci-destruct callback optional
Bluetooth: bluecard-cs: Remove empty destruct cb
Bluetooth: bt3c-cs: Remove empty destruct cb
Bluetooth: btmrvl: Remove empty destruct cb
Bluetooth: btuart-cs: Remove empty destruct cb
Bluetooth: btwilink: Remove empty destruct cb
Bluetooth: dtl1-cs: Remove empty destruct cb
Bluetooth: vhci: Free driver_data on file release
Bluetooth: bfusb: Free driver_data on USB shutdown
Bluetooth: btusb: Free driver data on USB shutdown
Bluetooth: bpa10x: Free private driver data on usb shutdown
Bluetooth: btsdio: Free driver data on SDIO shutdown
Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb
Bluetooth: Remove unused hci-destruct cb
Bluetooth: Correctly acquire module ref
Bluetooth: Remove HCI-owner field
Bluetooth: Correctly take hci_dev->dev refcount
Bluetooth: Remove __hci_dev_put/hold
drivers/bluetooth/bfusb.c | 13 +------------
drivers/bluetooth/bluecard_cs.c | 8 --------
drivers/bluetooth/bpa10x.c | 17 +++--------------
drivers/bluetooth/bt3c_cs.c | 8 --------
drivers/bluetooth/btmrvl_main.c | 6 ------
drivers/bluetooth/btsdio.c | 13 +------------
drivers/bluetooth/btuart_cs.c | 8 --------
drivers/bluetooth/btusb.c | 17 +++--------------
drivers/bluetooth/btwilink.c | 10 ----------
drivers/bluetooth/dtl1_cs.c | 8 --------
drivers/bluetooth/hci_ldisc.c | 14 ++------------
drivers/bluetooth/hci_vhci.c | 9 +--------
include/net/bluetooth/hci_core.h | 28 ++++------------------------
net/bluetooth/hci_core.c | 9 ++++-----
net/bluetooth/hci_sysfs.c | 2 ++
15 files changed, 21 insertions(+), 149 deletions(-)
--
1.7.8.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 01/18] Bluetooth: Make hci-destruct callback optional
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 02/18] Bluetooth: bluecard-cs: Remove empty destruct cb David Herrmann
` (18 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
Several drivers already provide an empty callback so we can actually make this
optional and then remove all those empty callbacks in the drivers.
This callback isn't needed at all by most drivers as they can remove their
allocated structures on device disconnect and not on hci destruction.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
include/net/bluetooth/hci_core.h | 6 ++++--
net/bluetooth/hci_core.c | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 5e2e984..b2c23e6 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -568,8 +568,10 @@ static inline void hci_conn_put(struct hci_conn *conn)
/* ----- HCI Devices ----- */
static inline void __hci_dev_put(struct hci_dev *d)
{
- if (atomic_dec_and_test(&d->refcnt))
- d->destruct(d);
+ if (atomic_dec_and_test(&d->refcnt)) {
+ if (d->destruct)
+ d->destruct(d);
+ }
}
/*
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6d38d80..6fdd6e5 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1450,7 +1450,7 @@ int hci_register_dev(struct hci_dev *hdev)
BT_DBG("%p name %s bus %d owner %p", hdev, hdev->name,
hdev->bus, hdev->owner);
- if (!hdev->open || !hdev->close || !hdev->destruct)
+ if (!hdev->open || !hdev->close)
return -EINVAL;
/* Do not allow HCI_AMP devices to register at index 0,
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 02/18] Bluetooth: bluecard-cs: Remove empty destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
2012-01-07 14:47 ` [PATCH 01/18] Bluetooth: Make hci-destruct callback optional David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 03/18] Bluetooth: bt3c-cs: " David Herrmann
` (17 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The destruct callback is optional and we provide an empty callback so remove it
entirely to avoid unnecessary code.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/bluecard_cs.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
index c6a0c61..5cb325a 100644
--- a/drivers/bluetooth/bluecard_cs.c
+++ b/drivers/bluetooth/bluecard_cs.c
@@ -691,11 +691,6 @@ static int bluecard_hci_send_frame(struct sk_buff *skb)
}
-static void bluecard_hci_destruct(struct hci_dev *hdev)
-{
-}
-
-
static int bluecard_hci_ioctl(struct hci_dev *hdev, unsigned int cmd, unsigned long arg)
{
return -ENOIOCTLCMD;
@@ -741,7 +736,6 @@ static int bluecard_open(bluecard_info_t *info)
hdev->close = bluecard_hci_close;
hdev->flush = bluecard_hci_flush;
hdev->send = bluecard_hci_send_frame;
- hdev->destruct = bluecard_hci_destruct;
hdev->ioctl = bluecard_hci_ioctl;
hdev->owner = THIS_MODULE;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 03/18] Bluetooth: bt3c-cs: Remove empty destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
2012-01-07 14:47 ` [PATCH 01/18] Bluetooth: Make hci-destruct callback optional David Herrmann
2012-01-07 14:47 ` [PATCH 02/18] Bluetooth: bluecard-cs: Remove empty destruct cb David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 04/18] Bluetooth: btmrvl: " David Herrmann
` (16 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The callback is optional and we provide an empty callback so remove it.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/bt3c_cs.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
index 0c97e5d..e74334d 100644
--- a/drivers/bluetooth/bt3c_cs.c
+++ b/drivers/bluetooth/bt3c_cs.c
@@ -456,11 +456,6 @@ static int bt3c_hci_send_frame(struct sk_buff *skb)
}
-static void bt3c_hci_destruct(struct hci_dev *hdev)
-{
-}
-
-
static int bt3c_hci_ioctl(struct hci_dev *hdev, unsigned int cmd, unsigned long arg)
{
return -ENOIOCTLCMD;
@@ -587,7 +582,6 @@ static int bt3c_open(bt3c_info_t *info)
hdev->close = bt3c_hci_close;
hdev->flush = bt3c_hci_flush;
hdev->send = bt3c_hci_send_frame;
- hdev->destruct = bt3c_hci_destruct;
hdev->ioctl = bt3c_hci_ioctl;
hdev->owner = THIS_MODULE;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 04/18] Bluetooth: btmrvl: Remove empty destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (2 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 03/18] Bluetooth: bt3c-cs: " David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 05/18] Bluetooth: btuart-cs: " David Herrmann
` (15 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The callback is optional and we provide an empty callback so remove it entirely.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/btmrvl_main.c | 5 -----
1 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index a88a78c..d62fc0d 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -387,10 +387,6 @@ static int btmrvl_ioctl(struct hci_dev *hdev,
return -ENOIOCTLCMD;
}
-static void btmrvl_destruct(struct hci_dev *hdev)
-{
-}
-
static int btmrvl_send_frame(struct sk_buff *skb)
{
struct hci_dev *hdev = (struct hci_dev *) skb->dev;
@@ -555,7 +551,6 @@ int btmrvl_register_hdev(struct btmrvl_private *priv)
hdev->close = btmrvl_close;
hdev->flush = btmrvl_flush;
hdev->send = btmrvl_send_frame;
- hdev->destruct = btmrvl_destruct;
hdev->ioctl = btmrvl_ioctl;
hdev->owner = THIS_MODULE;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 05/18] Bluetooth: btuart-cs: Remove empty destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (3 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 04/18] Bluetooth: btmrvl: " David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 06/18] Bluetooth: btwilink: " David Herrmann
` (14 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The destruct callback is optional and we provide an empty callback so remove it.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/btuart_cs.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
index 200b3a2..84e02f1 100644
--- a/drivers/bluetooth/btuart_cs.c
+++ b/drivers/bluetooth/btuart_cs.c
@@ -459,11 +459,6 @@ static int btuart_hci_send_frame(struct sk_buff *skb)
}
-static void btuart_hci_destruct(struct hci_dev *hdev)
-{
-}
-
-
static int btuart_hci_ioctl(struct hci_dev *hdev, unsigned int cmd, unsigned long arg)
{
return -ENOIOCTLCMD;
@@ -505,7 +500,6 @@ static int btuart_open(btuart_info_t *info)
hdev->close = btuart_hci_close;
hdev->flush = btuart_hci_flush;
hdev->send = btuart_hci_send_frame;
- hdev->destruct = btuart_hci_destruct;
hdev->ioctl = btuart_hci_ioctl;
hdev->owner = THIS_MODULE;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 06/18] Bluetooth: btwilink: Remove empty destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (4 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 05/18] Bluetooth: btuart-cs: " David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 07/18] Bluetooth: dtl1-cs: " David Herrmann
` (13 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The destruct cb is optional so remove our empty dummy cb.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/btwilink.c | 9 ---------
1 files changed, 0 insertions(+), 9 deletions(-)
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
index b5f83b4..da9cf6a 100644
--- a/drivers/bluetooth/btwilink.c
+++ b/drivers/bluetooth/btwilink.c
@@ -291,14 +291,6 @@ static int ti_st_send_frame(struct sk_buff *skb)
return 0;
}
-static void ti_st_destruct(struct hci_dev *hdev)
-{
- BT_DBG("%s", hdev->name);
- /* do nothing here, since platform remove
- * would free the hdev->driver_data
- */
-}
-
static int bt_ti_probe(struct platform_device *pdev)
{
static struct ti_st *hst;
@@ -325,7 +317,6 @@ static int bt_ti_probe(struct platform_device *pdev)
hdev->close = ti_st_close;
hdev->flush = NULL;
hdev->send = ti_st_send_frame;
- hdev->destruct = ti_st_destruct;
hdev->owner = THIS_MODULE;
err = hci_register_dev(hdev);
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 07/18] Bluetooth: dtl1-cs: Remove empty destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (5 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 06/18] Bluetooth: btwilink: " David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 08/18] Bluetooth: vhci: Free driver_data on file release David Herrmann
` (12 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The destruct cb is optional so we can safely remove our dummy cb.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/dtl1_cs.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
index b2db5e9..aae40ca 100644
--- a/drivers/bluetooth/dtl1_cs.c
+++ b/drivers/bluetooth/dtl1_cs.c
@@ -439,11 +439,6 @@ static int dtl1_hci_send_frame(struct sk_buff *skb)
}
-static void dtl1_hci_destruct(struct hci_dev *hdev)
-{
-}
-
-
static int dtl1_hci_ioctl(struct hci_dev *hdev, unsigned int cmd, unsigned long arg)
{
return -ENOIOCTLCMD;
@@ -487,7 +482,6 @@ static int dtl1_open(dtl1_info_t *info)
hdev->close = dtl1_hci_close;
hdev->flush = dtl1_hci_flush;
hdev->send = dtl1_hci_send_frame;
- hdev->destruct = dtl1_hci_destruct;
hdev->ioctl = dtl1_hci_ioctl;
hdev->owner = THIS_MODULE;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 08/18] Bluetooth: vhci: Free driver_data on file release
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (6 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 07/18] Bluetooth: dtl1-cs: " David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 09/18] Bluetooth: bfusb: Free driver_data on USB shutdown David Herrmann
` (11 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
This removes the hci-destruct callback and instead frees the private driver data
in the vhci_release file release function. There is no reason to keep private
driver data available if the driver has already shut down.
After vhci_release is called our module can be unloaded. The only reason it is
kept alive is the hci-core having a module-ref on us because of our destruct
callback. However, this callback only frees hdev->driver_data. That is, we wait
for the hdev-device to get destroyed to free our internal driver-data. In fact,
the hci-core does never touch hdev->driver_data so it doesn't care if it is
NULL. Therefore, we simply free it when unloading the driver.
Another important fact is that the hdev core does not call any callbacks other
than the destruct-cb after hci_unregister_dev() has been called. So there is no
function of our module that will be called nor does the hci-core touch
hdev->driver_data. Hence, no other code can touch hdev->driver_data after our
cleanup so the destruct callback is definitely unnecessary here.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/hci_vhci.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 2ed6ab1..44a8012 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -103,11 +103,6 @@ static int vhci_send_frame(struct sk_buff *skb)
return 0;
}
-static void vhci_destruct(struct hci_dev *hdev)
-{
- kfree(hdev->driver_data);
-}
-
static inline ssize_t vhci_get_user(struct vhci_data *data,
const char __user *buf, size_t count)
{
@@ -248,7 +243,6 @@ static int vhci_open(struct inode *inode, struct file *file)
hdev->close = vhci_close_dev;
hdev->flush = vhci_flush;
hdev->send = vhci_send_frame;
- hdev->destruct = vhci_destruct;
hdev->owner = THIS_MODULE;
@@ -273,6 +267,7 @@ static int vhci_release(struct inode *inode, struct file *file)
hci_free_dev(hdev);
file->private_data = NULL;
+ kfree(data);
return 0;
}
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 09/18] Bluetooth: bfusb: Free driver_data on USB shutdown
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (7 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 08/18] Bluetooth: vhci: Free driver_data on file release David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 10/18] Bluetooth: btusb: Free driver data " David Herrmann
` (10 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
This frees the private driver data on USB shutdown instead of using the
hci-destruct callback. We already call usb_set_intfdata(intf, NULL) but we do
not do the same with the hci object. This would be totally safe, though.
After calling hci_unregister_dev()/hci_free_dev() the hdev object will never
call any callback of us again except the destruct callback. Therefore, we can
safely set the destruct callback to NULL and free the driver data right away.
This allows to unload the module without waiting for the hdev device to be
released.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/bfusb.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index a936763..857a951 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -544,15 +544,6 @@ static int bfusb_send_frame(struct sk_buff *skb)
return 0;
}
-static void bfusb_destruct(struct hci_dev *hdev)
-{
- struct bfusb_data *data = hdev->driver_data;
-
- BT_DBG("hdev %p bfusb %p", hdev, data);
-
- kfree(data);
-}
-
static int bfusb_ioctl(struct hci_dev *hdev, unsigned int cmd, unsigned long arg)
{
return -ENOIOCTLCMD;
@@ -712,7 +703,6 @@ static int bfusb_probe(struct usb_interface *intf, const struct usb_device_id *i
hdev->close = bfusb_close;
hdev->flush = bfusb_flush;
hdev->send = bfusb_send_frame;
- hdev->destruct = bfusb_destruct;
hdev->ioctl = bfusb_ioctl;
hdev->owner = THIS_MODULE;
@@ -753,6 +743,7 @@ static void bfusb_disconnect(struct usb_interface *intf)
hci_unregister_dev(hdev);
hci_free_dev(hdev);
+ kfree(data);
}
static struct usb_driver bfusb_driver = {
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 10/18] Bluetooth: btusb: Free driver data on USB shutdown
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (8 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 09/18] Bluetooth: bfusb: Free driver_data on USB shutdown David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 11/18] Bluetooth: bpa10x: Free private driver data on usb shutdown David Herrmann
` (9 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
Instead of using the hci-destruct callback we free our private driver data on
USB shutdown. We already called hci_unregister_dev() here so the hci core will
never ever call our callbacks again except the destruct callback.
However, there is no reason to keep our *private* driver data alive if we get
never called again and the hci-core does never touch it the data. So we simply
free it right away and set the destruct callback to NULL.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/btusb.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index fbfba80..85bb17d 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -786,15 +786,6 @@ done:
return err;
}
-static void btusb_destruct(struct hci_dev *hdev)
-{
- struct btusb_data *data = hdev->driver_data;
-
- BT_DBG("%s", hdev->name);
-
- kfree(data);
-}
-
static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
{
struct btusb_data *data = hdev->driver_data;
@@ -1007,7 +998,6 @@ static int btusb_probe(struct usb_interface *intf,
hdev->close = btusb_close;
hdev->flush = btusb_flush;
hdev->send = btusb_send_frame;
- hdev->destruct = btusb_destruct;
hdev->notify = btusb_notify;
hdev->owner = THIS_MODULE;
@@ -1111,6 +1101,7 @@ static void btusb_disconnect(struct usb_interface *intf)
__hci_dev_put(hdev);
hci_free_dev(hdev);
+ kfree(data);
}
#ifdef CONFIG_PM
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 11/18] Bluetooth: bpa10x: Free private driver data on usb shutdown
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (9 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 10/18] Bluetooth: btusb: Free driver data " David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 12/18] Bluetooth: btsdio: Free driver data on SDIO shutdown David Herrmann
` (8 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
Instead of waiting for the hci-device to be destroyed we now free the private
driver data on driver shutdown right away. We call hci_unregister_dev() on
driver shutdown, that means, the hci-core will never ever call our callbacks
again except the destruct callback. It also does not access hdev->driver_data so
there is no reason to keep that alive.
We simply set the destruct cb to NULL to avoid getting called again.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/bpa10x.c | 15 +++------------
1 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index 751b338..92c2424 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -432,17 +432,6 @@ static int bpa10x_send_frame(struct sk_buff *skb)
return 0;
}
-static void bpa10x_destruct(struct hci_dev *hdev)
-{
- struct bpa10x_data *data = hdev->driver_data;
-
- BT_DBG("%s", hdev->name);
-
- kfree_skb(data->rx_skb[0]);
- kfree_skb(data->rx_skb[1]);
- kfree(data);
-}
-
static int bpa10x_probe(struct usb_interface *intf, const struct usb_device_id *id)
{
struct bpa10x_data *data;
@@ -480,7 +469,6 @@ static int bpa10x_probe(struct usb_interface *intf, const struct usb_device_id *
hdev->close = bpa10x_close;
hdev->flush = bpa10x_flush;
hdev->send = bpa10x_send_frame;
- hdev->destruct = bpa10x_destruct;
hdev->owner = THIS_MODULE;
@@ -512,6 +500,9 @@ static void bpa10x_disconnect(struct usb_interface *intf)
hci_unregister_dev(data->hdev);
hci_free_dev(data->hdev);
+ kfree_skb(data->rx_skb[0]);
+ kfree_skb(data->rx_skb[1]);
+ kfree(data);
}
static struct usb_driver bpa10x_driver = {
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 12/18] Bluetooth: btsdio: Free driver data on SDIO shutdown
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (10 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 11/18] Bluetooth: bpa10x: Free private driver data on usb shutdown David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 13/18] Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb David Herrmann
` (7 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
Instead of waiting for the hdev object to get freed we now free the private
driver-internal data on SDIO shutdown. This allows us to remove the obsolete
hci-destruct callback and free our data object right away after calling
hci_unregister_dev(). The HCI-core does not call any callbacks after this so we
are never called again and can safely exit the module.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/btsdio.c | 11 +----------
1 files changed, 1 insertions(+), 10 deletions(-)
diff --git a/drivers/bluetooth/btsdio.c b/drivers/bluetooth/btsdio.c
index 792e32d..d38945c 100644
--- a/drivers/bluetooth/btsdio.c
+++ b/drivers/bluetooth/btsdio.c
@@ -289,15 +289,6 @@ static int btsdio_send_frame(struct sk_buff *skb)
return 0;
}
-static void btsdio_destruct(struct hci_dev *hdev)
-{
- struct btsdio_data *data = hdev->driver_data;
-
- BT_DBG("%s", hdev->name);
-
- kfree(data);
-}
-
static int btsdio_probe(struct sdio_func *func,
const struct sdio_device_id *id)
{
@@ -345,7 +336,6 @@ static int btsdio_probe(struct sdio_func *func,
hdev->close = btsdio_close;
hdev->flush = btsdio_flush;
hdev->send = btsdio_send_frame;
- hdev->destruct = btsdio_destruct;
hdev->owner = THIS_MODULE;
@@ -378,6 +368,7 @@ static void btsdio_remove(struct sdio_func *func)
hci_unregister_dev(hdev);
hci_free_dev(hdev);
+ kfree(data);
}
static struct sdio_driver btsdio_driver = {
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 13/18] Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (11 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 12/18] Bluetooth: btsdio: Free driver data on SDIO shutdown David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 14/18] Bluetooth: Remove unused hci-destruct cb David Herrmann
` (6 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
We currently leak the hci_uart object if HCI_UART_PROTO_SET is never set because
the hci-destruct callback will then never be called.
This fix removes the hci-destruct callback and frees the driver internal private
hci_uart object directly on tty-close. We call hci_unregister_dev() here so the
hci-core will never call our callbacks again (except destruct). Therefore, we
can safely free the driver internal data right away and set the destruct
callback to NULL.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/hci_ldisc.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 48ad2a7..5ea49df 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -231,15 +231,6 @@ static int hci_uart_send_frame(struct sk_buff *skb)
return 0;
}
-static void hci_uart_destruct(struct hci_dev *hdev)
-{
- if (!hdev)
- return;
-
- BT_DBG("%s", hdev->name);
- kfree(hdev->driver_data);
-}
-
/* ------ LDISC part ------ */
/* hci_uart_tty_open
*
@@ -316,6 +307,8 @@ static void hci_uart_tty_close(struct tty_struct *tty)
hci_free_dev(hdev);
}
}
+
+ kfree(hu);
}
}
@@ -397,7 +390,6 @@ static int hci_uart_register_dev(struct hci_uart *hu)
hdev->close = hci_uart_close;
hdev->flush = hci_uart_flush;
hdev->send = hci_uart_send_frame;
- hdev->destruct = hci_uart_destruct;
hdev->parent = hu->tty->dev;
hdev->owner = THIS_MODULE;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 14/18] Bluetooth: Remove unused hci-destruct cb
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (12 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 13/18] Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 15/18] Bluetooth: Correctly acquire module ref David Herrmann
` (5 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The hci-destruct callback is not used by any driver so we can remove it. There
is no reason to keep it alive, anymore. Drivers can free their internal data on
driver-release and we do not need to provide a public destruct callback.
Internally, we still use a destruct callback inside of hci_sysfs.c. This one is
used to correctly free our hci_dev data structure if no more users have a
reference to it.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
include/net/bluetooth/hci_core.h | 6 +-----
1 files changed, 1 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b2c23e6..ab97ad3 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -254,7 +254,6 @@ struct hci_dev {
int (*close)(struct hci_dev *hdev);
int (*flush)(struct hci_dev *hdev);
int (*send)(struct sk_buff *skb);
- void (*destruct)(struct hci_dev *hdev);
void (*notify)(struct hci_dev *hdev, unsigned int evt);
int (*ioctl)(struct hci_dev *hdev, unsigned int cmd, unsigned long arg);
};
@@ -568,10 +567,7 @@ static inline void hci_conn_put(struct hci_conn *conn)
/* ----- HCI Devices ----- */
static inline void __hci_dev_put(struct hci_dev *d)
{
- if (atomic_dec_and_test(&d->refcnt)) {
- if (d->destruct)
- d->destruct(d);
- }
+ atomic_dec(&d->refcnt);
}
/*
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 15/18] Bluetooth: Correctly acquire module ref
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (13 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 14/18] Bluetooth: Remove unused hci-destruct cb David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 16/18] Bluetooth: Remove HCI-owner field David Herrmann
` (4 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
We provide a device-object to other subsystems and we provide our own
release-function. Therefore, the device-object must own a reference to our
module, otherwise the release-function may get deleted before the device-object
does.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
net/bluetooth/hci_sysfs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 5210956..e3bdee0 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -372,6 +372,7 @@ static void bt_host_release(struct device *dev)
{
void *data = dev_get_drvdata(dev);
kfree(data);
+ module_put(THIS_MODULE);
}
static struct device_type bt_host = {
@@ -523,6 +524,7 @@ void hci_init_sysfs(struct hci_dev *hdev)
dev->type = &bt_host;
dev->class = bt_class;
+ __module_get(THIS_MODULE);
dev_set_drvdata(dev, hdev);
device_initialize(dev);
}
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 16/18] Bluetooth: Remove HCI-owner field
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (14 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 15/18] Bluetooth: Correctly acquire module ref David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 17/18] Bluetooth: Correctly take hci_dev->dev refcount David Herrmann
` (3 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
After unregistering an hci_dev object a bluetooth driver does not have any
callbacks in the hci_dev structure left over. Therefore, there is no need to
keep a reference to the module.
Previously, we needed this to protect the hci-destruct callback. However, this
callback is no longer available so we do not need this owner field, anymore.
Drivers now call hci_unregister_dev() and they are done with the object.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/bfusb.c | 2 --
drivers/bluetooth/bluecard_cs.c | 2 --
drivers/bluetooth/bpa10x.c | 2 --
drivers/bluetooth/bt3c_cs.c | 2 --
drivers/bluetooth/btmrvl_main.c | 1 -
drivers/bluetooth/btsdio.c | 2 --
drivers/bluetooth/btuart_cs.c | 2 --
drivers/bluetooth/btusb.c | 2 --
drivers/bluetooth/btwilink.c | 1 -
drivers/bluetooth/dtl1_cs.c | 2 --
drivers/bluetooth/hci_ldisc.c | 2 --
drivers/bluetooth/hci_vhci.c | 2 --
include/net/bluetooth/hci_core.h | 13 ++-----------
net/bluetooth/hci_core.c | 3 +--
14 files changed, 3 insertions(+), 35 deletions(-)
diff --git a/drivers/bluetooth/bfusb.c b/drivers/bluetooth/bfusb.c
index 857a951..e97f42a 100644
--- a/drivers/bluetooth/bfusb.c
+++ b/drivers/bluetooth/bfusb.c
@@ -705,8 +705,6 @@ static int bfusb_probe(struct usb_interface *intf, const struct usb_device_id *i
hdev->send = bfusb_send_frame;
hdev->ioctl = bfusb_ioctl;
- hdev->owner = THIS_MODULE;
-
if (hci_register_dev(hdev) < 0) {
BT_ERR("Can't register HCI device");
hci_free_dev(hdev);
diff --git a/drivers/bluetooth/bluecard_cs.c b/drivers/bluetooth/bluecard_cs.c
index 5cb325a..6b1261f 100644
--- a/drivers/bluetooth/bluecard_cs.c
+++ b/drivers/bluetooth/bluecard_cs.c
@@ -738,8 +738,6 @@ static int bluecard_open(bluecard_info_t *info)
hdev->send = bluecard_hci_send_frame;
hdev->ioctl = bluecard_hci_ioctl;
- hdev->owner = THIS_MODULE;
-
id = inb(iobase + 0x30);
if ((id & 0x0f) == 0x02)
diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
index 92c2424..229bdc9 100644
--- a/drivers/bluetooth/bpa10x.c
+++ b/drivers/bluetooth/bpa10x.c
@@ -470,8 +470,6 @@ static int bpa10x_probe(struct usb_interface *intf, const struct usb_device_id *
hdev->flush = bpa10x_flush;
hdev->send = bpa10x_send_frame;
- hdev->owner = THIS_MODULE;
-
set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
err = hci_register_dev(hdev);
diff --git a/drivers/bluetooth/bt3c_cs.c b/drivers/bluetooth/bt3c_cs.c
index e74334d..0e304cb 100644
--- a/drivers/bluetooth/bt3c_cs.c
+++ b/drivers/bluetooth/bt3c_cs.c
@@ -584,8 +584,6 @@ static int bt3c_open(bt3c_info_t *info)
hdev->send = bt3c_hci_send_frame;
hdev->ioctl = bt3c_hci_ioctl;
- hdev->owner = THIS_MODULE;
-
/* Load firmware */
err = request_firmware(&firmware, "BT3CPCC.bin", &info->p_dev->dev);
if (err < 0) {
diff --git a/drivers/bluetooth/btmrvl_main.c b/drivers/bluetooth/btmrvl_main.c
index d62fc0d..d69c095 100644
--- a/drivers/bluetooth/btmrvl_main.c
+++ b/drivers/bluetooth/btmrvl_main.c
@@ -552,7 +552,6 @@ int btmrvl_register_hdev(struct btmrvl_private *priv)
hdev->flush = btmrvl_flush;
hdev->send = btmrvl_send_frame;
hdev->ioctl = btmrvl_ioctl;
- hdev->owner = THIS_MODULE;
btmrvl_send_module_cfg_cmd(priv, MODULE_BRINGUP_REQ);
diff --git a/drivers/bluetooth/btsdio.c b/drivers/bluetooth/btsdio.c
index d38945c..2d6e4ed 100644
--- a/drivers/bluetooth/btsdio.c
+++ b/drivers/bluetooth/btsdio.c
@@ -337,8 +337,6 @@ static int btsdio_probe(struct sdio_func *func,
hdev->flush = btsdio_flush;
hdev->send = btsdio_send_frame;
- hdev->owner = THIS_MODULE;
-
err = hci_register_dev(hdev);
if (err < 0) {
hci_free_dev(hdev);
diff --git a/drivers/bluetooth/btuart_cs.c b/drivers/bluetooth/btuart_cs.c
index 84e02f1..80ad2b9 100644
--- a/drivers/bluetooth/btuart_cs.c
+++ b/drivers/bluetooth/btuart_cs.c
@@ -502,8 +502,6 @@ static int btuart_open(btuart_info_t *info)
hdev->send = btuart_hci_send_frame;
hdev->ioctl = btuart_hci_ioctl;
- hdev->owner = THIS_MODULE;
-
spin_lock_irqsave(&(info->lock), flags);
/* Reset UART */
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 85bb17d..246944e 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1000,8 +1000,6 @@ static int btusb_probe(struct usb_interface *intf,
hdev->send = btusb_send_frame;
hdev->notify = btusb_notify;
- hdev->owner = THIS_MODULE;
-
/* Interface numbers are hardcoded in the specification */
data->isoc = usb_ifnum_to_if(data->udev, 1);
diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
index da9cf6a..b81b32e 100644
--- a/drivers/bluetooth/btwilink.c
+++ b/drivers/bluetooth/btwilink.c
@@ -317,7 +317,6 @@ static int bt_ti_probe(struct platform_device *pdev)
hdev->close = ti_st_close;
hdev->flush = NULL;
hdev->send = ti_st_send_frame;
- hdev->owner = THIS_MODULE;
err = hci_register_dev(hdev);
if (err < 0) {
diff --git a/drivers/bluetooth/dtl1_cs.c b/drivers/bluetooth/dtl1_cs.c
index aae40ca..295cf1b 100644
--- a/drivers/bluetooth/dtl1_cs.c
+++ b/drivers/bluetooth/dtl1_cs.c
@@ -484,8 +484,6 @@ static int dtl1_open(dtl1_info_t *info)
hdev->send = dtl1_hci_send_frame;
hdev->ioctl = dtl1_hci_ioctl;
- hdev->owner = THIS_MODULE;
-
spin_lock_irqsave(&(info->lock), flags);
/* Reset UART */
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index 5ea49df..459ff0b 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -392,8 +392,6 @@ static int hci_uart_register_dev(struct hci_uart *hu)
hdev->send = hci_uart_send_frame;
hdev->parent = hu->tty->dev;
- hdev->owner = THIS_MODULE;
-
if (!reset)
set_bit(HCI_QUIRK_NO_RESET, &hdev->quirks);
diff --git a/drivers/bluetooth/hci_vhci.c b/drivers/bluetooth/hci_vhci.c
index 44a8012..5f305c1 100644
--- a/drivers/bluetooth/hci_vhci.c
+++ b/drivers/bluetooth/hci_vhci.c
@@ -244,8 +244,6 @@ static int vhci_open(struct inode *inode, struct file *file)
hdev->flush = vhci_flush;
hdev->send = vhci_send_frame;
- hdev->owner = THIS_MODULE;
-
if (hci_register_dev(hdev) < 0) {
BT_ERR("Can't register HCI device");
kfree(data);
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ab97ad3..3d41b2e 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -246,8 +246,6 @@ struct hci_dev {
struct rfkill *rfkill;
- struct module *owner;
-
unsigned long dev_flags;
int (*open)(struct hci_dev *hdev);
@@ -574,11 +572,7 @@ static inline void __hci_dev_put(struct hci_dev *d)
* hci_dev_put and hci_dev_hold are macros to avoid dragging all the
* overhead of all the modular infrastructure into this header.
*/
-#define hci_dev_put(d) \
-do { \
- __hci_dev_put(d); \
- module_put(d->owner); \
-} while (0)
+#define hci_dev_put(d) __hci_dev_put(d)
static inline struct hci_dev *__hci_dev_hold(struct hci_dev *d)
{
@@ -586,10 +580,7 @@ static inline struct hci_dev *__hci_dev_hold(struct hci_dev *d)
return d;
}
-#define hci_dev_hold(d) \
-({ \
- try_module_get(d->owner) ? __hci_dev_hold(d) : NULL; \
-})
+#define hci_dev_hold(d) __hci_dev_hold(d)
#define hci_dev_lock(d) mutex_lock(&d->lock)
#define hci_dev_unlock(d) mutex_unlock(&d->lock)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 6fdd6e5..b37db46 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1447,8 +1447,7 @@ int hci_register_dev(struct hci_dev *hdev)
struct list_head *head = &hci_dev_list, *p;
int i, id, error;
- BT_DBG("%p name %s bus %d owner %p", hdev, hdev->name,
- hdev->bus, hdev->owner);
+ BT_DBG("%p name %s bus %d", hdev, hdev->name, hdev->bus);
if (!hdev->open || !hdev->close)
return -EINVAL;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 17/18] Bluetooth: Correctly take hci_dev->dev refcount
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (15 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 16/18] Bluetooth: Remove HCI-owner field David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-07 14:47 ` [PATCH 18/18] Bluetooth: Remove __hci_dev_put/hold David Herrmann
` (2 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
The hci_dev->dev device structure has an internal refcount. This refcount is
used to protect the whole hci_dev object. However, we currently do not use it.
Therefore, if someone calls hci_free_dev() we currently immediately destroy the
hci_dev object because we never took the device refcount.
This even happens if the hci_dev->refcnt is not 0. In fact, the hci_dev->refcnt
is totally useless in its current state. Therefore, we simply remove
hci_dev->refcnt and instead use hci_dev->dev refcnt.
This fixes all the symptoms and also correctly integrates the device structure
into our bluetooth bus system.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
include/net/bluetooth/hci_core.h | 5 ++---
net/bluetooth/hci_core.c | 2 +-
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3d41b2e..00da2e1 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -113,7 +113,6 @@ struct adv_entry {
struct hci_dev {
struct list_head list;
struct mutex lock;
- atomic_t refcnt;
char name[8];
unsigned long flags;
@@ -565,7 +564,7 @@ static inline void hci_conn_put(struct hci_conn *conn)
/* ----- HCI Devices ----- */
static inline void __hci_dev_put(struct hci_dev *d)
{
- atomic_dec(&d->refcnt);
+ put_device(&d->dev);
}
/*
@@ -576,7 +575,7 @@ static inline void __hci_dev_put(struct hci_dev *d)
static inline struct hci_dev *__hci_dev_hold(struct hci_dev *d)
{
- atomic_inc(&d->refcnt);
+ get_device(&d->dev);
return d;
}
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b37db46..3ee109f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1470,7 +1470,6 @@ int hci_register_dev(struct hci_dev *hdev)
hdev->id = id;
list_add_tail(&hdev->list, head);
- atomic_set(&hdev->refcnt, 1);
mutex_init(&hdev->lock);
hdev->flags = 0;
@@ -1554,6 +1553,7 @@ int hci_register_dev(struct hci_dev *hdev)
schedule_work(&hdev->power_on);
hci_notify(hdev, HCI_DEV_REG);
+ __hci_dev_hold(hdev);
return id;
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 18/18] Bluetooth: Remove __hci_dev_put/hold
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (16 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 17/18] Bluetooth: Correctly take hci_dev->dev refcount David Herrmann
@ 2012-01-07 14:47 ` David Herrmann
2012-01-09 7:54 ` [PATCH 00/18] Cleanup HCI reference counts Marcel Holtmann
2012-01-09 11:44 ` Johan Hedberg
19 siblings, 0 replies; 21+ messages in thread
From: David Herrmann @ 2012-01-07 14:47 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, marcel, David Herrmann
Since we remove the owner field of hci_dev hci_dev_put and __hci_dev_put do the
same so we can merge them into one function.
Same for hci_dev_hold and __hci_dev_hold.
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>
---
drivers/bluetooth/btusb.c | 4 ++--
include/net/bluetooth/hci_core.h | 12 ++----------
net/bluetooth/hci_core.c | 4 ++--
3 files changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 246944e..2dbecfa 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1082,7 +1082,7 @@ static void btusb_disconnect(struct usb_interface *intf)
hdev = data->hdev;
- __hci_dev_hold(hdev);
+ hci_dev_hold(hdev);
usb_set_intfdata(data->intf, NULL);
@@ -1096,7 +1096,7 @@ static void btusb_disconnect(struct usb_interface *intf)
else if (data->isoc)
usb_driver_release_interface(&btusb_driver, data->isoc);
- __hci_dev_put(hdev);
+ hci_dev_put(hdev);
hci_free_dev(hdev);
kfree(data);
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 00da2e1..d0ed6ee 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -562,25 +562,17 @@ static inline void hci_conn_put(struct hci_conn *conn)
}
/* ----- HCI Devices ----- */
-static inline void __hci_dev_put(struct hci_dev *d)
+static inline void hci_dev_put(struct hci_dev *d)
{
put_device(&d->dev);
}
-/*
- * hci_dev_put and hci_dev_hold are macros to avoid dragging all the
- * overhead of all the modular infrastructure into this header.
- */
-#define hci_dev_put(d) __hci_dev_put(d)
-
-static inline struct hci_dev *__hci_dev_hold(struct hci_dev *d)
+static inline struct hci_dev *hci_dev_hold(struct hci_dev *d)
{
get_device(&d->dev);
return d;
}
-#define hci_dev_hold(d) __hci_dev_hold(d)
-
#define hci_dev_lock(d) mutex_lock(&d->lock)
#define hci_dev_unlock(d) mutex_unlock(&d->lock)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3ee109f..80175ab 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1553,7 +1553,7 @@ int hci_register_dev(struct hci_dev *hdev)
schedule_work(&hdev->power_on);
hci_notify(hdev, HCI_DEV_REG);
- __hci_dev_hold(hdev);
+ hci_dev_hold(hdev);
return id;
@@ -1616,7 +1616,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
hci_adv_entries_clear(hdev);
hci_dev_unlock(hdev);
- __hci_dev_put(hdev);
+ hci_dev_put(hdev);
}
EXPORT_SYMBOL(hci_unregister_dev);
--
1.7.8.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 00/18] Cleanup HCI reference counts
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (17 preceding siblings ...)
2012-01-07 14:47 ` [PATCH 18/18] Bluetooth: Remove __hci_dev_put/hold David Herrmann
@ 2012-01-09 7:54 ` Marcel Holtmann
2012-01-09 11:44 ` Johan Hedberg
19 siblings, 0 replies; 21+ messages in thread
From: Marcel Holtmann @ 2012-01-09 7:54 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-bluetooth, padovan
Hi David,
> This is the same as my previous patchset but split into sequential patches. The
> first patch makes the destruct-cb optional so the patches 2-7 can remove the
> empty callbacks.
> Patches 8-14 make the remaining drivers drop the destruct-callback and directly
> free the driver data.
> Patches 15-18 then remove the (now unused!) destruct callback and then properly
> fixes the reference counts (same patches as already posted).
>
> I hope the first 14 patches make clear that half of our drivers already work
> without the destruct callback and the other half can be safely converted into
> not using it. In fact, the drivers that use it introduce some memleaks which are
> fixed by simply dropping the callback and freeing everything right away.
>
> I've added detailed commit messages to each driver. If a single step is unclear,
> please point me to it.
>
>
> By the way, this fixes several bugs we currently have, including some mem-leaks
> as described in the commits and the fact that hci_free_dev() currently
> *directly* destroys the hci object regardless of any previous calls to
> hci_hold_dev/hci_put_dev(). This is because the hci->refcnt counter is in no way
> linked to the lifetime of the hci object. See hci_sysfs the release_host
> callback which actually frees the hci object. It is in no way triggered by
> hci->refcnt.
> This patchset fixes all this and introduces proper reference counting.
I went through the patches again and after thinking about it for a bit,
this all looks good.
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Johan, feel free to include this patchset into your tree.
Regards
Marcel
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/18] Cleanup HCI reference counts
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
` (18 preceding siblings ...)
2012-01-09 7:54 ` [PATCH 00/18] Cleanup HCI reference counts Marcel Holtmann
@ 2012-01-09 11:44 ` Johan Hedberg
19 siblings, 0 replies; 21+ messages in thread
From: Johan Hedberg @ 2012-01-09 11:44 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-bluetooth, padovan, marcel
Hi David,
On Sat, Jan 07, 2012, David Herrmann wrote:
> David Herrmann (18):
> Bluetooth: Make hci-destruct callback optional
> Bluetooth: bluecard-cs: Remove empty destruct cb
> Bluetooth: bt3c-cs: Remove empty destruct cb
> Bluetooth: btmrvl: Remove empty destruct cb
> Bluetooth: btuart-cs: Remove empty destruct cb
> Bluetooth: btwilink: Remove empty destruct cb
> Bluetooth: dtl1-cs: Remove empty destruct cb
> Bluetooth: vhci: Free driver_data on file release
> Bluetooth: bfusb: Free driver_data on USB shutdown
> Bluetooth: btusb: Free driver data on USB shutdown
> Bluetooth: bpa10x: Free private driver data on usb shutdown
> Bluetooth: btsdio: Free driver data on SDIO shutdown
> Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb
> Bluetooth: Remove unused hci-destruct cb
> Bluetooth: Correctly acquire module ref
> Bluetooth: Remove HCI-owner field
> Bluetooth: Correctly take hci_dev->dev refcount
> Bluetooth: Remove __hci_dev_put/hold
>
> drivers/bluetooth/bfusb.c | 13 +------------
> drivers/bluetooth/bluecard_cs.c | 8 --------
> drivers/bluetooth/bpa10x.c | 17 +++--------------
> drivers/bluetooth/bt3c_cs.c | 8 --------
> drivers/bluetooth/btmrvl_main.c | 6 ------
> drivers/bluetooth/btsdio.c | 13 +------------
> drivers/bluetooth/btuart_cs.c | 8 --------
> drivers/bluetooth/btusb.c | 17 +++--------------
> drivers/bluetooth/btwilink.c | 10 ----------
> drivers/bluetooth/dtl1_cs.c | 8 --------
> drivers/bluetooth/hci_ldisc.c | 14 ++------------
> drivers/bluetooth/hci_vhci.c | 9 +--------
> include/net/bluetooth/hci_core.h | 28 ++++------------------------
> net/bluetooth/hci_core.c | 9 ++++-----
> net/bluetooth/hci_sysfs.c | 2 ++
> 15 files changed, 21 insertions(+), 149 deletions(-)
All of these patches have been applied to my bluetooth-next tree. In the
future please adjust your editor settings so that your commit message
lines aren't longer than 72 characters. The message should be fully
visible with git log (which indents the messages) on a 80-wide terminal
I ended up fixing the messages manually this time.
Johan
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2012-01-09 11:44 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-07 14:47 [PATCH 00/18] Cleanup HCI reference counts David Herrmann
2012-01-07 14:47 ` [PATCH 01/18] Bluetooth: Make hci-destruct callback optional David Herrmann
2012-01-07 14:47 ` [PATCH 02/18] Bluetooth: bluecard-cs: Remove empty destruct cb David Herrmann
2012-01-07 14:47 ` [PATCH 03/18] Bluetooth: bt3c-cs: " David Herrmann
2012-01-07 14:47 ` [PATCH 04/18] Bluetooth: btmrvl: " David Herrmann
2012-01-07 14:47 ` [PATCH 05/18] Bluetooth: btuart-cs: " David Herrmann
2012-01-07 14:47 ` [PATCH 06/18] Bluetooth: btwilink: " David Herrmann
2012-01-07 14:47 ` [PATCH 07/18] Bluetooth: dtl1-cs: " David Herrmann
2012-01-07 14:47 ` [PATCH 08/18] Bluetooth: vhci: Free driver_data on file release David Herrmann
2012-01-07 14:47 ` [PATCH 09/18] Bluetooth: bfusb: Free driver_data on USB shutdown David Herrmann
2012-01-07 14:47 ` [PATCH 10/18] Bluetooth: btusb: Free driver data " David Herrmann
2012-01-07 14:47 ` [PATCH 11/18] Bluetooth: bpa10x: Free private driver data on usb shutdown David Herrmann
2012-01-07 14:47 ` [PATCH 12/18] Bluetooth: btsdio: Free driver data on SDIO shutdown David Herrmann
2012-01-07 14:47 ` [PATCH 13/18] Bluetooth: uart-ldisc: Fix memory leak and remove destruct cb David Herrmann
2012-01-07 14:47 ` [PATCH 14/18] Bluetooth: Remove unused hci-destruct cb David Herrmann
2012-01-07 14:47 ` [PATCH 15/18] Bluetooth: Correctly acquire module ref David Herrmann
2012-01-07 14:47 ` [PATCH 16/18] Bluetooth: Remove HCI-owner field David Herrmann
2012-01-07 14:47 ` [PATCH 17/18] Bluetooth: Correctly take hci_dev->dev refcount David Herrmann
2012-01-07 14:47 ` [PATCH 18/18] Bluetooth: Remove __hci_dev_put/hold David Herrmann
2012-01-09 7:54 ` [PATCH 00/18] Cleanup HCI reference counts Marcel Holtmann
2012-01-09 11:44 ` Johan Hedberg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).