* [RFC 00/22] Bluetooth: change tasklets to workqueue
@ 2011-12-17 21:29 Gustavo F. Padovan
2011-12-17 21:29 ` [RFC 01/22] Bluetooth: Process recv path in a workqueue instead of a tasklet Gustavo F. Padovan
` (2 more replies)
0 siblings, 3 replies; 34+ messages in thread
From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Gustavo F. Padovan
From: "Gustavo F. Padovan" <padovan@profusion.mobi>
Here are the patches to run the receive path in workqueue. With these patches
l2cap and rfcomm seems to be working fine, but some more work still needs to be
done.
Please review.
Gustavo F. Padovan (21):
Bluetooth: Replace spin_lock by mutex in hci_dev
Bluetooth: Use delayed_work for connection timeout
Bluetooth: Use delayed work for advertisiment cache timeout
Bluetooth: hci_conn_auto_accept() doesn't need locking
Bluetooth: Move L2CAP timers to workqueue
Bluetooth: Don't use spin_lock socket lock anymore
Bluetooth: Remove sk_backlog usage from L2CAP
Bluetooth: move hci_task_lock to mutex
Bluetooth: convert chan_lock to mutex
Bluetooth: Use RCU to manipulate chan_list
Bluetooth: convert conn hash to RCU
Bluetooth: Don't disable tasklets to call hdev->notify()
Bluetooth: Move command task to workqueue
Bluetooth: convert tx_task to workqueue
Bluetooth: convert info timer to delayed_work
Bluetooth: remove power_on work_struct
Bluetooth: invert locking order in connect path
Bluetooth: Change l2cap chan_list to use RCU
Bluetooth: move power_off to system workqueue
Bluetooth: Use new alloc_workqueue()
Bluetooth: Remove work_add and work_del from hci_sysfs
Marcel Holtmann (1):
Bluetooth: Process recv path in a workqueue instead of a tasklet
include/net/bluetooth/hci_core.h | 75 +++++----
include/net/bluetooth/l2cap.h | 24 ++--
net/bluetooth/hci_conn.c | 48 ++----
net/bluetooth/hci_core.c | 165 ++++++++++--------
net/bluetooth/hci_event.c | 26 +--
net/bluetooth/hci_sock.c | 14 +-
net/bluetooth/hci_sysfs.c | 91 ++++------
net/bluetooth/hidp/core.c | 4 +-
net/bluetooth/l2cap_core.c | 345 +++++++++++++++++++-------------------
net/bluetooth/l2cap_sock.c | 61 +-------
net/bluetooth/mgmt.c | 108 ++++++------
net/bluetooth/sco.c | 4 +-
12 files changed, 456 insertions(+), 509 deletions(-)
--
1.7.6.4
^ permalink raw reply [flat|nested] 34+ messages in thread* [RFC 01/22] Bluetooth: Process recv path in a workqueue instead of a tasklet 2011-12-17 21:29 [RFC 00/22] Bluetooth: change tasklets to workqueue Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 02/22] Bluetooth: Replace spin_lock by mutex in hci_dev Gustavo F. Padovan 2011-12-17 21:34 ` [RFC 00/22] Bluetooth: change tasklets " Gustavo Padovan 2011-12-17 22:17 ` Marcel Holtmann 2 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Marcel Holtmann From: Marcel Holtmann <marcel@holtmann.org> Signed-off-by: Marcel Holtmann <marcel@holtmann.org> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 3 ++- net/bluetooth/hci_core.c | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 6a1ac2c..1e28be4 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -194,8 +194,9 @@ struct hci_dev { struct delayed_work discov_off; struct timer_list cmd_timer; + + struct work_struct rx_work; struct tasklet_struct cmd_task; - struct tasklet_struct rx_task; struct tasklet_struct tx_task; struct sk_buff_head rx_q; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 700d0ab..223809f 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -56,8 +56,8 @@ int enable_hs; +static void hci_rx_work(struct work_struct *work); static void hci_cmd_task(unsigned long arg); -static void hci_rx_task(unsigned long arg); static void hci_tx_task(unsigned long arg); static DEFINE_RWLOCK(hci_task_lock); @@ -547,9 +547,9 @@ int hci_dev_open(__u16 dev) } } else { /* Init failed, cleanup */ - tasklet_kill(&hdev->rx_task); tasklet_kill(&hdev->tx_task); tasklet_kill(&hdev->cmd_task); + flush_work(&hdev->rx_work); skb_queue_purge(&hdev->cmd_q); skb_queue_purge(&hdev->rx_q); @@ -586,7 +586,7 @@ static int hci_dev_do_close(struct hci_dev *hdev) } /* Kill RX and TX tasks */ - tasklet_kill(&hdev->rx_task); + cancel_work_sync(&hdev->rx_work); tasklet_kill(&hdev->tx_task); if (hdev->discov_timeout > 0) { @@ -617,8 +617,9 @@ static int hci_dev_do_close(struct hci_dev *hdev) clear_bit(HCI_INIT, &hdev->flags); } - /* Kill cmd task */ + /* Kill cmd task and evt work */ tasklet_kill(&hdev->cmd_task); + flush_work(&hdev->rx_work); /* Drop queues */ skb_queue_purge(&hdev->rx_q); @@ -1456,8 +1457,9 @@ int hci_register_dev(struct hci_dev *hdev) hdev->sniff_max_interval = 800; hdev->sniff_min_interval = 80; - tasklet_init(&hdev->cmd_task, hci_cmd_task, (unsigned long) hdev); - tasklet_init(&hdev->rx_task, hci_rx_task, (unsigned long) hdev); + INIT_WORK(&hdev->rx_work, hci_rx_work); + + tasklet_init(&hdev->cmd_task, hci_cmd_task,(unsigned long) hdev); tasklet_init(&hdev->tx_task, hci_tx_task, (unsigned long) hdev); skb_queue_head_init(&hdev->rx_q); @@ -1623,9 +1625,8 @@ int hci_recv_frame(struct sk_buff *skb) /* Time stamp */ __net_timestamp(skb); - /* Queue frame for rx task */ skb_queue_tail(&hdev->rx_q, skb); - tasklet_schedule(&hdev->rx_task); + queue_work(hdev->workqueue, &hdev->rx_work); return 0; } @@ -2486,9 +2487,9 @@ static inline void hci_scodata_packet(struct hci_dev *hdev, struct sk_buff *skb) kfree_skb(skb); } -static void hci_rx_task(unsigned long arg) +static void hci_rx_work(struct work_struct *work) { - struct hci_dev *hdev = (struct hci_dev *) arg; + struct hci_dev *hdev = container_of(work, struct hci_dev, rx_work); struct sk_buff *skb; BT_DBG("%s", hdev->name); @@ -2519,6 +2520,7 @@ static void hci_rx_task(unsigned long arg) /* Process frame */ switch (bt_cb(skb)->pkt_type) { case HCI_EVENT_PKT: + BT_DBG("%s Event packet", hdev->name); hci_event_packet(hdev, skb); break; -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 02/22] Bluetooth: Replace spin_lock by mutex in hci_dev 2011-12-17 21:29 ` [RFC 01/22] Bluetooth: Process recv path in a workqueue instead of a tasklet Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 03/22] Bluetooth: Use delayed_work for connection timeout Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Now we run everything in HCI in process context, so it's a better idea use mutex instead spin_lock. The macro remains hci_dev_lock() (and I got rid of hci_dev_lock_bh()), of course. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 8 +-- net/bluetooth/hci_conn.c | 12 ++-- net/bluetooth/hci_core.c | 38 +++++++------- net/bluetooth/hci_sock.c | 8 ++-- net/bluetooth/hci_sysfs.c | 20 ++++---- net/bluetooth/hidp/core.c | 4 +- net/bluetooth/l2cap_core.c | 4 +- net/bluetooth/mgmt.c | 104 +++++++++++++++++++------------------- net/bluetooth/sco.c | 4 +- 9 files changed, 100 insertions(+), 102 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 1e28be4..e7dbe59 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -117,7 +117,7 @@ struct adv_entry { #define NUM_REASSEMBLY 4 struct hci_dev { struct list_head list; - spinlock_t lock; + struct mutex lock; atomic_t refcnt; char name[8]; @@ -566,10 +566,8 @@ static inline struct hci_dev *hci_dev_hold(struct hci_dev *d) return NULL; } -#define hci_dev_lock(d) spin_lock(&d->lock) -#define hci_dev_unlock(d) spin_unlock(&d->lock) -#define hci_dev_lock_bh(d) spin_lock_bh(&d->lock) -#define hci_dev_unlock_bh(d) spin_unlock_bh(&d->lock) +#define hci_dev_lock(d) mutex_lock(&d->lock) +#define hci_dev_unlock(d) mutex_unlock(&d->lock) struct hci_dev *hci_dev_get(int index); struct hci_dev *hci_get_route(bdaddr_t *src, bdaddr_t *dst); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 3131a99..d45783d 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -876,7 +876,7 @@ int hci_get_conn_list(void __user *arg) ci = cl->conn_info; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); list_for_each_entry(c, &hdev->conn_hash.list, list) { bacpy(&(ci + n)->bdaddr, &c->dst); (ci + n)->handle = c->handle; @@ -887,7 +887,7 @@ int hci_get_conn_list(void __user *arg) if (++n >= req.conn_num) break; } - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); cl->dev_id = hdev->id; cl->conn_num = n; @@ -911,7 +911,7 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&req, arg, sizeof(req))) return -EFAULT; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, req.type, &req.bdaddr); if (conn) { bacpy(&ci.bdaddr, &conn->dst); @@ -921,7 +921,7 @@ int hci_get_conn_info(struct hci_dev *hdev, void __user *arg) ci.state = conn->state; ci.link_mode = conn->link_mode; } - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); if (!conn) return -ENOENT; @@ -937,11 +937,11 @@ int hci_get_auth_info(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&req, arg, sizeof(req))) return -EFAULT; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &req.bdaddr); if (conn) req.type = conn->auth_type; - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); if (!conn) return -ENOENT; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 223809f..279074c 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -433,14 +433,14 @@ int hci_inquiry(void __user *arg) if (!hdev) return -ENODEV; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (inquiry_cache_age(hdev) > INQUIRY_CACHE_AGE_MAX || inquiry_cache_empty(hdev) || ir.flags & IREQ_CACHE_FLUSH) { inquiry_cache_flush(hdev); do_inquiry = 1; } - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); timeo = ir.length * msecs_to_jiffies(2000); @@ -462,9 +462,9 @@ int hci_inquiry(void __user *arg) goto done; } - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); ir.num_rsp = inquiry_cache_dump(hdev, max_rsp, buf); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); BT_DBG("num_rsp %d", ir.num_rsp); @@ -541,9 +541,9 @@ int hci_dev_open(__u16 dev) set_bit(HCI_UP, &hdev->flags); hci_notify(hdev, HCI_DEV_UP); if (!test_bit(HCI_SETUP, &hdev->flags)) { - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); mgmt_powered(hdev, 1); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); } } else { /* Init failed, cleanup */ @@ -597,10 +597,10 @@ static int hci_dev_do_close(struct hci_dev *hdev) if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->flags)) cancel_delayed_work(&hdev->power_off); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); inquiry_cache_flush(hdev); hci_conn_hash_flush(hdev); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_notify(hdev, HCI_DEV_DOWN); @@ -637,9 +637,9 @@ static int hci_dev_do_close(struct hci_dev *hdev) * and no tasks are scheduled. */ hdev->close(hdev); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); mgmt_powered(hdev, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); /* Clear flags */ hdev->flags = 0; @@ -682,10 +682,10 @@ int hci_dev_reset(__u16 dev) skb_queue_purge(&hdev->rx_q); skb_queue_purge(&hdev->cmd_q); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); inquiry_cache_flush(hdev); hci_conn_hash_flush(hdev); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); if (hdev->flush) hdev->flush(hdev); @@ -968,13 +968,13 @@ static void hci_discov_off(struct work_struct *work) BT_DBG("%s", hdev->name); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); hci_send_cmd(hdev, HCI_OP_WRITE_SCAN_ENABLE, sizeof(scan), &scan); hdev->discov_timeout = 0; - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); } int hci_uuids_clear(struct hci_dev *hdev) @@ -1444,7 +1444,7 @@ int hci_register_dev(struct hci_dev *hdev) list_add_tail(&hdev->list, head); atomic_set(&hdev->refcnt, 1); - spin_lock_init(&hdev->lock); + mutex_init(&hdev->lock); hdev->flags = 0; hdev->dev_flags = 0; @@ -1559,9 +1559,9 @@ void hci_unregister_dev(struct hci_dev *hdev) if (!test_bit(HCI_INIT, &hdev->flags) && !test_bit(HCI_SETUP, &hdev->flags)) { - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); mgmt_index_removed(hdev); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); } /* mgmt_index_removed should take care of emptying the @@ -1581,13 +1581,13 @@ void hci_unregister_dev(struct hci_dev *hdev) destroy_workqueue(hdev->workqueue); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); hci_blacklist_clear(hdev); hci_uuids_clear(hdev); hci_link_keys_clear(hdev); hci_remote_oob_data_clear(hdev); hci_adv_entries_clear(hdev); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); __hci_dev_put(hdev); } diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index f6afe3d..399be34 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -188,11 +188,11 @@ static int hci_sock_blacklist_add(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&bdaddr, arg, sizeof(bdaddr))) return -EFAULT; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); err = hci_blacklist_add(hdev, &bdaddr); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return err; } @@ -205,11 +205,11 @@ static int hci_sock_blacklist_del(struct hci_dev *hdev, void __user *arg) if (copy_from_user(&bdaddr, arg, sizeof(bdaddr))) return -EFAULT; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); err = hci_blacklist_del(hdev, &bdaddr); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return err; } diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index f8e6aa3..c3c1ec8 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -402,7 +402,7 @@ static int inquiry_cache_show(struct seq_file *f, void *p) struct inquiry_cache *cache = &hdev->inq_cache; struct inquiry_entry *e; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); for (e = cache->list; e; e = e->next) { struct inquiry_data *data = &e->data; @@ -415,7 +415,7 @@ static int inquiry_cache_show(struct seq_file *f, void *p) data->rssi, data->ssp_mode, e->timestamp); } - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return 0; } @@ -437,12 +437,12 @@ static int blacklist_show(struct seq_file *f, void *p) struct hci_dev *hdev = f->private; struct bdaddr_list *b; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); list_for_each_entry(b, &hdev->blacklist, list) seq_printf(f, "%s\n", batostr(&b->bdaddr)); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return 0; } @@ -481,12 +481,12 @@ static int uuids_show(struct seq_file *f, void *p) struct hci_dev *hdev = f->private; struct bt_uuid *uuid; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); list_for_each_entry(uuid, &hdev->uuids, list) print_bt_uuid(f, uuid->uuid); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return 0; } @@ -507,11 +507,11 @@ static int auto_accept_delay_set(void *data, u64 val) { struct hci_dev *hdev = data; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); hdev->auto_accept_delay = val; - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return 0; } @@ -520,11 +520,11 @@ static int auto_accept_delay_get(void *data, u64 *val) { struct hci_dev *hdev = data; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); *val = hdev->auto_accept_delay; - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); return 0; } diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 3c2d888..d478be1 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -795,11 +795,11 @@ static struct hci_conn *hidp_get_connection(struct hidp_session *session) if (!hdev) return NULL; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst); if (conn) hci_conn_hold_device(conn); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 014fdec..0369a9b 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1171,7 +1171,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan) if (!hdev) return -EHOSTUNREACH; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); auth_type = l2cap_get_auth_type(chan); @@ -1214,7 +1214,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan) err = 0; done: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; } diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 7a23f21..ad4817c 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -257,7 +257,7 @@ static int read_controller_info(struct sock *sk, u16 index) if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->flags)) cancel_delayed_work_sync(&hdev->power_off); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); set_bit(HCI_MGMT, &hdev->flags); @@ -286,7 +286,7 @@ static int read_controller_info(struct sock *sk, u16 index) memcpy(rp.name, hdev->dev_name, sizeof(hdev->dev_name)); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return cmd_complete(sk, index, MGMT_OP_READ_INFO, &rp, sizeof(rp)); @@ -394,7 +394,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len) return cmd_status(sk, index, MGMT_OP_SET_POWERED, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); up = test_bit(HCI_UP, &hdev->flags); if ((cp->val && up) || (!cp->val && !up)) { @@ -422,7 +422,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len) err = 0; failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; } @@ -449,7 +449,7 @@ static int set_discoverable(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_DISCOVERABLE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_SET_DISCOVERABLE, @@ -492,7 +492,7 @@ static int set_discoverable(struct sock *sk, u16 index, unsigned char *data, hdev->discov_timeout = get_unaligned_le16(&cp->timeout); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -520,7 +520,7 @@ static int set_connectable(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_CONNECTABLE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_SET_CONNECTABLE, @@ -557,7 +557,7 @@ static int set_connectable(struct sock *sk, u16 index, unsigned char *data, mgmt_pending_remove(cmd); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -612,7 +612,7 @@ static int set_pairable(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_PAIRABLE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (cp->val) set_bit(HCI_PAIRABLE, &hdev->flags); @@ -628,7 +628,7 @@ static int set_pairable(struct sock *sk, u16 index, unsigned char *data, err = mgmt_event(MGMT_EV_PAIRABLE, hdev, &ev, sizeof(ev), sk); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -827,7 +827,7 @@ static int add_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len) return cmd_status(sk, index, MGMT_OP_ADD_UUID, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); uuid = kmalloc(sizeof(*uuid), GFP_ATOMIC); if (!uuid) { @@ -851,7 +851,7 @@ static int add_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len) err = cmd_complete(sk, index, MGMT_OP_ADD_UUID, NULL, 0); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -878,7 +878,7 @@ static int remove_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len) return cmd_status(sk, index, MGMT_OP_REMOVE_UUID, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (memcmp(cp->uuid, bt_uuid_any, 16) == 0) { err = hci_uuids_clear(hdev); @@ -914,7 +914,7 @@ static int remove_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len) err = cmd_complete(sk, index, MGMT_OP_REMOVE_UUID, NULL, 0); unlock: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -940,7 +940,7 @@ static int set_dev_class(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_DEV_CLASS, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); hdev->major_class = cp->major; hdev->minor_class = cp->minor; @@ -950,7 +950,7 @@ static int set_dev_class(struct sock *sk, u16 index, unsigned char *data, if (err == 0) err = cmd_complete(sk, index, MGMT_OP_SET_DEV_CLASS, NULL, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -974,7 +974,7 @@ static int set_service_cache(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_SERVICE_CACHE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); BT_DBG("hci%u enable %d", index, cp->enable); @@ -995,7 +995,7 @@ static int set_service_cache(struct sock *sk, u16 index, unsigned char *data, cmd_status(sk, index, MGMT_OP_SET_SERVICE_CACHE, -err); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1034,7 +1034,7 @@ static int load_link_keys(struct sock *sk, u16 index, unsigned char *data, BT_DBG("hci%u debug_keys %u key_count %u", index, cp->debug_keys, key_count); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); hci_link_keys_clear(hdev); @@ -1054,7 +1054,7 @@ static int load_link_keys(struct sock *sk, u16 index, unsigned char *data, cmd_complete(sk, index, MGMT_OP_LOAD_LINK_KEYS, NULL, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return 0; @@ -1082,7 +1082,7 @@ static int remove_keys(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_REMOVE_KEYS, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); memset(&rp, 0, sizeof(rp)); bacpy(&rp.bdaddr, &cp->bdaddr); @@ -1123,7 +1123,7 @@ unlock: if (err < 0) err = cmd_complete(sk, index, MGMT_OP_REMOVE_KEYS, &rp, sizeof(rp)); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1151,7 +1151,7 @@ static int disconnect(struct sock *sk, u16 index, unsigned char *data, u16 len) return cmd_status(sk, index, MGMT_OP_DISCONNECT, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_DISCONNECT, @@ -1189,7 +1189,7 @@ static int disconnect(struct sock *sk, u16 index, unsigned char *data, u16 len) mgmt_pending_remove(cmd); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1231,7 +1231,7 @@ static int get_connections(struct sock *sk, u16 index) return cmd_status(sk, index, MGMT_OP_GET_CONNECTIONS, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); count = 0; list_for_each(p, &hdev->conn_hash.list) { @@ -1263,7 +1263,7 @@ static int get_connections(struct sock *sk, u16 index) unlock: kfree(rp); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; } @@ -1311,7 +1311,7 @@ static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_PIN_CODE_REPLY, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_PIN_CODE_REPLY, @@ -1354,7 +1354,7 @@ static int pin_code_reply(struct sock *sk, u16 index, unsigned char *data, mgmt_pending_remove(cmd); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1380,7 +1380,7 @@ static int pin_code_neg_reply(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_PIN_CODE_NEG_REPLY, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_PIN_CODE_NEG_REPLY, @@ -1391,7 +1391,7 @@ static int pin_code_neg_reply(struct sock *sk, u16 index, unsigned char *data, err = send_pin_code_neg_reply(sk, index, hdev, cp); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1416,14 +1416,14 @@ static int set_io_capability(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_IO_CAPABILITY, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); hdev->io_capability = cp->io_capability; BT_DBG("%s IO capability set to 0x%02x", hdev->name, hdev->io_capability); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return cmd_complete(sk, index, MGMT_OP_SET_IO_CAPABILITY, NULL, 0); @@ -1504,7 +1504,7 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len) return cmd_status(sk, index, MGMT_OP_PAIR_DEVICE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); sec_level = BT_SECURITY_MEDIUM; if (cp->io_cap == 0x03) @@ -1561,7 +1561,7 @@ static int pair_device(struct sock *sk, u16 index, unsigned char *data, u16 len) err = 0; unlock: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1580,7 +1580,7 @@ static int user_pairing_resp(struct sock *sk, u16 index, bdaddr_t *bdaddr, return cmd_status(sk, index, mgmt_op, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, mgmt_op, MGMT_STATUS_NOT_POWERED); @@ -1631,7 +1631,7 @@ static int user_pairing_resp(struct sock *sk, u16 index, bdaddr_t *bdaddr, mgmt_pending_remove(cmd); done: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1719,7 +1719,7 @@ static int set_local_name(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_SET_LOCAL_NAME, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); cmd = mgmt_pending_add(sk, MGMT_OP_SET_LOCAL_NAME, hdev, data, len); if (!cmd) { @@ -1734,7 +1734,7 @@ static int set_local_name(struct sock *sk, u16 index, unsigned char *data, mgmt_pending_remove(cmd); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1753,7 +1753,7 @@ static int read_local_oob_data(struct sock *sk, u16 index) return cmd_status(sk, index, MGMT_OP_READ_LOCAL_OOB_DATA, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_READ_LOCAL_OOB_DATA, @@ -1784,7 +1784,7 @@ static int read_local_oob_data(struct sock *sk, u16 index) mgmt_pending_remove(cmd); unlock: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1808,7 +1808,7 @@ static int add_remote_oob_data(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_ADD_REMOTE_OOB_DATA, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); err = hci_add_remote_oob_data(hdev, &cp->bdaddr, cp->hash, cp->randomizer); @@ -1819,7 +1819,7 @@ static int add_remote_oob_data(struct sock *sk, u16 index, unsigned char *data, err = cmd_complete(sk, index, MGMT_OP_ADD_REMOTE_OOB_DATA, NULL, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1843,7 +1843,7 @@ static int remove_remote_oob_data(struct sock *sk, u16 index, return cmd_status(sk, index, MGMT_OP_REMOVE_REMOTE_OOB_DATA, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); err = hci_remove_remote_oob_data(hdev, &cp->bdaddr); if (err < 0) @@ -1853,7 +1853,7 @@ static int remove_remote_oob_data(struct sock *sk, u16 index, err = cmd_complete(sk, index, MGMT_OP_REMOVE_REMOTE_OOB_DATA, NULL, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1878,7 +1878,7 @@ static int start_discovery(struct sock *sk, u16 index, return cmd_status(sk, index, MGMT_OP_START_DISCOVERY, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (!test_bit(HCI_UP, &hdev->flags)) { err = cmd_status(sk, index, MGMT_OP_START_DISCOVERY, @@ -1897,7 +1897,7 @@ static int start_discovery(struct sock *sk, u16 index, mgmt_pending_remove(cmd); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1916,7 +1916,7 @@ static int stop_discovery(struct sock *sk, u16 index) return cmd_status(sk, index, MGMT_OP_STOP_DISCOVERY, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); cmd = mgmt_pending_add(sk, MGMT_OP_STOP_DISCOVERY, hdev, NULL, 0); if (!cmd) { @@ -1929,7 +1929,7 @@ static int stop_discovery(struct sock *sk, u16 index) mgmt_pending_remove(cmd); failed: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1953,7 +1953,7 @@ static int block_device(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_BLOCK_DEVICE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); err = hci_blacklist_add(hdev, &cp->bdaddr); if (err < 0) @@ -1963,7 +1963,7 @@ static int block_device(struct sock *sk, u16 index, unsigned char *data, err = cmd_complete(sk, index, MGMT_OP_BLOCK_DEVICE, NULL, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; @@ -1987,7 +1987,7 @@ static int unblock_device(struct sock *sk, u16 index, unsigned char *data, return cmd_status(sk, index, MGMT_OP_UNBLOCK_DEVICE, MGMT_STATUS_INVALID_PARAMS); - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); err = hci_blacklist_del(hdev, &cp->bdaddr); @@ -1998,7 +1998,7 @@ static int unblock_device(struct sock *sk, u16 index, unsigned char *data, err = cmd_complete(sk, index, MGMT_OP_UNBLOCK_DEVICE, NULL, 0); - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index a324b00..725e10d 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -189,7 +189,7 @@ static int sco_connect(struct sock *sk) if (!hdev) return -EHOSTUNREACH; - hci_dev_lock_bh(hdev); + hci_dev_lock(hdev); if (lmp_esco_capable(hdev) && !disable_esco) type = ESCO_LINK; @@ -225,7 +225,7 @@ static int sco_connect(struct sock *sk) } done: - hci_dev_unlock_bh(hdev); + hci_dev_unlock(hdev); hci_dev_put(hdev); return err; } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 03/22] Bluetooth: Use delayed_work for connection timeout 2011-12-17 21:29 ` [RFC 02/22] Bluetooth: Replace spin_lock by mutex in hci_dev Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 04/22] Bluetooth: Use delayed work for advertisiment cache timeout Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Bluetooth rx task runs now in a workqueue, so it a good approach run any timer that share locking with process context code also in a workqueue. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 8 +++++--- net/bluetooth/hci_conn.c | 9 +++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index e7dbe59..d915908 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -297,7 +297,7 @@ struct hci_conn { struct sk_buff_head data_q; struct list_head chan_list; - struct timer_list disc_timer; + struct delayed_work disc_work; struct timer_list idle_timer; struct timer_list auto_accept_timer; @@ -517,7 +517,7 @@ void hci_conn_put_device(struct hci_conn *conn); static inline void hci_conn_hold(struct hci_conn *conn) { atomic_inc(&conn->refcnt); - del_timer(&conn->disc_timer); + cancel_delayed_work_sync(&conn->disc_work); } static inline void hci_conn_put(struct hci_conn *conn) @@ -536,7 +536,9 @@ static inline void hci_conn_put(struct hci_conn *conn) } else { timeo = msecs_to_jiffies(10); } - mod_timer(&conn->disc_timer, jiffies + timeo); + cancel_delayed_work_sync(&conn->disc_work); + queue_delayed_work(conn->hdev->workqueue, + &conn->disc_work, jiffies + timeo); } } diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index d45783d..7d88a61 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -275,9 +275,10 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status) } } -static void hci_conn_timeout(unsigned long arg) +static void hci_conn_timeout(struct work_struct *work) { - struct hci_conn *conn = (void *) arg; + struct hci_conn *conn = container_of(work, struct hci_conn, + disc_work.work); struct hci_dev *hdev = conn->hdev; __u8 reason; @@ -412,7 +413,7 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst) INIT_LIST_HEAD(&conn->chan_list);; - setup_timer(&conn->disc_timer, hci_conn_timeout, (unsigned long)conn); + INIT_DELAYED_WORK(&conn->disc_work, hci_conn_timeout); setup_timer(&conn->idle_timer, hci_conn_idle, (unsigned long)conn); setup_timer(&conn->auto_accept_timer, hci_conn_auto_accept, (unsigned long) conn); @@ -444,7 +445,7 @@ int hci_conn_del(struct hci_conn *conn) del_timer(&conn->idle_timer); - del_timer(&conn->disc_timer); + cancel_delayed_work_sync(&conn->disc_work); del_timer(&conn->auto_accept_timer); -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 04/22] Bluetooth: Use delayed work for advertisiment cache timeout 2011-12-17 21:29 ` [RFC 03/22] Bluetooth: Use delayed_work for connection timeout Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 05/22] Bluetooth: hci_conn_auto_accept() doesn't need locking Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> As HCI rx path is now done in process context it makes sense to do all the timer in process context as well. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_core.c | 10 +++++----- net/bluetooth/hci_event.c | 6 ++++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index d915908..14b200b 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -226,7 +226,7 @@ struct hci_dev { struct list_head remote_oob_data; struct list_head adv_entries; - struct timer_list adv_timer; + struct delayed_work adv_work; struct hci_dev_stats stat; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 279074c..1605fe7 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1341,9 +1341,10 @@ int hci_blacklist_del(struct hci_dev *hdev, bdaddr_t *bdaddr) return mgmt_device_unblocked(hdev, bdaddr); } -static void hci_clear_adv_cache(unsigned long arg) +static void hci_clear_adv_cache(struct work_struct *work) { - struct hci_dev *hdev = (void *) arg; + struct hci_dev *hdev = container_of(work, struct hci_dev, + adv_work.work); hci_dev_lock(hdev); @@ -1489,9 +1490,8 @@ int hci_register_dev(struct hci_dev *hdev) INIT_LIST_HEAD(&hdev->remote_oob_data); INIT_LIST_HEAD(&hdev->adv_entries); - setup_timer(&hdev->adv_timer, hci_clear_adv_cache, - (unsigned long) hdev); + INIT_DELAYED_WORK(&hdev->adv_work, hci_clear_adv_cache); INIT_WORK(&hdev->power_on, hci_power_on); INIT_DELAYED_WORK(&hdev->power_off, hci_power_off); @@ -1577,7 +1577,7 @@ void hci_unregister_dev(struct hci_dev *hdev) hci_del_sysfs(hdev); - del_timer(&hdev->adv_timer); + cancel_delayed_work_sync(&hdev->adv_work); destroy_workqueue(hdev->workqueue); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 35cb56e..0a9501f 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1017,7 +1017,7 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev, if (cp->enable == 0x01) { set_bit(HCI_LE_SCAN, &hdev->dev_flags); - del_timer(&hdev->adv_timer); + cancel_delayed_work_sync(&hdev->adv_work); hci_dev_lock(hdev); hci_adv_entries_clear(hdev); @@ -1025,7 +1025,9 @@ static void hci_cc_le_set_scan_enable(struct hci_dev *hdev, } else if (cp->enable == 0x00) { clear_bit(HCI_LE_SCAN, &hdev->dev_flags); - mod_timer(&hdev->adv_timer, jiffies + ADV_CLEAR_TIMEOUT); + cancel_delayed_work_sync(&hdev->adv_work); + queue_delayed_work(hdev->workqueue, &hdev->adv_work, + jiffies + ADV_CLEAR_TIMEOUT); } } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 05/22] Bluetooth: hci_conn_auto_accept() doesn't need locking 2011-12-17 21:29 ` [RFC 04/22] Bluetooth: Use delayed work for advertisiment cache timeout Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> It doesn't really touch any sensitive information about hdev. So no need to lock here. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/hci_conn.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 7d88a61..e6d8a22 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -362,12 +362,8 @@ static void hci_conn_auto_accept(unsigned long arg) struct hci_conn *conn = (void *) arg; struct hci_dev *hdev = conn->hdev; - hci_dev_lock(hdev); - hci_send_cmd(hdev, HCI_OP_USER_CONFIRM_REPLY, sizeof(conn->dst), &conn->dst); - - hci_dev_unlock(hdev); } struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst) -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue 2011-12-17 21:29 ` [RFC 05/22] Bluetooth: hci_conn_auto_accept() doesn't need locking Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Gustavo F. Padovan 2011-12-19 11:05 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Andrei Emeltchenko 0 siblings, 2 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> L2CAP timers also need to run in process context. As the works in l2cap are small we are using the system worqueue. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/l2cap.h | 17 +++++----- net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++----------------------- 2 files changed, 40 insertions(+), 47 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 30719eb..03be911 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -482,10 +482,11 @@ struct l2cap_chan { __u32 remote_acc_lat; __u32 remote_flush_to; - struct timer_list chan_timer; - struct timer_list retrans_timer; - struct timer_list monitor_timer; - struct timer_list ack_timer; + struct delayed_work chan_timer; + struct delayed_work retrans_timer; + struct delayed_work monitor_timer; + struct delayed_work ack_timer; + struct sk_buff *tx_send_head; struct sk_buff_head tx_q; struct sk_buff_head srej_q; @@ -595,16 +596,16 @@ enum { }; #define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t)) -#define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer) +#define __clear_chan_timer(c) l2cap_clear_timer(&c->chan_timer) #define __set_retrans_timer(c) l2cap_set_timer(c, &c->retrans_timer, \ L2CAP_DEFAULT_RETRANS_TO); -#define __clear_retrans_timer(c) l2cap_clear_timer(c, &c->retrans_timer) +#define __clear_retrans_timer(c) l2cap_clear_timer(&c->retrans_timer) #define __set_monitor_timer(c) l2cap_set_timer(c, &c->monitor_timer, \ L2CAP_DEFAULT_MONITOR_TO); -#define __clear_monitor_timer(c) l2cap_clear_timer(c, &c->monitor_timer) +#define __clear_monitor_timer(c) l2cap_clear_timer(&c->monitor_timer) #define __set_ack_timer(c) l2cap_set_timer(c, &chan->ack_timer, \ L2CAP_DEFAULT_ACK_TO); -#define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer) +#define __clear_ack_timer(c) l2cap_clear_timer(&c->ack_timer) static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2) { diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 0369a9b..89cda6d 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -213,20 +213,18 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn) return 0; } -static void l2cap_set_timer(struct l2cap_chan *chan, struct timer_list *timer, long timeout) +static void l2cap_set_timer(struct l2cap_chan *chan, struct delayed_work *work, long timeout) { BT_DBG("chan %p state %d timeout %ld", chan, chan->state, timeout); - if (!mod_timer(timer, jiffies + msecs_to_jiffies(timeout))) - chan_hold(chan); + cancel_delayed_work_sync(work); + + schedule_delayed_work(work, timeout); } -static void l2cap_clear_timer(struct l2cap_chan *chan, struct timer_list *timer) +static void l2cap_clear_timer(struct delayed_work *work) { - BT_DBG("chan %p state %d", chan, chan->state); - - if (timer_pending(timer) && del_timer(timer)) - chan_put(chan); + cancel_delayed_work_sync(work); } static char *state_to_string(int state) @@ -264,23 +262,16 @@ static void l2cap_state_change(struct l2cap_chan *chan, int state) chan->ops->state_change(chan->data, state); } -static void l2cap_chan_timeout(unsigned long arg) +static void l2cap_chan_timeout(struct work_struct *work) { - struct l2cap_chan *chan = (struct l2cap_chan *) arg; + struct l2cap_chan *chan = container_of(work, struct l2cap_chan, + chan_timer.work); struct sock *sk = chan->sk; int reason; BT_DBG("chan %p state %d", chan, chan->state); - bh_lock_sock(sk); - - if (sock_owned_by_user(sk)) { - /* sk is owned by user. Try again later */ - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); - bh_unlock_sock(sk); - chan_put(chan); - return; - } + lock_sock(sk); if (chan->state == BT_CONNECTED || chan->state == BT_CONFIG) reason = ECONNREFUSED; @@ -292,7 +283,7 @@ static void l2cap_chan_timeout(unsigned long arg) l2cap_chan_close(chan, reason); - bh_unlock_sock(sk); + release_sock(sk); chan->ops->close(chan->data); chan_put(chan); @@ -312,7 +303,7 @@ struct l2cap_chan *l2cap_chan_create(struct sock *sk) list_add(&chan->global_l, &chan_list); write_unlock_bh(&chan_list_lock); - setup_timer(&chan->chan_timer, l2cap_chan_timeout, (unsigned long) chan); + INIT_DELAYED_WORK(&chan->chan_timer, l2cap_chan_timeout); chan->state = BT_OPEN; @@ -1251,17 +1242,18 @@ int __l2cap_wait_ack(struct sock *sk) return err; } -static void l2cap_monitor_timeout(unsigned long arg) +static void l2cap_monitor_timeout(struct work_struct *work) { - struct l2cap_chan *chan = (void *) arg; + struct l2cap_chan *chan = container_of(work, struct l2cap_chan, + monitor_timer.work); struct sock *sk = chan->sk; BT_DBG("chan %p", chan); - bh_lock_sock(sk); + lock_sock(sk); if (chan->retry_count >= chan->remote_max_tx) { l2cap_send_disconn_req(chan->conn, chan, ECONNABORTED); - bh_unlock_sock(sk); + release_sock(sk); return; } @@ -1269,24 +1261,25 @@ static void l2cap_monitor_timeout(unsigned long arg) __set_monitor_timer(chan); l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_POLL); - bh_unlock_sock(sk); + release_sock(sk); } -static void l2cap_retrans_timeout(unsigned long arg) +static void l2cap_retrans_timeout(struct work_struct *work) { - struct l2cap_chan *chan = (void *) arg; + struct l2cap_chan *chan = container_of(work, struct l2cap_chan, + retrans_timer.work); struct sock *sk = chan->sk; BT_DBG("chan %p", chan); - bh_lock_sock(sk); + lock_sock(sk); chan->retry_count = 1; __set_monitor_timer(chan); set_bit(CONN_WAIT_F, &chan->conn_state); l2cap_send_rr_or_rnr(chan, L2CAP_CTRL_POLL); - bh_unlock_sock(sk); + release_sock(sk); } static void l2cap_drop_acked_frames(struct l2cap_chan *chan) @@ -1955,13 +1948,14 @@ static void l2cap_add_opt_efs(void **ptr, struct l2cap_chan *chan) (unsigned long) &efs); } -static void l2cap_ack_timeout(unsigned long arg) +static void l2cap_ack_timeout(struct work_struct *work) { - struct l2cap_chan *chan = (void *) arg; + struct l2cap_chan *chan = container_of(work, struct l2cap_chan, + ack_timer.work); - bh_lock_sock(chan->sk); + lock_sock(chan->sk); l2cap_send_ack(chan); - bh_unlock_sock(chan->sk); + release_sock(chan->sk); } static inline void l2cap_ertm_init(struct l2cap_chan *chan) @@ -1974,11 +1968,9 @@ static inline void l2cap_ertm_init(struct l2cap_chan *chan) chan->num_acked = 0; chan->frames_sent = 0; - setup_timer(&chan->retrans_timer, l2cap_retrans_timeout, - (unsigned long) chan); - setup_timer(&chan->monitor_timer, l2cap_monitor_timeout, - (unsigned long) chan); - setup_timer(&chan->ack_timer, l2cap_ack_timeout, (unsigned long) chan); + INIT_DELAYED_WORK(&chan->retrans_timer, l2cap_retrans_timeout); + INIT_DELAYED_WORK(&chan->monitor_timer, l2cap_monitor_timeout); + INIT_DELAYED_WORK(&chan->ack_timer, l2cap_ack_timeout); skb_queue_head_init(&chan->srej_q); -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore 2011-12-17 21:29 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 08/22] Bluetooth: Remove sk_backlog usage from L2CAP Gustavo F. Padovan 2011-12-19 9:53 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Andrei Emeltchenko 2011-12-19 11:05 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Andrei Emeltchenko 1 sibling, 2 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> We now run everything in process context, so the mutex lock is the best option. But in some places we still need the bh_lock_sock() Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/l2cap_core.c | 66 +++++++++++++------------------------------ 1 files changed, 20 insertions(+), 46 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 89cda6d..ed67ac0 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -118,7 +118,7 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci read_lock(&conn->chan_lock); c = __l2cap_get_chan_by_scid(conn, cid); if (c) - bh_lock_sock(c->sk); + lock_sock(c->sk); read_unlock(&conn->chan_lock); return c; } @@ -141,7 +141,7 @@ static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn read_lock(&conn->chan_lock); c = __l2cap_get_chan_by_ident(conn, ident); if (c) - bh_lock_sock(c->sk); + lock_sock(c->sk); read_unlock(&conn->chan_lock); return c; } @@ -889,7 +889,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) parent = pchan->sk; - bh_lock_sock(parent); + lock_sock(parent); /* Check for backlog size */ if (sk_acceptq_is_full(parent)) { @@ -922,7 +922,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) write_unlock_bh(&conn->chan_lock); clean: - bh_unlock_sock(parent); + release_sock(parent); } static void l2cap_chan_ready(struct sock *sk) @@ -1024,9 +1024,9 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err) /* Kill channels */ list_for_each_entry_safe(chan, l, &conn->chan_l, list) { sk = chan->sk; - bh_lock_sock(sk); + lock_sock(sk); l2cap_chan_del(chan, err); - bh_unlock_sock(sk); + release_sock(sk); chan->ops->close(chan->data); } @@ -2568,7 +2568,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd parent = pchan->sk; - bh_lock_sock(parent); + lock_sock(parent); /* Check if the ACL is secure enough (if not SDP) */ if (psm != cpu_to_le16(0x0001) && @@ -2645,7 +2645,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd write_unlock_bh(&conn->chan_lock); response: - bh_unlock_sock(parent); + release_sock(parent); sendresp: rsp.scid = cpu_to_le16(scid); @@ -2727,19 +2727,11 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd break; default: - /* don't delete l2cap channel if sk is owned by user */ - if (sock_owned_by_user(sk)) { - l2cap_state_change(chan, BT_DISCONN); - __clear_chan_timer(chan); - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); - break; - } - l2cap_chan_del(chan, ECONNREFUSED); break; } - bh_unlock_sock(sk); + release_sock(sk); return 0; } @@ -2861,7 +2853,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr } unlock: - bh_unlock_sock(sk); + release_sock(sk); return 0; } @@ -2968,7 +2960,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr } done: - bh_unlock_sock(sk); + release_sock(sk); return 0; } @@ -2997,17 +2989,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd sk->sk_shutdown = SHUTDOWN_MASK; - /* don't delete l2cap channel if sk is owned by user */ - if (sock_owned_by_user(sk)) { - l2cap_state_change(chan, BT_DISCONN); - __clear_chan_timer(chan); - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); - bh_unlock_sock(sk); - return 0; - } - l2cap_chan_del(chan, ECONNRESET); - bh_unlock_sock(sk); + release_sock(sk); chan->ops->close(chan->data); return 0; @@ -3031,17 +3014,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd sk = chan->sk; - /* don't delete l2cap channel if sk is owned by user */ - if (sock_owned_by_user(sk)) { - l2cap_state_change(chan, BT_DISCONN); - __clear_chan_timer(chan); - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); - bh_unlock_sock(sk); - return 0; - } - l2cap_chan_del(chan, 0); - bh_unlock_sock(sk); + release_sock(sk); chan->ops->close(chan->data); return 0; @@ -4284,7 +4258,7 @@ drop: done: if (sk) - bh_unlock_sock(sk); + release_sock(sk); return 0; } @@ -4300,7 +4274,7 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str sk = chan->sk; - bh_lock_sock(sk); + lock_sock(sk); BT_DBG("sk %p, len %d", sk, skb->len); @@ -4318,7 +4292,7 @@ drop: done: if (sk) - bh_unlock_sock(sk); + release_sock(sk); return 0; } @@ -4333,7 +4307,7 @@ static inline int l2cap_att_channel(struct l2cap_conn *conn, __le16 cid, struct sk = chan->sk; - bh_lock_sock(sk); + lock_sock(sk); BT_DBG("sk %p, len %d", sk, skb->len); @@ -4351,7 +4325,7 @@ drop: done: if (sk) - bh_unlock_sock(sk); + release_sock(sk); return 0; } @@ -4656,11 +4630,11 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl BT_ERR("Frame exceeding recv MTU (len %d, " "MTU %d)", len, chan->imtu); - bh_unlock_sock(sk); + release_sock(sk); l2cap_conn_unreliable(conn, ECOMM); goto drop; } - bh_unlock_sock(sk); + release_sock(sk); } /* Allocate skb for the complete frame (with header) */ -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 08/22] Bluetooth: Remove sk_backlog usage from L2CAP 2011-12-17 21:29 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 09/22] Bluetooth: move hci_task_lock to mutex Gustavo F. Padovan 2011-12-19 9:53 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Andrei Emeltchenko 1 sibling, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> We run everything in the same lock now. The backlog queue is useless now Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/l2cap_core.c | 12 +----------- 1 files changed, 1 insertions(+), 11 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index ed67ac0..31c94fd 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1960,8 +1960,6 @@ static void l2cap_ack_timeout(struct work_struct *work) static inline void l2cap_ertm_init(struct l2cap_chan *chan) { - struct sock *sk = chan->sk; - chan->expected_ack_seq = 0; chan->unacked_frames = 0; chan->buffer_seq = 0; @@ -1975,9 +1973,6 @@ static inline void l2cap_ertm_init(struct l2cap_chan *chan) skb_queue_head_init(&chan->srej_q); INIT_LIST_HEAD(&chan->srej_l); - - - sk->sk_backlog_rcv = l2cap_ertm_data_rcv; } static inline __u8 l2cap_select_mode(__u8 mode, __u16 remote_feat_mask) @@ -4203,12 +4198,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk break; case L2CAP_MODE_ERTM: - if (!sock_owned_by_user(sk)) { - l2cap_ertm_data_rcv(sk, skb); - } else { - if (sk_add_backlog(sk, skb)) - goto drop; - } + l2cap_ertm_data_rcv(sk, skb); goto done; -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 09/22] Bluetooth: move hci_task_lock to mutex 2011-12-17 21:29 ` [RFC 08/22] Bluetooth: Remove sk_backlog usage from L2CAP Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 10/22] Bluetooth: convert chan_lock " Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Now we can sleep in any path inside Bluetooth core, so mutex can make sense here. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/hci_core.c | 18 +++++++++--------- 1 files changed, 9 insertions(+), 9 deletions(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 1605fe7..1f0198b 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -60,7 +60,7 @@ static void hci_rx_work(struct work_struct *work); static void hci_cmd_task(unsigned long arg); static void hci_tx_task(unsigned long arg); -static DEFINE_RWLOCK(hci_task_lock); +static DEFINE_MUTEX(hci_task_lock); /* HCI device list */ LIST_HEAD(hci_dev_list); @@ -1809,14 +1809,14 @@ int hci_register_proto(struct hci_proto *hp) if (hp->id >= HCI_MAX_PROTO) return -EINVAL; - write_lock_bh(&hci_task_lock); + mutex_lock(&hci_task_lock); if (!hci_proto[hp->id]) hci_proto[hp->id] = hp; else err = -EEXIST; - write_unlock_bh(&hci_task_lock); + mutex_unlock(&hci_task_lock); return err; } @@ -1831,14 +1831,14 @@ int hci_unregister_proto(struct hci_proto *hp) if (hp->id >= HCI_MAX_PROTO) return -EINVAL; - write_lock_bh(&hci_task_lock); + mutex_lock(&hci_task_lock); if (hci_proto[hp->id]) hci_proto[hp->id] = NULL; else err = -ENOENT; - write_unlock_bh(&hci_task_lock); + mutex_unlock(&hci_task_lock); return err; } @@ -2387,7 +2387,7 @@ static void hci_tx_task(unsigned long arg) struct hci_dev *hdev = (struct hci_dev *) arg; struct sk_buff *skb; - read_lock(&hci_task_lock); + mutex_lock(&hci_task_lock); BT_DBG("%s acl %d sco %d le %d", hdev->name, hdev->acl_cnt, hdev->sco_cnt, hdev->le_cnt); @@ -2406,7 +2406,7 @@ static void hci_tx_task(unsigned long arg) while ((skb = skb_dequeue(&hdev->raw_q))) hci_send_frame(skb); - read_unlock(&hci_task_lock); + mutex_unlock(&hci_task_lock); } /* ----- HCI RX task (incoming data processing) ----- */ @@ -2494,7 +2494,7 @@ static void hci_rx_work(struct work_struct *work) BT_DBG("%s", hdev->name); - read_lock(&hci_task_lock); + mutex_lock(&hci_task_lock); while ((skb = skb_dequeue(&hdev->rx_q))) { if (atomic_read(&hdev->promisc)) { @@ -2540,7 +2540,7 @@ static void hci_rx_work(struct work_struct *work) } } - read_unlock(&hci_task_lock); + mutex_unlock(&hci_task_lock); } static void hci_cmd_task(unsigned long arg) -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 10/22] Bluetooth: convert chan_lock to mutex 2011-12-17 21:29 ` [RFC 09/22] Bluetooth: move hci_task_lock to mutex Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Gustavo F. Padovan 2011-12-19 9:58 ` [RFC 10/22] Bluetooth: convert chan_lock to mutex Andrei Emeltchenko 0 siblings, 2 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> spin lock doesn't fit ok anymore on the new code based on workqueues. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/l2cap.h | 2 +- net/bluetooth/l2cap_core.c | 52 ++++++++++++++++++++-------------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 03be911..a175091 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -536,7 +536,7 @@ struct l2cap_conn { struct smp_chan *smp_chan; struct list_head chan_l; - rwlock_t chan_lock; + struct mutex chan_lock; }; #define L2CAP_INFO_CL_MTU_REQ_SENT 0x01 diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 31c94fd..5c5948f 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -115,11 +115,11 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci { struct l2cap_chan *c; - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); c = __l2cap_get_chan_by_scid(conn, cid); if (c) lock_sock(c->sk); - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); return c; } @@ -138,11 +138,11 @@ static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn { struct l2cap_chan *c; - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); c = __l2cap_get_chan_by_ident(conn, ident); if (c) lock_sock(c->sk); - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); return c; } @@ -381,9 +381,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err) if (conn) { /* Delete from channel list */ - write_lock_bh(&conn->chan_lock); + mutex_lock(&conn->chan_lock); list_del(&chan->list); - write_unlock_bh(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); chan_put(chan); chan->conn = NULL; @@ -754,7 +754,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) BT_DBG("conn %p", conn); - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) { struct sock *sk = chan->sk; @@ -780,9 +780,9 @@ static void l2cap_conn_start(struct l2cap_conn *conn) &chan->conf_state)) { /* l2cap_chan_close() calls list_del(chan) * so release the lock */ - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); l2cap_chan_close(chan, ECONNRESET); - read_lock(&conn->chan_lock); + utex_lock(&conn->chan_lock); bh_unlock_sock(sk); continue; } @@ -838,7 +838,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) bh_unlock_sock(sk); } - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); } /* Find socket with cid and source bdaddr. @@ -903,7 +903,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) sk = chan->sk; - write_lock_bh(&conn->chan_lock); + mutex_lock(&conn->chan_lock); hci_conn_hold(conn->hcon); @@ -919,7 +919,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) l2cap_state_change(chan, BT_CONNECTED); parent->sk_data_ready(parent, 0); - write_unlock_bh(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); clean: release_sock(parent); @@ -954,7 +954,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) if (conn->hcon->out && conn->hcon->type == LE_LINK) smp_conn_security(conn, conn->hcon->pending_sec_level); - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); list_for_each_entry(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; @@ -976,7 +976,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) bh_unlock_sock(sk); } - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); } /* Notify sockets that we cannot guaranty reliability anymore */ @@ -986,7 +986,7 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) BT_DBG("conn %p", conn); - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); list_for_each_entry(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; @@ -995,7 +995,7 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) sk->sk_err = err; } - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); } static void l2cap_info_timeout(unsigned long arg) @@ -1086,7 +1086,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) conn->feat_mask = 0; spin_lock_init(&conn->lock); - rwlock_init(&conn->chan_lock); + mutex_init(&conn->chan_lock); INIT_LIST_HEAD(&conn->chan_l); @@ -1104,9 +1104,9 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) static inline void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) { - write_lock_bh(&conn->chan_lock); + mutex_lock(&conn->chan_lock); __l2cap_chan_add(conn, chan); - write_unlock_bh(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); } /* ---- Socket interface ---- */ @@ -1771,7 +1771,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) BT_DBG("conn %p", conn); - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); list_for_each_entry(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; if (chan->chan_type != L2CAP_CHAN_RAW) @@ -1787,7 +1787,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) if (chan->ops->recv(chan->data, nskb)) kfree_skb(nskb); } - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); } /* ---- L2CAP signalling commands ---- */ @@ -2587,11 +2587,11 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd sk = chan->sk; - write_lock_bh(&conn->chan_lock); + mutex_lock(&conn->chan_lock); /* Check if we already have channel with that dcid */ if (__l2cap_get_chan_by_dcid(conn, scid)) { - write_unlock_bh(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); sock_set_flag(sk, SOCK_ZAPPED); chan->ops->close(chan->data); goto response; @@ -2637,7 +2637,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd status = L2CAP_CS_NO_INFO; } - write_unlock_bh(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); response: release_sock(parent); @@ -4474,7 +4474,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) del_timer(&conn->security_timer); } - read_lock(&conn->chan_lock); + mutex_lock(&conn->chan_lock); list_for_each_entry(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; @@ -4554,7 +4554,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) bh_unlock_sock(sk); } - read_unlock(&conn->chan_lock); + mutex_unlock(&conn->chan_lock); return 0; } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list 2011-12-17 21:29 ` [RFC 10/22] Bluetooth: convert chan_lock " Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 12/22] Bluetooth: convert conn hash to RCU Gustavo F. Padovan 2012-01-26 13:20 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Andrei Emeltchenko 2011-12-19 9:58 ` [RFC 10/22] Bluetooth: convert chan_lock to mutex Andrei Emeltchenko 1 sibling, 2 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Instead of using tasklet_disable() to prevent acess to the channel use, we can use RCU and improve the performance of our code. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/hci_conn.c | 14 ++++++-------- net/bluetooth/hci_core.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index e6d8a22..b044676 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -960,9 +960,7 @@ struct hci_chan *hci_chan_create(struct hci_conn *conn) chan->conn = conn; skb_queue_head_init(&chan->data_q); - tasklet_disable(&hdev->tx_task); - list_add(&conn->chan_list, &chan->list); - tasklet_enable(&hdev->tx_task); + list_add_rcu(&chan->list, &conn->chan_list); return chan; } @@ -974,9 +972,9 @@ int hci_chan_del(struct hci_chan *chan) BT_DBG("%s conn %p chan %p", hdev->name, conn, chan); - tasklet_disable(&hdev->tx_task); - list_del(&chan->list); - tasklet_enable(&hdev->tx_task); + list_del_rcu(&chan->list); + + synchronize_rcu(); skb_queue_purge(&chan->data_q); kfree(chan); @@ -986,10 +984,10 @@ int hci_chan_del(struct hci_chan *chan) void hci_chan_list_flush(struct hci_conn *conn) { - struct hci_chan *chan, *tmp; + struct hci_chan *chan; BT_DBG("conn %p", conn); - list_for_each_entry_safe(chan, tmp, &conn->chan_list, list) + list_for_each_entry_rcu(chan, &conn->chan_list, list) hci_chan_del(chan); } diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 1f0198b..f016a40 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2136,7 +2136,9 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type, conn_num++; - list_for_each_entry(tmp, &conn->chan_list, list) { + rcu_read_lock(); + + list_for_each_entry_rcu(tmp, &conn->chan_list, list) { struct sk_buff *skb; if (skb_queue_empty(&tmp->data_q)) @@ -2160,6 +2162,8 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type, } } + rcu_read_unlock(); + if (hci_conn_num(hdev, type) == conn_num) break; } @@ -2208,7 +2212,9 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type) num++; - list_for_each_entry(chan, &conn->chan_list, list) { + rcu_read_lock(); + + list_for_each_entry_rcu(chan, &conn->chan_list, list) { struct sk_buff *skb; if (chan->sent) { @@ -2229,6 +2235,8 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type) skb->priority); } + rcu_read_unlock(); + if (hci_conn_num(hdev, type) == num) break; } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 12/22] Bluetooth: convert conn hash to RCU 2011-12-17 21:29 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 13/22] Bluetooth: Don't disable tasklets to call hdev->notify() Gustavo F. Padovan 2012-01-26 13:20 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Andrei Emeltchenko 1 sibling, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Handling hci_conn_hash with RCU make us avoid some locking and disable tasklets. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 45 ++++++++++++++++++++++++++----------- net/bluetooth/hci_conn.c | 19 +++++++-------- net/bluetooth/hci_core.c | 34 ++++++++++++++++++---------- 3 files changed, 62 insertions(+), 36 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 14b200b..e832433 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -392,7 +392,7 @@ static inline void hci_conn_hash_init(struct hci_dev *hdev) static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c) { struct hci_conn_hash *h = &hdev->conn_hash; - list_add(&c->list, &h->list); + list_add_rcu(&c->list, &h->list); switch (c->type) { case ACL_LINK: h->acl_num++; @@ -410,7 +410,10 @@ static inline void hci_conn_hash_add(struct hci_dev *hdev, struct hci_conn *c) static inline void hci_conn_hash_del(struct hci_dev *hdev, struct hci_conn *c) { struct hci_conn_hash *h = &hdev->conn_hash; - list_del(&c->list); + + list_del_rcu(&c->list); + synchronize_rcu(); + switch (c->type) { case ACL_LINK: h->acl_num--; @@ -445,14 +448,18 @@ static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev, __u16 handle) { struct hci_conn_hash *h = &hdev->conn_hash; - struct list_head *p; struct hci_conn *c; - list_for_each(p, &h->list) { - c = list_entry(p, struct hci_conn, list); - if (c->handle == handle) + rcu_read_lock(); + + list_for_each_entry_rcu(c, &h->list, list) { + if (c->handle == handle) { + rcu_read_unlock(); return c; + } } + rcu_read_unlock(); + return NULL; } @@ -460,14 +467,19 @@ static inline struct hci_conn *hci_conn_hash_lookup_ba(struct hci_dev *hdev, __u8 type, bdaddr_t *ba) { struct hci_conn_hash *h = &hdev->conn_hash; - struct list_head *p; struct hci_conn *c; - list_for_each(p, &h->list) { - c = list_entry(p, struct hci_conn, list); - if (c->type == type && !bacmp(&c->dst, ba)) + rcu_read_lock(); + + list_for_each_entry_rcu(c, &h->list, list) { + if (c->type == type && !bacmp(&c->dst, ba)) { + rcu_read_unlock(); return c; + } } + + rcu_read_unlock(); + return NULL; } @@ -475,14 +487,19 @@ static inline struct hci_conn *hci_conn_hash_lookup_state(struct hci_dev *hdev, __u8 type, __u16 state) { struct hci_conn_hash *h = &hdev->conn_hash; - struct list_head *p; struct hci_conn *c; - list_for_each(p, &h->list) { - c = list_entry(p, struct hci_conn, list); - if (c->type == type && c->state == state) + rcu_read_lock(); + + list_for_each_entry_rcu(c, &h->list, list) { + if (c->type == type && c->state == state) { + rcu_read_unlock(); return c; + } } + + rcu_read_unlock(); + return NULL; } diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index b044676..5e9e193 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -418,18 +418,17 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst) hci_dev_hold(hdev); - tasklet_disable(&hdev->tx_task); - hci_conn_hash_add(hdev, conn); - if (hdev->notify) + if (hdev->notify) { + tasklet_disable(&hdev->tx_task); hdev->notify(hdev, HCI_NOTIFY_CONN_ADD); + tasklet_enable(&hdev->tx_task); + } atomic_set(&conn->devref, 0); hci_conn_init_sysfs(conn); - tasklet_enable(&hdev->tx_task); - return conn; } @@ -465,15 +464,15 @@ int hci_conn_del(struct hci_conn *conn) } } - tasklet_disable(&hdev->tx_task); hci_chan_list_flush(conn); hci_conn_hash_del(hdev, conn); - if (hdev->notify) + if (hdev->notify) { + tasklet_disable(&hdev->tx_task); hdev->notify(hdev, HCI_NOTIFY_CONN_DEL); - - tasklet_enable(&hdev->tx_task); + tasklet_enable(&hdev->tx_task); + } skb_queue_purge(&conn->data_q); @@ -808,7 +807,7 @@ void hci_conn_hash_flush(struct hci_dev *hdev) BT_DBG("hdev %s", hdev->name); - list_for_each_entry(c, &h->list, list) { + list_for_each_entry_rcu(c, &h->list, list) { c->state = BT_CLOSED; hci_proto_disconn_cfm(c, HCI_ERROR_LOCAL_HOST_TERM); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index f016a40..7654777 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -2051,7 +2051,10 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int /* We don't have to lock device here. Connections are always * added and removed with TX task disabled. */ - list_for_each_entry(c, &h->list, list) { + + rcu_read_lock(); + + list_for_each_entry_rcu(c, &h->list, list) { if (c->type != type || skb_queue_empty(&c->data_q)) continue; @@ -2069,6 +2072,8 @@ static inline struct hci_conn *hci_low_sent(struct hci_dev *hdev, __u8 type, int break; } + rcu_read_unlock(); + if (conn) { int cnt, q; @@ -2104,14 +2109,18 @@ static inline void hci_link_tx_to(struct hci_dev *hdev, __u8 type) BT_ERR("%s link tx timeout", hdev->name); + rcu_read_lock(); + /* Kill stalled connections */ - list_for_each_entry(c, &h->list, list) { + list_for_each_entry_rcu(c, &h->list, list) { if (c->type == type && c->sent) { BT_ERR("%s killing stalled connection %s", hdev->name, batostr(&c->dst)); hci_acl_disconn(c, 0x13); } } + + rcu_read_unlock(); } static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type, @@ -2125,7 +2134,9 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type, BT_DBG("%s", hdev->name); - list_for_each_entry(conn, &h->list, list) { + rcu_read_lock(); + + list_for_each_entry_rcu(conn, &h->list, list) { struct hci_chan *tmp; if (conn->type != type) @@ -2136,8 +2147,6 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type, conn_num++; - rcu_read_lock(); - list_for_each_entry_rcu(tmp, &conn->chan_list, list) { struct sk_buff *skb; @@ -2162,12 +2171,12 @@ static inline struct hci_chan *hci_chan_sent(struct hci_dev *hdev, __u8 type, } } - rcu_read_unlock(); - if (hci_conn_num(hdev, type) == conn_num) break; } + rcu_read_unlock(); + if (!chan) return NULL; @@ -2201,7 +2210,9 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type) BT_DBG("%s", hdev->name); - list_for_each_entry(conn, &h->list, list) { + rcu_read_lock(); + + list_for_each_entry_rcu(conn, &h->list, list) { struct hci_chan *chan; if (conn->type != type) @@ -2212,8 +2223,6 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type) num++; - rcu_read_lock(); - list_for_each_entry_rcu(chan, &conn->chan_list, list) { struct sk_buff *skb; @@ -2235,11 +2244,12 @@ static void hci_prio_recalculate(struct hci_dev *hdev, __u8 type) skb->priority); } - rcu_read_unlock(); - if (hci_conn_num(hdev, type) == num) break; } + + rcu_read_unlock(); + } static inline void hci_sched_acl(struct hci_dev *hdev) -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 13/22] Bluetooth: Don't disable tasklets to call hdev->notify() 2011-12-17 21:29 ` [RFC 12/22] Bluetooth: convert conn hash to RCU Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 14/22] Bluetooth: Move command task to workqueue Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> It's pointless, we aren't protecting anything since btusb_notify() schedules a work to run, then all it operation happens without protection. If protection is really needed here, we will fix it further. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/hci_conn.c | 10 ++-------- net/bluetooth/hci_event.c | 10 ++-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 5e9e193..385cccb 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -419,11 +419,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst) hci_dev_hold(hdev); hci_conn_hash_add(hdev, conn); - if (hdev->notify) { - tasklet_disable(&hdev->tx_task); + if (hdev->notify) hdev->notify(hdev, HCI_NOTIFY_CONN_ADD); - tasklet_enable(&hdev->tx_task); - } atomic_set(&conn->devref, 0); @@ -468,11 +465,8 @@ int hci_conn_del(struct hci_conn *conn) hci_chan_list_flush(conn); hci_conn_hash_del(hdev, conn); - if (hdev->notify) { - tasklet_disable(&hdev->tx_task); + if (hdev->notify) hdev->notify(hdev, HCI_NOTIFY_CONN_DEL); - tasklet_enable(&hdev->tx_task); - } skb_queue_purge(&conn->data_q); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 0a9501f..93ecb2d 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -378,11 +378,8 @@ static void hci_cc_read_voice_setting(struct hci_dev *hdev, struct sk_buff *skb) BT_DBG("%s voice setting 0x%04x", hdev->name, setting); - if (hdev->notify) { - tasklet_disable(&hdev->tx_task); + if (hdev->notify) hdev->notify(hdev, HCI_NOTIFY_VOICE_SETTING); - tasklet_enable(&hdev->tx_task); - } } static void hci_cc_write_voice_setting(struct hci_dev *hdev, struct sk_buff *skb) @@ -409,11 +406,8 @@ static void hci_cc_write_voice_setting(struct hci_dev *hdev, struct sk_buff *skb BT_DBG("%s voice setting 0x%04x", hdev->name, setting); - if (hdev->notify) { - tasklet_disable(&hdev->tx_task); + if (hdev->notify) hdev->notify(hdev, HCI_NOTIFY_VOICE_SETTING); - tasklet_enable(&hdev->tx_task); - } } static void hci_cc_host_buffer_size(struct hci_dev *hdev, struct sk_buff *skb) -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 14/22] Bluetooth: Move command task to workqueue 2011-12-17 21:29 ` [RFC 13/22] Bluetooth: Don't disable tasklets to call hdev->notify() Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 15/22] Bluetooth: convert tx_task " Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> As part of the moving on all the Bluetooth processing to Process context. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_core.c | 22 +++++++++++----------- net/bluetooth/hci_event.c | 4 ++-- net/bluetooth/hci_sock.c | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index e832433..051fd7f 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -196,7 +196,7 @@ struct hci_dev { struct timer_list cmd_timer; struct work_struct rx_work; - struct tasklet_struct cmd_task; + struct work_struct cmd_work; struct tasklet_struct tx_task; struct sk_buff_head rx_q; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 7654777..405319e 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -57,7 +57,7 @@ int enable_hs; static void hci_rx_work(struct work_struct *work); -static void hci_cmd_task(unsigned long arg); +static void hci_cmd_work(struct work_struct *work); static void hci_tx_task(unsigned long arg); static DEFINE_MUTEX(hci_task_lock); @@ -209,7 +209,7 @@ static void hci_init_req(struct hci_dev *hdev, unsigned long opt) skb->dev = (void *) hdev; skb_queue_tail(&hdev->cmd_q, skb); - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); } skb_queue_purge(&hdev->driver_init); @@ -548,7 +548,7 @@ int hci_dev_open(__u16 dev) } else { /* Init failed, cleanup */ tasklet_kill(&hdev->tx_task); - tasklet_kill(&hdev->cmd_task); + flush_work(&hdev->cmd_work); flush_work(&hdev->rx_work); skb_queue_purge(&hdev->cmd_q); @@ -617,8 +617,8 @@ static int hci_dev_do_close(struct hci_dev *hdev) clear_bit(HCI_INIT, &hdev->flags); } - /* Kill cmd task and evt work */ - tasklet_kill(&hdev->cmd_task); + /* flush cmd and rx work */ + flush_work(&hdev->cmd_work); flush_work(&hdev->rx_work); /* Drop queues */ @@ -1208,7 +1208,7 @@ static void hci_cmd_timer(unsigned long arg) BT_ERR("%s command tx timeout", hdev->name); atomic_set(&hdev->cmd_cnt, 1); - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); } struct oob_data *hci_find_remote_oob_data(struct hci_dev *hdev, @@ -1459,8 +1459,8 @@ int hci_register_dev(struct hci_dev *hdev) hdev->sniff_min_interval = 80; INIT_WORK(&hdev->rx_work, hci_rx_work); + INIT_WORK(&hdev->cmd_work, hci_cmd_work); - tasklet_init(&hdev->cmd_task, hci_cmd_task,(unsigned long) hdev); tasklet_init(&hdev->tx_task, hci_tx_task, (unsigned long) hdev); skb_queue_head_init(&hdev->rx_q); @@ -1923,7 +1923,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param) hdev->init_last_cmd = opcode; skb_queue_tail(&hdev->cmd_q, skb); - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); return 0; } @@ -2561,9 +2561,9 @@ static void hci_rx_work(struct work_struct *work) mutex_unlock(&hci_task_lock); } -static void hci_cmd_task(unsigned long arg) +static void hci_cmd_work(struct work_struct *work) { - struct hci_dev *hdev = (struct hci_dev *) arg; + struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_work); struct sk_buff *skb; BT_DBG("%s cmd %d", hdev->name, atomic_read(&hdev->cmd_cnt)); @@ -2587,7 +2587,7 @@ static void hci_cmd_task(unsigned long arg) jiffies + msecs_to_jiffies(HCI_CMD_TIMEOUT)); } else { skb_queue_head(&hdev->cmd_q, skb); - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); } } } diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 93ecb2d..23466bb 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2112,7 +2112,7 @@ static inline void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *sk if (ev->ncmd) { atomic_set(&hdev->cmd_cnt, 1); if (!skb_queue_empty(&hdev->cmd_q)) - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); } } @@ -2194,7 +2194,7 @@ static inline void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb) if (ev->ncmd && !test_bit(HCI_RESET, &hdev->flags)) { atomic_set(&hdev->cmd_cnt, 1); if (!skb_queue_empty(&hdev->cmd_q)) - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); } } diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index 399be34..d10a724 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -538,7 +538,7 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock, tasklet_schedule(&hdev->tx_task); } else { skb_queue_tail(&hdev->cmd_q, skb); - tasklet_schedule(&hdev->cmd_task); + queue_work(hdev->workqueue, &hdev->cmd_work); } } else { if (!capable(CAP_NET_RAW)) { -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 15/22] Bluetooth: convert tx_task to workqueue 2011-12-17 21:29 ` [RFC 14/22] Bluetooth: Move command task to workqueue Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 16/22] Bluetooth: convert info timer to delayed_work Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> This should simplify Bluetooth core processing a lot. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_core.c | 18 ++++++++---------- net/bluetooth/hci_event.c | 6 +----- net/bluetooth/hci_sock.c | 4 ++-- 4 files changed, 12 insertions(+), 18 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 051fd7f..5d1bb51 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -197,7 +197,7 @@ struct hci_dev { struct work_struct rx_work; struct work_struct cmd_work; - struct tasklet_struct tx_task; + struct work_struct tx_work; struct sk_buff_head rx_q; struct sk_buff_head raw_q; diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 405319e..fbc7968 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -58,7 +58,7 @@ int enable_hs; static void hci_rx_work(struct work_struct *work); static void hci_cmd_work(struct work_struct *work); -static void hci_tx_task(unsigned long arg); +static void hci_tx_work(struct work_struct *work); static DEFINE_MUTEX(hci_task_lock); @@ -547,7 +547,7 @@ int hci_dev_open(__u16 dev) } } else { /* Init failed, cleanup */ - tasklet_kill(&hdev->tx_task); + flush_work(&hdev->tx_work); flush_work(&hdev->cmd_work); flush_work(&hdev->rx_work); @@ -587,7 +587,6 @@ static int hci_dev_do_close(struct hci_dev *hdev) /* Kill RX and TX tasks */ cancel_work_sync(&hdev->rx_work); - tasklet_kill(&hdev->tx_task); if (hdev->discov_timeout > 0) { cancel_delayed_work(&hdev->discov_off); @@ -620,6 +619,7 @@ static int hci_dev_do_close(struct hci_dev *hdev) /* flush cmd and rx work */ flush_work(&hdev->cmd_work); flush_work(&hdev->rx_work); + flush_work(&hdev->tx_work); /* Drop queues */ skb_queue_purge(&hdev->rx_q); @@ -673,7 +673,6 @@ int hci_dev_reset(__u16 dev) return -ENODEV; hci_req_lock(hdev); - tasklet_disable(&hdev->tx_task); if (!test_bit(HCI_UP, &hdev->flags)) goto done; @@ -698,7 +697,6 @@ int hci_dev_reset(__u16 dev) msecs_to_jiffies(HCI_INIT_TIMEOUT)); done: - tasklet_enable(&hdev->tx_task); hci_req_unlock(hdev); hci_dev_put(hdev); return ret; @@ -1460,8 +1458,8 @@ int hci_register_dev(struct hci_dev *hdev) INIT_WORK(&hdev->rx_work, hci_rx_work); INIT_WORK(&hdev->cmd_work, hci_cmd_work); + INIT_WORK(&hdev->tx_work, hci_tx_work); - tasklet_init(&hdev->tx_task, hci_tx_task, (unsigned long) hdev); skb_queue_head_init(&hdev->rx_q); skb_queue_head_init(&hdev->cmd_q); @@ -2013,7 +2011,7 @@ void hci_send_acl(struct hci_chan *chan, struct sk_buff *skb, __u16 flags) hci_queue_acl(conn, &chan->data_q, skb, flags); - tasklet_schedule(&hdev->tx_task); + queue_work(hdev->workqueue, &hdev->tx_work); } EXPORT_SYMBOL(hci_send_acl); @@ -2036,7 +2034,7 @@ void hci_send_sco(struct hci_conn *conn, struct sk_buff *skb) bt_cb(skb)->pkt_type = HCI_SCODATA_PKT; skb_queue_tail(&conn->data_q, skb); - tasklet_schedule(&hdev->tx_task); + queue_work(hdev->workqueue, &hdev->tx_work); } EXPORT_SYMBOL(hci_send_sco); @@ -2400,9 +2398,9 @@ static inline void hci_sched_le(struct hci_dev *hdev) hci_prio_recalculate(hdev, LE_LINK); } -static void hci_tx_task(unsigned long arg) +static void hci_tx_work(struct work_struct *work) { - struct hci_dev *hdev = (struct hci_dev *) arg; + struct hci_dev *hdev = container_of(work, struct hci_dev, tx_work); struct sk_buff *skb; mutex_lock(&hci_task_lock); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 23466bb..74f7583 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2239,8 +2239,6 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s return; } - tasklet_disable(&hdev->tx_task); - for (i = 0, ptr = (__le16 *) skb->data; i < ev->num_hndl; i++) { struct hci_conn *conn; __u16 handle, count; @@ -2274,9 +2272,7 @@ static inline void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *s } } - tasklet_schedule(&hdev->tx_task); - - tasklet_enable(&hdev->tx_task); + queue_work(hdev->workqueue, &hdev->tx_work); } static inline void hci_mode_change_evt(struct hci_dev *hdev, struct sk_buff *skb) diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c index d10a724..cd06406 100644 --- a/net/bluetooth/hci_sock.c +++ b/net/bluetooth/hci_sock.c @@ -535,7 +535,7 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock, if (test_bit(HCI_RAW, &hdev->flags) || (ogf == 0x3f)) { skb_queue_tail(&hdev->raw_q, skb); - tasklet_schedule(&hdev->tx_task); + queue_work(hdev->workqueue, &hdev->tx_work); } else { skb_queue_tail(&hdev->cmd_q, skb); queue_work(hdev->workqueue, &hdev->cmd_work); @@ -547,7 +547,7 @@ static int hci_sock_sendmsg(struct kiocb *iocb, struct socket *sock, } skb_queue_tail(&hdev->raw_q, skb); - tasklet_schedule(&hdev->tx_task); + queue_work(hdev->workqueue, &hdev->tx_work); } err = len; -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 16/22] Bluetooth: convert info timer to delayed_work 2011-12-17 21:29 ` [RFC 15/22] Bluetooth: convert tx_task " Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 17/22] Bluetooth: remove power_on work_struct Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Another step of remove interrupt context from Bluetooth Core. Use the system workqueue. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/l2cap.h | 2 +- net/bluetooth/l2cap_core.c | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index a175091..f791374 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -522,7 +522,7 @@ struct l2cap_conn { __u8 info_state; __u8 info_ident; - struct timer_list info_timer; + struct delayed_work info_work; spinlock_t lock; diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 5c5948f..a78cdf7 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -698,7 +698,7 @@ static void l2cap_do_start(struct l2cap_chan *chan) conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_SENT; conn->info_ident = l2cap_get_ident(conn); - mod_timer(&conn->info_timer, jiffies + + schedule_delayed_work(&conn->info_work, msecs_to_jiffies(L2CAP_INFO_TIMEOUT)); l2cap_send_cmd(conn, conn->info_ident, @@ -998,9 +998,10 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) mutex_unlock(&conn->chan_lock); } -static void l2cap_info_timeout(unsigned long arg) +static void l2cap_info_timeout(struct work_struct *work) { - struct l2cap_conn *conn = (void *) arg; + struct l2cap_conn *conn = container_of(work, struct l2cap_conn, + info_work.work); conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE; conn->info_ident = 0; @@ -1033,7 +1034,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err) hci_chan_del(conn->hchan); if (conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT) - del_timer_sync(&conn->info_timer); + cancel_delayed_work_sync(&conn->info_work); if (test_and_clear_bit(HCI_CONN_LE_SMP_PEND, &hcon->pend)) { del_timer(&conn->security_timer); @@ -1094,8 +1095,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) setup_timer(&conn->security_timer, security_timeout, (unsigned long) conn); else - setup_timer(&conn->info_timer, l2cap_info_timeout, - (unsigned long) conn); + INIT_DELAYED_WORK(&conn->info_work, l2cap_info_timeout); conn->disc_reason = HCI_ERROR_REMOTE_USER_TERM; @@ -2530,7 +2530,7 @@ static inline int l2cap_command_rej(struct l2cap_conn *conn, struct l2cap_cmd_hd if ((conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_SENT) && cmd->ident == conn->info_ident) { - del_timer(&conn->info_timer); + cancel_delayed_work_sync(&conn->info_work); conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE; conn->info_ident = 0; @@ -2656,7 +2656,7 @@ sendresp: conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_SENT; conn->info_ident = l2cap_get_ident(conn); - mod_timer(&conn->info_timer, jiffies + + schedule_delayed_work(&conn->info_work, msecs_to_jiffies(L2CAP_INFO_TIMEOUT)); l2cap_send_cmd(conn, conn->info_ident, @@ -3081,7 +3081,7 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE) return 0; - del_timer(&conn->info_timer); + cancel_delayed_work_sync(&conn->info_work); if (result != L2CAP_IR_SUCCESS) { conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE; -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 17/22] Bluetooth: remove power_on work_struct 2011-12-17 21:29 ` [RFC 16/22] Bluetooth: convert info timer to delayed_work Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 18/22] Bluetooth: invert locking order in connect path Gustavo F. Padovan 2011-12-17 22:13 ` [RFC 17/22] Bluetooth: remove power_on work_struct Marcel Holtmann 0 siblings, 2 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Since now we run in process context, we can now call hci_power_on directly. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_core.c | 7 ++----- net/bluetooth/mgmt.c | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 5d1bb51..662877a 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -187,7 +187,6 @@ struct hci_dev { struct workqueue_struct *workqueue; - struct work_struct power_on; struct delayed_work power_off; __u16 discov_timeout; @@ -597,6 +596,7 @@ int hci_register_dev(struct hci_dev *hdev); void hci_unregister_dev(struct hci_dev *hdev); int hci_suspend_dev(struct hci_dev *hdev); int hci_resume_dev(struct hci_dev *hdev); +void hci_power_on(struct hci_dev *hdev); int hci_dev_open(__u16 dev); int hci_dev_close(__u16 dev); int hci_dev_reset(__u16 dev); diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index fbc7968..5c7fec8 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -928,10 +928,8 @@ void hci_free_dev(struct hci_dev *hdev) } EXPORT_SYMBOL(hci_free_dev); -static void hci_power_on(struct work_struct *work) +void hci_power_on(struct hci_dev *hdev) { - struct hci_dev *hdev = container_of(work, struct hci_dev, power_on); - BT_DBG("%s", hdev->name); if (hci_dev_open(hdev->id) < 0) @@ -1490,7 +1488,6 @@ int hci_register_dev(struct hci_dev *hdev) INIT_LIST_HEAD(&hdev->adv_entries); INIT_DELAYED_WORK(&hdev->adv_work, hci_clear_adv_cache); - INIT_WORK(&hdev->power_on, hci_power_on); INIT_DELAYED_WORK(&hdev->power_off, hci_power_off); INIT_DELAYED_WORK(&hdev->discov_off, hci_discov_off); @@ -1522,7 +1519,7 @@ int hci_register_dev(struct hci_dev *hdev) set_bit(HCI_AUTO_OFF, &hdev->flags); set_bit(HCI_SETUP, &hdev->flags); - queue_work(hdev->workqueue, &hdev->power_on); + hci_power_on(hdev); hci_notify(hdev, HCI_DEV_REG); diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index ad4817c..30cbdd7 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -415,7 +415,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len) } if (cp->val) - queue_work(hdev->workqueue, &hdev->power_on); + hci_power_on(hdev); else queue_work(hdev->workqueue, &hdev->power_off.work); -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 18/22] Bluetooth: invert locking order in connect path 2011-12-17 21:29 ` [RFC 17/22] Bluetooth: remove power_on work_struct Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Gustavo F. Padovan 2011-12-17 22:13 ` [RFC 17/22] Bluetooth: remove power_on work_struct Marcel Holtmann 1 sibling, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> This move some checking code that was in l2cap_sock_connect() to l2cap_chan_connect(). Thus we can invert the lock calls, i.e., call lock_sock() before hci_dev_lock() to avoid a deadlock scenario. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/l2cap.h | 3 +- net/bluetooth/l2cap_core.c | 58 +++++++++++++++++++++++++++++++++++++- net/bluetooth/l2cap_sock.c | 61 ++--------------------------------------- 3 files changed, 61 insertions(+), 61 deletions(-) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index f791374..c0d168a 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -806,7 +806,8 @@ int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid); struct l2cap_chan *l2cap_chan_create(struct sock *sk); void l2cap_chan_close(struct l2cap_chan *chan, int reason); void l2cap_chan_destroy(struct l2cap_chan *chan); -int l2cap_chan_connect(struct l2cap_chan *chan); +inline int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, + bdaddr_t *dst); int l2cap_chan_send(struct l2cap_chan *chan, struct msghdr *msg, size_t len, u32 priority); void l2cap_chan_busy(struct l2cap_chan *chan, int busy); diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index a78cdf7..d616519 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1144,11 +1144,10 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, bdaddr return c1; } -int l2cap_chan_connect(struct l2cap_chan *chan) +inline int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, bdaddr_t *dst) { struct sock *sk = chan->sk; bdaddr_t *src = &bt_sk(sk)->src; - bdaddr_t *dst = &bt_sk(sk)->dst; struct l2cap_conn *conn; struct hci_conn *hcon; struct hci_dev *hdev; @@ -1164,6 +1163,61 @@ int l2cap_chan_connect(struct l2cap_chan *chan) hci_dev_lock(hdev); + lock_sock(sk); + + /* PSM must be odd and lsb of upper byte must be 0 */ + if ((__le16_to_cpu(psm) & 0x0101) != 0x0001 && !cid && + chan->chan_type != L2CAP_CHAN_RAW) { + err = -EINVAL; + goto done; + } + + if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED && !(psm || cid)) { + err = -EINVAL; + goto done; + } + + switch (chan->mode) { + case L2CAP_MODE_BASIC: + break; + case L2CAP_MODE_ERTM: + case L2CAP_MODE_STREAMING: + if (!disable_ertm) + break; + /* fall through */ + default: + err = -ENOTSUPP; + goto done; + } + + switch (sk->sk_state) { + case BT_CONNECT: + case BT_CONNECT2: + case BT_CONFIG: + /* Already connecting */ + err = 0; + goto done; + + case BT_CONNECTED: + /* Already connected */ + err = -EISCONN; + goto done; + + case BT_OPEN: + case BT_BOUND: + /* Can connect */ + break; + + default: + err = -EBADFD; + goto done; + } + + /* Set destination address and psm */ + bacpy(&bt_sk(sk)->dst, src); + chan->psm = psm; + chan->dcid = cid; + auth_type = l2cap_get_auth_type(chan); if (chan->dcid == L2CAP_CID_LE_DATA) diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c index fbdc8b3..6c7d432 100644 --- a/net/bluetooth/l2cap_sock.c +++ b/net/bluetooth/l2cap_sock.c @@ -121,70 +121,15 @@ static int l2cap_sock_connect(struct socket *sock, struct sockaddr *addr, int al if (la.l2_cid && la.l2_psm) return -EINVAL; - lock_sock(sk); - - if (chan->chan_type == L2CAP_CHAN_CONN_ORIENTED - && !(la.l2_psm || la.l2_cid)) { - err = -EINVAL; - goto done; - } - - switch (chan->mode) { - case L2CAP_MODE_BASIC: - break; - case L2CAP_MODE_ERTM: - case L2CAP_MODE_STREAMING: - if (!disable_ertm) - break; - /* fall through */ - default: - err = -ENOTSUPP; - goto done; - } - - switch (sk->sk_state) { - case BT_CONNECT: - case BT_CONNECT2: - case BT_CONFIG: - /* Already connecting */ - goto wait; - - case BT_CONNECTED: - /* Already connected */ - err = -EISCONN; - goto done; - - case BT_OPEN: - case BT_BOUND: - /* Can connect */ - break; - - default: - err = -EBADFD; - goto done; - } - - /* PSM must be odd and lsb of upper byte must be 0 */ - if ((__le16_to_cpu(la.l2_psm) & 0x0101) != 0x0001 && !la.l2_cid && - chan->chan_type != L2CAP_CHAN_RAW) { - err = -EINVAL; - goto done; - } - - /* Set destination address and psm */ - bacpy(&bt_sk(sk)->dst, &la.l2_bdaddr); - chan->psm = la.l2_psm; - chan->dcid = la.l2_cid; - - err = l2cap_chan_connect(chan); + err = l2cap_chan_connect(chan, la.l2_psm, la.l2_cid, &la.l2_bdaddr); if (err) goto done; -wait: err = bt_sock_wait_state(sk, BT_CONNECTED, sock_sndtimeo(sk, flags & O_NONBLOCK)); done: - release_sock(sk); + if (sock_owned_by_user(sk)) + release_sock(sk); return err; } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU 2011-12-17 21:29 ` [RFC 18/22] Bluetooth: invert locking order in connect path Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 20/22] Bluetooth: move power_off to system workqueue Gustavo F. Padovan ` (2 more replies) 0 siblings, 3 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> This list has much more reads than writes, so RCU makes senses here, also it avoid deadlock against the socket lock. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/l2cap_core.c | 117 +++++++++++++++++++++---------------------- 1 files changed, 57 insertions(+), 60 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index d616519..a212295 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -89,24 +89,36 @@ static inline void chan_put(struct l2cap_chan *c) static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid) { - struct l2cap_chan *c; + struct l2cap_chan *c, *r = NULL; - list_for_each_entry(c, &conn->chan_l, list) { - if (c->dcid == cid) - return c; + rcu_read_lock(); + + list_for_each_entry_rcu(c, &conn->chan_l, list) { + if (c->dcid == cid) { + r = c; + break; + } } + + rcu_read_unlock(); return NULL; } static struct l2cap_chan *__l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 cid) { - struct l2cap_chan *c; + struct l2cap_chan *c, *r = NULL; - list_for_each_entry(c, &conn->chan_l, list) { - if (c->scid == cid) - return c; + rcu_read_lock(); + + list_for_each_entry_rcu(c, &conn->chan_l, list) { + if (c->scid == cid) { + r = c; + break; + } } - return NULL; + + rcu_read_unlock(); + return r; } /* Find channel with given SCID. @@ -115,34 +127,36 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci { struct l2cap_chan *c; - mutex_lock(&conn->chan_lock); c = __l2cap_get_chan_by_scid(conn, cid); if (c) lock_sock(c->sk); - mutex_unlock(&conn->chan_lock); return c; } static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident) { - struct l2cap_chan *c; + struct l2cap_chan *c, *r = NULL; - list_for_each_entry(c, &conn->chan_l, list) { - if (c->ident == ident) - return c; + rcu_read_lock(); + + list_for_each_entry_rcu(c, &conn->chan_l, list) { + if (c->ident == ident) { + r = c; + break; + } } - return NULL; + + rcu_read_unlock(); + return r; } static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident) { struct l2cap_chan *c; - mutex_lock(&conn->chan_lock); c = __l2cap_get_chan_by_ident(conn, ident); if (c) lock_sock(c->sk); - mutex_unlock(&conn->chan_lock); return c; } @@ -323,7 +337,7 @@ void l2cap_chan_destroy(struct l2cap_chan *chan) chan_put(chan); } -static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) +static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) { BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn, chan->psm, chan->dcid); @@ -364,7 +378,7 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) chan_hold(chan); - list_add(&chan->list, &conn->chan_l); + list_add_rcu(&chan->list, &conn->chan_l); } /* Delete channel. @@ -381,9 +395,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err) if (conn) { /* Delete from channel list */ - mutex_lock(&conn->chan_lock); - list_del(&chan->list); - mutex_unlock(&conn->chan_lock); + list_del_rcu(&chan->list); + synchronize_rcu(); + chan_put(chan); chan->conn = NULL; @@ -750,13 +764,13 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c /* ---- L2CAP connections ---- */ static void l2cap_conn_start(struct l2cap_conn *conn) { - struct l2cap_chan *chan, *tmp; + struct l2cap_chan *chan; BT_DBG("conn %p", conn); - mutex_lock(&conn->chan_lock); + rcu_read_lock(); - list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) { + list_for_each_entry_rcu(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; bh_lock_sock(sk); @@ -780,9 +794,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) &chan->conf_state)) { /* l2cap_chan_close() calls list_del(chan) * so release the lock */ - mutex_unlock(&conn->chan_lock); l2cap_chan_close(chan, ECONNRESET); - utex_lock(&conn->chan_lock); bh_unlock_sock(sk); continue; } @@ -838,7 +850,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) bh_unlock_sock(sk); } - mutex_unlock(&conn->chan_lock); + rcu_read_unlock(); } /* Find socket with cid and source bdaddr. @@ -903,8 +915,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) sk = chan->sk; - mutex_lock(&conn->chan_lock); - hci_conn_hold(conn->hcon); bacpy(&bt_sk(sk)->src, conn->src); @@ -912,15 +922,13 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) bt_accept_enqueue(parent, sk); - __l2cap_chan_add(conn, chan); + l2cap_chan_add(conn, chan); __set_chan_timer(chan, sk->sk_sndtimeo); l2cap_state_change(chan, BT_CONNECTED); parent->sk_data_ready(parent, 0); - mutex_unlock(&conn->chan_lock); - clean: release_sock(parent); } @@ -954,9 +962,9 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) if (conn->hcon->out && conn->hcon->type == LE_LINK) smp_conn_security(conn, conn->hcon->pending_sec_level); - mutex_lock(&conn->chan_lock); + rcu_read_lock(); - list_for_each_entry(chan, &conn->chan_l, list) { + list_for_each_entry_rcu(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; bh_lock_sock(sk); @@ -976,7 +984,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) bh_unlock_sock(sk); } - mutex_unlock(&conn->chan_lock); + rcu_read_unlock(); } /* Notify sockets that we cannot guaranty reliability anymore */ @@ -986,16 +994,16 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) BT_DBG("conn %p", conn); - mutex_lock(&conn->chan_lock); + rcu_read_lock(); - list_for_each_entry(chan, &conn->chan_l, list) { + list_for_each_entry_rcu(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; if (test_bit(FLAG_FORCE_RELIABLE, &chan->flags)) sk->sk_err = err; } - mutex_unlock(&conn->chan_lock); + rcu_read_unlock(); } static void l2cap_info_timeout(struct work_struct *work) @@ -1087,7 +1095,6 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) conn->feat_mask = 0; spin_lock_init(&conn->lock); - mutex_init(&conn->chan_lock); INIT_LIST_HEAD(&conn->chan_l); @@ -1102,13 +1109,6 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) return conn; } -static inline void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) -{ - mutex_lock(&conn->chan_lock); - __l2cap_chan_add(conn, chan); - mutex_unlock(&conn->chan_lock); -} - /* ---- Socket interface ---- */ /* Find socket with psm and source bdaddr. @@ -1825,8 +1825,9 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) BT_DBG("conn %p", conn); - mutex_lock(&conn->chan_lock); - list_for_each_entry(chan, &conn->chan_l, list) { + rcu_read_lock(); + + list_for_each_entry_rcu(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; if (chan->chan_type != L2CAP_CHAN_RAW) continue; @@ -1841,7 +1842,8 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) if (chan->ops->recv(chan->data, nskb)) kfree_skb(nskb); } - mutex_unlock(&conn->chan_lock); + + rcu_read_unlock(); } /* ---- L2CAP signalling commands ---- */ @@ -2641,11 +2643,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd sk = chan->sk; - mutex_lock(&conn->chan_lock); - /* Check if we already have channel with that dcid */ if (__l2cap_get_chan_by_dcid(conn, scid)) { - mutex_unlock(&conn->chan_lock); sock_set_flag(sk, SOCK_ZAPPED); chan->ops->close(chan->data); goto response; @@ -2660,7 +2659,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd bt_accept_enqueue(parent, sk); - __l2cap_chan_add(conn, chan); + l2cap_chan_add(conn, chan); dcid = chan->scid; @@ -2691,8 +2690,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd status = L2CAP_CS_NO_INFO; } - mutex_unlock(&conn->chan_lock); - response: release_sock(parent); @@ -4528,9 +4525,9 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) del_timer(&conn->security_timer); } - mutex_lock(&conn->chan_lock); + rcu_read_lock(); - list_for_each_entry(chan, &conn->chan_l, list) { + list_for_each_entry_rcu(chan, &conn->chan_l, list) { struct sock *sk = chan->sk; bh_lock_sock(sk); @@ -4608,7 +4605,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) bh_unlock_sock(sk); } - mutex_unlock(&conn->chan_lock); + rcu_read_unlock(); return 0; } -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 20/22] Bluetooth: move power_off to system workqueue 2011-12-17 21:29 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 21/22] Bluetooth: Use new alloc_workqueue() Gustavo F. Padovan 2011-12-17 22:15 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Marcel Holtmann 2011-12-19 10:42 ` Andrei Emeltchenko 2 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> hdev->workqueue will be only for for rx/tx/cmd processing, all other small works should go to the system workqueue for now. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/hci_core.c | 2 +- net/bluetooth/mgmt.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 5c7fec8..e4ff7b6 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -936,7 +936,7 @@ void hci_power_on(struct hci_dev *hdev) return; if (test_bit(HCI_AUTO_OFF, &hdev->flags)) - queue_delayed_work(hdev->workqueue, &hdev->power_off, + schedule_delayed_work(&hdev->power_off, msecs_to_jiffies(AUTO_OFF_TIMEOUT)); if (test_and_clear_bit(HCI_SETUP, &hdev->flags)) diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 30cbdd7..4402c4d 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -417,7 +417,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len) if (cp->val) hci_power_on(hdev); else - queue_work(hdev->workqueue, &hdev->power_off.work); + schedule_work(&hdev->power_off.work); err = 0; -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 21/22] Bluetooth: Use new alloc_workqueue() 2011-12-17 21:29 ` [RFC 20/22] Bluetooth: move power_off to system workqueue Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 22/22] Bluetooth: Remove work_add and work_del from hci_sysfs Gustavo F. Padovan 0 siblings, 1 reply; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> Update hdev workqueue API usage to use the new interface, this new interface also allow us to mark this workqueue as WQ_HIGHPRI, so now rx and tx work gets higher priority when running. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/hci_core.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index e4ff7b6..21816dd 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1498,7 +1498,7 @@ int hci_register_dev(struct hci_dev *hdev) write_unlock_bh(&hci_dev_list_lock); - hdev->workqueue = create_singlethread_workqueue(hdev->name); + hdev->workqueue = alloc_workqueue(hdev->name, WQ_HIGHPRI, 1); if (!hdev->workqueue) { error = -ENOMEM; goto err; -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* [RFC 22/22] Bluetooth: Remove work_add and work_del from hci_sysfs 2011-12-17 21:29 ` [RFC 21/22] Bluetooth: Use new alloc_workqueue() Gustavo F. Padovan @ 2011-12-17 21:29 ` Gustavo F. Padovan 0 siblings, 0 replies; 34+ messages in thread From: Gustavo F. Padovan @ 2011-12-17 21:29 UTC (permalink / raw) To: linux-bluetooth; +Cc: Gustavo F. Padovan From: "Gustavo F. Padovan" <padovan@profusion.mobi> As we run in process context now we don't need worqueue to add e del from sysfs. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- include/net/bluetooth/hci_core.h | 3 -- net/bluetooth/hci_sysfs.c | 71 ++++++++++++++----------------------- 2 files changed, 27 insertions(+), 47 deletions(-) diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 662877a..d10d2ba 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -300,9 +300,6 @@ struct hci_conn { struct timer_list idle_timer; struct timer_list auto_accept_timer; - struct work_struct work_add; - struct work_struct work_del; - struct device dev; atomic_t devref; diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index c3c1ec8..db6af70 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -88,11 +88,35 @@ static struct device_type bt_link = { .release = bt_link_release, }; -static void add_conn(struct work_struct *work) +/* + * The rfcomm tty device will possibly retain even when conn + * is down, and sysfs doesn't support move zombie device, + * so we should move the device before conn device is destroyed. + */ +static int __match_tty(struct device *dev, void *data) +{ + return !strncmp(dev_name(dev), "rfcomm", 6); +} + +void hci_conn_init_sysfs(struct hci_conn *conn) +{ + struct hci_dev *hdev = conn->hdev; + + BT_DBG("conn %p", conn); + + conn->dev.type = &bt_link; + conn->dev.class = bt_class; + conn->dev.parent = &hdev->dev; + + device_initialize(&conn->dev); +} + +void hci_conn_add_sysfs(struct hci_conn *conn) { - struct hci_conn *conn = container_of(work, struct hci_conn, work_add); struct hci_dev *hdev = conn->hdev; + BT_DBG("conn %p", conn); + dev_set_name(&conn->dev, "%s:%d", hdev->name, conn->handle); dev_set_drvdata(&conn->dev, conn); @@ -105,19 +129,8 @@ static void add_conn(struct work_struct *work) hci_dev_hold(hdev); } -/* - * The rfcomm tty device will possibly retain even when conn - * is down, and sysfs doesn't support move zombie device, - * so we should move the device before conn device is destroyed. - */ -static int __match_tty(struct device *dev, void *data) -{ - return !strncmp(dev_name(dev), "rfcomm", 6); -} - -static void del_conn(struct work_struct *work) +void hci_conn_del_sysfs(struct hci_conn *conn) { - struct hci_conn *conn = container_of(work, struct hci_conn, work_del); struct hci_dev *hdev = conn->hdev; if (!device_is_registered(&conn->dev)) @@ -139,36 +152,6 @@ static void del_conn(struct work_struct *work) hci_dev_put(hdev); } -void hci_conn_init_sysfs(struct hci_conn *conn) -{ - struct hci_dev *hdev = conn->hdev; - - BT_DBG("conn %p", conn); - - conn->dev.type = &bt_link; - conn->dev.class = bt_class; - conn->dev.parent = &hdev->dev; - - device_initialize(&conn->dev); - - INIT_WORK(&conn->work_add, add_conn); - INIT_WORK(&conn->work_del, del_conn); -} - -void hci_conn_add_sysfs(struct hci_conn *conn) -{ - BT_DBG("conn %p", conn); - - queue_work(conn->hdev->workqueue, &conn->work_add); -} - -void hci_conn_del_sysfs(struct hci_conn *conn) -{ - BT_DBG("conn %p", conn); - - queue_work(conn->hdev->workqueue, &conn->work_del); -} - static inline char *host_bustostr(int bus) { switch (bus) { -- 1.7.6.4 ^ permalink raw reply related [flat|nested] 34+ messages in thread
* Re: [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU 2011-12-17 21:29 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 20/22] Bluetooth: move power_off to system workqueue Gustavo F. Padovan @ 2011-12-17 22:15 ` Marcel Holtmann 2011-12-19 10:42 ` Andrei Emeltchenko 2 siblings, 0 replies; 34+ messages in thread From: Marcel Holtmann @ 2011-12-17 22:15 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, > This list has much more reads than writes, so RCU makes senses here, also > it avoid deadlock against the socket lock. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > net/bluetooth/l2cap_core.c | 117 +++++++++++++++++++++---------------------- > 1 files changed, 57 insertions(+), 60 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index d616519..a212295 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -89,24 +89,36 @@ static inline void chan_put(struct l2cap_chan *c) > > static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid) > { > - struct l2cap_chan *c; > + struct l2cap_chan *c, *r = NULL; > > - list_for_each_entry(c, &conn->chan_l, list) { > - if (c->dcid == cid) > - return c; > + rcu_read_lock(); > + > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > + if (c->dcid == cid) { > + r = c; > + break; > + } > } > + > + rcu_read_unlock(); > return NULL; > } shouldn't this say return r; Regards Marcel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU 2011-12-17 21:29 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 20/22] Bluetooth: move power_off to system workqueue Gustavo F. Padovan 2011-12-17 22:15 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Marcel Holtmann @ 2011-12-19 10:42 ` Andrei Emeltchenko 2011-12-19 13:53 ` Gustavo Padovan 2 siblings, 1 reply; 34+ messages in thread From: Andrei Emeltchenko @ 2011-12-19 10:42 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, On Sat, Dec 17, 2011 at 07:29:39PM -0200, Gustavo F. Padovan wrote: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > This list has much more reads than writes, so RCU makes senses here, also > it avoid deadlock against the socket lock. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > net/bluetooth/l2cap_core.c | 117 +++++++++++++++++++++---------------------- > 1 files changed, 57 insertions(+), 60 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index d616519..a212295 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -89,24 +89,36 @@ static inline void chan_put(struct l2cap_chan *c) > > static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid) > { > - struct l2cap_chan *c; > + struct l2cap_chan *c, *r = NULL; > > - list_for_each_entry(c, &conn->chan_l, list) { > - if (c->dcid == cid) > - return c; > + rcu_read_lock(); > + > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > + if (c->dcid == cid) { > + r = c; > + break; > + } > } > + > + rcu_read_unlock(); > return NULL; > } > > static struct l2cap_chan *__l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 cid) > { > - struct l2cap_chan *c; > + struct l2cap_chan *c, *r = NULL; > > - list_for_each_entry(c, &conn->chan_l, list) { > - if (c->scid == cid) > - return c; > + rcu_read_lock(); > + > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > + if (c->scid == cid) { > + r = c; > + break; > + } > } > - return NULL; > + > + rcu_read_unlock(); > + return r; > } > > /* Find channel with given SCID. > @@ -115,34 +127,36 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci > { > struct l2cap_chan *c; > > - mutex_lock(&conn->chan_lock); > c = __l2cap_get_chan_by_scid(conn, cid); > if (c) > lock_sock(c->sk); > - mutex_unlock(&conn->chan_lock); > return c; > } > > static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident) > { > - struct l2cap_chan *c; > + struct l2cap_chan *c, *r = NULL; > > - list_for_each_entry(c, &conn->chan_l, list) { > - if (c->ident == ident) > - return c; > + rcu_read_lock(); > + > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > + if (c->ident == ident) { > + r = c; > + break; > + } > } > - return NULL; > + > + rcu_read_unlock(); > + return r; > } > > static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident) > { > struct l2cap_chan *c; > > - mutex_lock(&conn->chan_lock); > c = __l2cap_get_chan_by_ident(conn, ident); > if (c) > lock_sock(c->sk); > - mutex_unlock(&conn->chan_lock); > return c; > } > > @@ -323,7 +337,7 @@ void l2cap_chan_destroy(struct l2cap_chan *chan) > chan_put(chan); > } > > -static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > +static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > { > BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn, > chan->psm, chan->dcid); > @@ -364,7 +378,7 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > > chan_hold(chan); > > - list_add(&chan->list, &conn->chan_l); > + list_add_rcu(&chan->list, &conn->chan_l); > } > > /* Delete channel. > @@ -381,9 +395,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err) > > if (conn) { > /* Delete from channel list */ > - mutex_lock(&conn->chan_lock); > - list_del(&chan->list); > - mutex_unlock(&conn->chan_lock); > + list_del_rcu(&chan->list); > + synchronize_rcu(); > + > chan_put(chan); > > chan->conn = NULL; > @@ -750,13 +764,13 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c > /* ---- L2CAP connections ---- */ > static void l2cap_conn_start(struct l2cap_conn *conn) > { > - struct l2cap_chan *chan, *tmp; > + struct l2cap_chan *chan; > > BT_DBG("conn %p", conn); > > - mutex_lock(&conn->chan_lock); > + rcu_read_lock(); > > - list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) { > + list_for_each_entry_rcu(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > > bh_lock_sock(sk); > @@ -780,9 +794,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > &chan->conf_state)) { > /* l2cap_chan_close() calls list_del(chan) > * so release the lock */ > - mutex_unlock(&conn->chan_lock); > l2cap_chan_close(chan, ECONNRESET); > - utex_lock(&conn->chan_lock); OK, I see why this works. BTW: IMO the mutex and rcu patches shall be amended, otherwise we do unneeded work of implementing mutexes and then changing them to RCU. Best regards Andrei Emeltchenko > bh_unlock_sock(sk); > continue; > } > @@ -838,7 +850,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > bh_unlock_sock(sk); > } > > - mutex_unlock(&conn->chan_lock); > + rcu_read_unlock(); > } > > /* Find socket with cid and source bdaddr. > @@ -903,8 +915,6 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > > sk = chan->sk; > > - mutex_lock(&conn->chan_lock); > - > hci_conn_hold(conn->hcon); > > bacpy(&bt_sk(sk)->src, conn->src); > @@ -912,15 +922,13 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > > bt_accept_enqueue(parent, sk); > > - __l2cap_chan_add(conn, chan); > + l2cap_chan_add(conn, chan); > > __set_chan_timer(chan, sk->sk_sndtimeo); > > l2cap_state_change(chan, BT_CONNECTED); > parent->sk_data_ready(parent, 0); > > - mutex_unlock(&conn->chan_lock); > - > clean: > release_sock(parent); > } > @@ -954,9 +962,9 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) > if (conn->hcon->out && conn->hcon->type == LE_LINK) > smp_conn_security(conn, conn->hcon->pending_sec_level); > > - mutex_lock(&conn->chan_lock); > + rcu_read_lock(); > > - list_for_each_entry(chan, &conn->chan_l, list) { > + list_for_each_entry_rcu(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > > bh_lock_sock(sk); > @@ -976,7 +984,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) > bh_unlock_sock(sk); > } > > - mutex_unlock(&conn->chan_lock); > + rcu_read_unlock(); > } > > /* Notify sockets that we cannot guaranty reliability anymore */ > @@ -986,16 +994,16 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) > > BT_DBG("conn %p", conn); > > - mutex_lock(&conn->chan_lock); > + rcu_read_lock(); > > - list_for_each_entry(chan, &conn->chan_l, list) { > + list_for_each_entry_rcu(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > > if (test_bit(FLAG_FORCE_RELIABLE, &chan->flags)) > sk->sk_err = err; > } > > - mutex_unlock(&conn->chan_lock); > + rcu_read_unlock(); > } > > static void l2cap_info_timeout(struct work_struct *work) > @@ -1087,7 +1095,6 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) > conn->feat_mask = 0; > > spin_lock_init(&conn->lock); > - mutex_init(&conn->chan_lock); > > INIT_LIST_HEAD(&conn->chan_l); > > @@ -1102,13 +1109,6 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) > return conn; > } > > -static inline void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > -{ > - mutex_lock(&conn->chan_lock); > - __l2cap_chan_add(conn, chan); > - mutex_unlock(&conn->chan_lock); > -} > - > /* ---- Socket interface ---- */ > > /* Find socket with psm and source bdaddr. > @@ -1825,8 +1825,9 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) > > BT_DBG("conn %p", conn); > > - mutex_lock(&conn->chan_lock); > - list_for_each_entry(chan, &conn->chan_l, list) { > + rcu_read_lock(); > + > + list_for_each_entry_rcu(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > if (chan->chan_type != L2CAP_CHAN_RAW) > continue; > @@ -1841,7 +1842,8 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) > if (chan->ops->recv(chan->data, nskb)) > kfree_skb(nskb); > } > - mutex_unlock(&conn->chan_lock); > + > + rcu_read_unlock(); > } > > /* ---- L2CAP signalling commands ---- */ > @@ -2641,11 +2643,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > > sk = chan->sk; > > - mutex_lock(&conn->chan_lock); > - > /* Check if we already have channel with that dcid */ > if (__l2cap_get_chan_by_dcid(conn, scid)) { > - mutex_unlock(&conn->chan_lock); > sock_set_flag(sk, SOCK_ZAPPED); > chan->ops->close(chan->data); > goto response; > @@ -2660,7 +2659,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > > bt_accept_enqueue(parent, sk); > > - __l2cap_chan_add(conn, chan); > + l2cap_chan_add(conn, chan); > > dcid = chan->scid; > > @@ -2691,8 +2690,6 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > status = L2CAP_CS_NO_INFO; > } > > - mutex_unlock(&conn->chan_lock); > - > response: > release_sock(parent); > > @@ -4528,9 +4525,9 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) > del_timer(&conn->security_timer); > } > > - mutex_lock(&conn->chan_lock); > + rcu_read_lock(); > > - list_for_each_entry(chan, &conn->chan_l, list) { > + list_for_each_entry_rcu(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > > bh_lock_sock(sk); > @@ -4608,7 +4605,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) > bh_unlock_sock(sk); > } > > - mutex_unlock(&conn->chan_lock); > + rcu_read_unlock(); > > return 0; > } > -- > 1.7.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU 2011-12-19 10:42 ` Andrei Emeltchenko @ 2011-12-19 13:53 ` Gustavo Padovan 0 siblings, 0 replies; 34+ messages in thread From: Gustavo Padovan @ 2011-12-19 13:53 UTC (permalink / raw) To: Andrei Emeltchenko, linux-bluetooth Hi Andrei, * Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2011-12-19 12:42:57 +0200]: > Hi Gustavo, > > On Sat, Dec 17, 2011 at 07:29:39PM -0200, Gustavo F. Padovan wrote: > > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > > > This list has much more reads than writes, so RCU makes senses here, also > > it avoid deadlock against the socket lock. > > > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > > --- > > net/bluetooth/l2cap_core.c | 117 +++++++++++++++++++++---------------------- > > 1 files changed, 57 insertions(+), 60 deletions(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index d616519..a212295 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -89,24 +89,36 @@ static inline void chan_put(struct l2cap_chan *c) > > > > static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid) > > { > > - struct l2cap_chan *c; > > + struct l2cap_chan *c, *r = NULL; > > > > - list_for_each_entry(c, &conn->chan_l, list) { > > - if (c->dcid == cid) > > - return c; > > + rcu_read_lock(); > > + > > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > > + if (c->dcid == cid) { > > + r = c; > > + break; > > + } > > } > > + > > + rcu_read_unlock(); > > return NULL; > > } > > > > static struct l2cap_chan *__l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 cid) > > { > > - struct l2cap_chan *c; > > + struct l2cap_chan *c, *r = NULL; > > > > - list_for_each_entry(c, &conn->chan_l, list) { > > - if (c->scid == cid) > > - return c; > > + rcu_read_lock(); > > + > > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > > + if (c->scid == cid) { > > + r = c; > > + break; > > + } > > } > > - return NULL; > > + > > + rcu_read_unlock(); > > + return r; > > } > > > > /* Find channel with given SCID. > > @@ -115,34 +127,36 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci > > { > > struct l2cap_chan *c; > > > > - mutex_lock(&conn->chan_lock); > > c = __l2cap_get_chan_by_scid(conn, cid); > > if (c) > > lock_sock(c->sk); > > - mutex_unlock(&conn->chan_lock); > > return c; > > } > > > > static struct l2cap_chan *__l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident) > > { > > - struct l2cap_chan *c; > > + struct l2cap_chan *c, *r = NULL; > > > > - list_for_each_entry(c, &conn->chan_l, list) { > > - if (c->ident == ident) > > - return c; > > + rcu_read_lock(); > > + > > + list_for_each_entry_rcu(c, &conn->chan_l, list) { > > + if (c->ident == ident) { > > + r = c; > > + break; > > + } > > } > > - return NULL; > > + > > + rcu_read_unlock(); > > + return r; > > } > > > > static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn, u8 ident) > > { > > struct l2cap_chan *c; > > > > - mutex_lock(&conn->chan_lock); > > c = __l2cap_get_chan_by_ident(conn, ident); > > if (c) > > lock_sock(c->sk); > > - mutex_unlock(&conn->chan_lock); > > return c; > > } > > > > @@ -323,7 +337,7 @@ void l2cap_chan_destroy(struct l2cap_chan *chan) > > chan_put(chan); > > } > > > > -static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > > +static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > > { > > BT_DBG("conn %p, psm 0x%2.2x, dcid 0x%4.4x", conn, > > chan->psm, chan->dcid); > > @@ -364,7 +378,7 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > > > > chan_hold(chan); > > > > - list_add(&chan->list, &conn->chan_l); > > + list_add_rcu(&chan->list, &conn->chan_l); > > } > > > > /* Delete channel. > > @@ -381,9 +395,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err) > > > > if (conn) { > > /* Delete from channel list */ > > - mutex_lock(&conn->chan_lock); > > - list_del(&chan->list); > > - mutex_unlock(&conn->chan_lock); > > + list_del_rcu(&chan->list); > > + synchronize_rcu(); > > + > > chan_put(chan); > > > > chan->conn = NULL; > > @@ -750,13 +764,13 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct l2cap_chan *c > > /* ---- L2CAP connections ---- */ > > static void l2cap_conn_start(struct l2cap_conn *conn) > > { > > - struct l2cap_chan *chan, *tmp; > > + struct l2cap_chan *chan; > > > > BT_DBG("conn %p", conn); > > > > - mutex_lock(&conn->chan_lock); > > + rcu_read_lock(); > > > > - list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) { > > + list_for_each_entry_rcu(chan, &conn->chan_l, list) { > > struct sock *sk = chan->sk; > > > > bh_lock_sock(sk); > > @@ -780,9 +794,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > > &chan->conf_state)) { > > /* l2cap_chan_close() calls list_del(chan) > > * so release the lock */ > > - mutex_unlock(&conn->chan_lock); > > l2cap_chan_close(chan, ECONNRESET); > > - utex_lock(&conn->chan_lock); > > OK, I see why this works. BTW: IMO the mutex and rcu patches shall be > amended, otherwise we do unneeded work of implementing mutexes and then > changing them to RCU. Indeed, but unfortunately I pushed the patches already and forgot to ammend both path patches. I was trying to push this ASAP so you guys could rebase your work on top of it. Gustavo ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 17/22] Bluetooth: remove power_on work_struct 2011-12-17 21:29 ` [RFC 17/22] Bluetooth: remove power_on work_struct Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 18/22] Bluetooth: invert locking order in connect path Gustavo F. Padovan @ 2011-12-17 22:13 ` Marcel Holtmann 1 sibling, 0 replies; 34+ messages in thread From: Marcel Holtmann @ 2011-12-17 22:13 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, > Since now we run in process context, we can now call hci_power_on > directly. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > include/net/bluetooth/hci_core.h | 2 +- > net/bluetooth/hci_core.c | 7 ++----- > net/bluetooth/mgmt.c | 2 +- > 3 files changed, 4 insertions(+), 7 deletions(-) > > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h > index 5d1bb51..662877a 100644 > --- a/include/net/bluetooth/hci_core.h > +++ b/include/net/bluetooth/hci_core.h > @@ -187,7 +187,6 @@ struct hci_dev { > > struct workqueue_struct *workqueue; > > - struct work_struct power_on; > struct delayed_work power_off; > > __u16 discov_timeout; > @@ -597,6 +596,7 @@ int hci_register_dev(struct hci_dev *hdev); > void hci_unregister_dev(struct hci_dev *hdev); > int hci_suspend_dev(struct hci_dev *hdev); > int hci_resume_dev(struct hci_dev *hdev); > +void hci_power_on(struct hci_dev *hdev); > int hci_dev_open(__u16 dev); > int hci_dev_close(__u16 dev); > int hci_dev_reset(__u16 dev); > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index fbc7968..5c7fec8 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -928,10 +928,8 @@ void hci_free_dev(struct hci_dev *hdev) > } > EXPORT_SYMBOL(hci_free_dev); > > -static void hci_power_on(struct work_struct *work) > +void hci_power_on(struct hci_dev *hdev) > { > - struct hci_dev *hdev = container_of(work, struct hci_dev, power_on); > - > BT_DBG("%s", hdev->name); > > if (hci_dev_open(hdev->id) < 0) > @@ -1490,7 +1488,6 @@ int hci_register_dev(struct hci_dev *hdev) > INIT_LIST_HEAD(&hdev->adv_entries); > > INIT_DELAYED_WORK(&hdev->adv_work, hci_clear_adv_cache); > - INIT_WORK(&hdev->power_on, hci_power_on); > INIT_DELAYED_WORK(&hdev->power_off, hci_power_off); > > INIT_DELAYED_WORK(&hdev->discov_off, hci_discov_off); > @@ -1522,7 +1519,7 @@ int hci_register_dev(struct hci_dev *hdev) > > set_bit(HCI_AUTO_OFF, &hdev->flags); > set_bit(HCI_SETUP, &hdev->flags); > - queue_work(hdev->workqueue, &hdev->power_on); > + hci_power_on(hdev); > > hci_notify(hdev, HCI_DEV_REG); > > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c > index ad4817c..30cbdd7 100644 > --- a/net/bluetooth/mgmt.c > +++ b/net/bluetooth/mgmt.c > @@ -415,7 +415,7 @@ static int set_powered(struct sock *sk, u16 index, unsigned char *data, u16 len) > } > > if (cp->val) > - queue_work(hdev->workqueue, &hdev->power_on); > + hci_power_on(hdev); > else > queue_work(hdev->workqueue, &hdev->power_off.work); this is a bad idea. We are running with the hdev->workqueue now here we do not wanna interrupt this. So I would rather leave this one out for now. At least until we figured out on what is the best solution. Obviously we should push it onto the system workqueue instead of hdev->workqueue. Regards Marcel ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list 2011-12-17 21:29 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 12/22] Bluetooth: convert conn hash to RCU Gustavo F. Padovan @ 2012-01-26 13:20 ` Andrei Emeltchenko 1 sibling, 0 replies; 34+ messages in thread From: Andrei Emeltchenko @ 2012-01-26 13:20 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel Hi Gustavo, On Sat, Dec 17, 2011 at 07:29:31PM -0200, Gustavo F. Padovan wrote: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > Instead of using tasklet_disable() to prevent acess to the channel use, we > can use RCU and improve the performance of our code. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> ... > @@ -986,10 +984,10 @@ int hci_chan_del(struct hci_chan *chan) > > void hci_chan_list_flush(struct hci_conn *conn) > { > - struct hci_chan *chan, *tmp; > + struct hci_chan *chan; > > BT_DBG("conn %p", conn); > > - list_for_each_entry_safe(chan, tmp, &conn->chan_list, list) > + list_for_each_entry_rcu(chan, &conn->chan_list, list) > hci_chan_del(chan); > } I feel that this code is not exactly correct. hci_chan_del frees chan: <------8<-------------------------------------------------------- | int hci_chan_del(struct hci_chan *chan) | { | struct hci_conn *conn = chan->conn; | struct hci_dev *hdev = conn->hdev; | | BT_DBG("%s conn %p chan %p", hdev->name, conn, chan); | | list_del_rcu(&chan->list); | | synchronize_rcu(); | | skb_queue_purge(&chan->data_q); | kfree(chan); | | return 0; | } <------8<-------------------------------------------------------- and then list_for_each_entry_rcu references chan->member.next which is not safe ;) I've sent patch reverting this change. Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 10/22] Bluetooth: convert chan_lock to mutex 2011-12-17 21:29 ` [RFC 10/22] Bluetooth: convert chan_lock " Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Gustavo F. Padovan @ 2011-12-19 9:58 ` Andrei Emeltchenko 1 sibling, 0 replies; 34+ messages in thread From: Andrei Emeltchenko @ 2011-12-19 9:58 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, On Sat, Dec 17, 2011 at 07:29:30PM -0200, Gustavo F. Padovan wrote: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > spin lock doesn't fit ok anymore on the new code based on workqueues. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > include/net/bluetooth/l2cap.h | 2 +- > net/bluetooth/l2cap_core.c | 52 ++++++++++++++++++++-------------------- > 2 files changed, 27 insertions(+), 27 deletions(-) > > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h > index 03be911..a175091 100644 > --- a/include/net/bluetooth/l2cap.h > +++ b/include/net/bluetooth/l2cap.h > @@ -536,7 +536,7 @@ struct l2cap_conn { > struct smp_chan *smp_chan; > > struct list_head chan_l; > - rwlock_t chan_lock; > + struct mutex chan_lock; > }; > > #define L2CAP_INFO_CL_MTU_REQ_SENT 0x01 > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 31c94fd..5c5948f 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -115,11 +115,11 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci > { > struct l2cap_chan *c; > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > c = __l2cap_get_chan_by_scid(conn, cid); > if (c) > lock_sock(c->sk); > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > return c; > } > > @@ -138,11 +138,11 @@ static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn > { > struct l2cap_chan *c; > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > c = __l2cap_get_chan_by_ident(conn, ident); > if (c) > lock_sock(c->sk); > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > return c; > } > > @@ -381,9 +381,9 @@ static void l2cap_chan_del(struct l2cap_chan *chan, int err) > > if (conn) { > /* Delete from channel list */ > - write_lock_bh(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > list_del(&chan->list); > - write_unlock_bh(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > chan_put(chan); > > chan->conn = NULL; > @@ -754,7 +754,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > > BT_DBG("conn %p", conn); > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > > list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) { > struct sock *sk = chan->sk; > @@ -780,9 +780,9 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > &chan->conf_state)) { > /* l2cap_chan_close() calls list_del(chan) > * so release the lock */ > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > l2cap_chan_close(chan, ECONNRESET); > - read_lock(&conn->chan_lock); > + utex_lock(&conn->chan_lock); Something corrupted here? Best regards Andrei Emeltchenko > bh_unlock_sock(sk); > continue; > } > @@ -838,7 +838,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn) > bh_unlock_sock(sk); > } > > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > } > > /* Find socket with cid and source bdaddr. > @@ -903,7 +903,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > > sk = chan->sk; > > - write_lock_bh(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > > hci_conn_hold(conn->hcon); > > @@ -919,7 +919,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > l2cap_state_change(chan, BT_CONNECTED); > parent->sk_data_ready(parent, 0); > > - write_unlock_bh(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > > clean: > release_sock(parent); > @@ -954,7 +954,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) > if (conn->hcon->out && conn->hcon->type == LE_LINK) > smp_conn_security(conn, conn->hcon->pending_sec_level); > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > > list_for_each_entry(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > @@ -976,7 +976,7 @@ static void l2cap_conn_ready(struct l2cap_conn *conn) > bh_unlock_sock(sk); > } > > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > } > > /* Notify sockets that we cannot guaranty reliability anymore */ > @@ -986,7 +986,7 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) > > BT_DBG("conn %p", conn); > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > > list_for_each_entry(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > @@ -995,7 +995,7 @@ static void l2cap_conn_unreliable(struct l2cap_conn *conn, int err) > sk->sk_err = err; > } > > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > } > > static void l2cap_info_timeout(unsigned long arg) > @@ -1086,7 +1086,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) > conn->feat_mask = 0; > > spin_lock_init(&conn->lock); > - rwlock_init(&conn->chan_lock); > + mutex_init(&conn->chan_lock); > > INIT_LIST_HEAD(&conn->chan_l); > > @@ -1104,9 +1104,9 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon, u8 status) > > static inline void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan) > { > - write_lock_bh(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > __l2cap_chan_add(conn, chan); > - write_unlock_bh(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > } > > /* ---- Socket interface ---- */ > @@ -1771,7 +1771,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) > > BT_DBG("conn %p", conn); > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > list_for_each_entry(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > if (chan->chan_type != L2CAP_CHAN_RAW) > @@ -1787,7 +1787,7 @@ static void l2cap_raw_recv(struct l2cap_conn *conn, struct sk_buff *skb) > if (chan->ops->recv(chan->data, nskb)) > kfree_skb(nskb); > } > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > } > > /* ---- L2CAP signalling commands ---- */ > @@ -2587,11 +2587,11 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > > sk = chan->sk; > > - write_lock_bh(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > > /* Check if we already have channel with that dcid */ > if (__l2cap_get_chan_by_dcid(conn, scid)) { > - write_unlock_bh(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > sock_set_flag(sk, SOCK_ZAPPED); > chan->ops->close(chan->data); > goto response; > @@ -2637,7 +2637,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > status = L2CAP_CS_NO_INFO; > } > > - write_unlock_bh(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > > response: > release_sock(parent); > @@ -4474,7 +4474,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) > del_timer(&conn->security_timer); > } > > - read_lock(&conn->chan_lock); > + mutex_lock(&conn->chan_lock); > > list_for_each_entry(chan, &conn->chan_l, list) { > struct sock *sk = chan->sk; > @@ -4554,7 +4554,7 @@ static int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt) > bh_unlock_sock(sk); > } > > - read_unlock(&conn->chan_lock); > + mutex_unlock(&conn->chan_lock); > > return 0; > } > -- > 1.7.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore 2011-12-17 21:29 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 08/22] Bluetooth: Remove sk_backlog usage from L2CAP Gustavo F. Padovan @ 2011-12-19 9:53 ` Andrei Emeltchenko 1 sibling, 0 replies; 34+ messages in thread From: Andrei Emeltchenko @ 2011-12-19 9:53 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, On Sat, Dec 17, 2011 at 07:29:27PM -0200, Gustavo F. Padovan wrote: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > We now run everything in process context, so the mutex lock is the best > option. But in some places we still need the bh_lock_sock() minor comment: {lock,release}_sock Best regards Andrei Emeltchenko > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > net/bluetooth/l2cap_core.c | 66 +++++++++++++------------------------------ > 1 files changed, 20 insertions(+), 46 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 89cda6d..ed67ac0 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -118,7 +118,7 @@ static struct l2cap_chan *l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 ci > read_lock(&conn->chan_lock); > c = __l2cap_get_chan_by_scid(conn, cid); > if (c) > - bh_lock_sock(c->sk); > + lock_sock(c->sk); > read_unlock(&conn->chan_lock); > return c; > } > @@ -141,7 +141,7 @@ static inline struct l2cap_chan *l2cap_get_chan_by_ident(struct l2cap_conn *conn > read_lock(&conn->chan_lock); > c = __l2cap_get_chan_by_ident(conn, ident); > if (c) > - bh_lock_sock(c->sk); > + lock_sock(c->sk); > read_unlock(&conn->chan_lock); > return c; > } > @@ -889,7 +889,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > > parent = pchan->sk; > > - bh_lock_sock(parent); > + lock_sock(parent); > > /* Check for backlog size */ > if (sk_acceptq_is_full(parent)) { > @@ -922,7 +922,7 @@ static void l2cap_le_conn_ready(struct l2cap_conn *conn) > write_unlock_bh(&conn->chan_lock); > > clean: > - bh_unlock_sock(parent); > + release_sock(parent); > } > > static void l2cap_chan_ready(struct sock *sk) > @@ -1024,9 +1024,9 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err) > /* Kill channels */ > list_for_each_entry_safe(chan, l, &conn->chan_l, list) { > sk = chan->sk; > - bh_lock_sock(sk); > + lock_sock(sk); > l2cap_chan_del(chan, err); > - bh_unlock_sock(sk); > + release_sock(sk); > chan->ops->close(chan->data); > } > > @@ -2568,7 +2568,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > > parent = pchan->sk; > > - bh_lock_sock(parent); > + lock_sock(parent); > > /* Check if the ACL is secure enough (if not SDP) */ > if (psm != cpu_to_le16(0x0001) && > @@ -2645,7 +2645,7 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd > write_unlock_bh(&conn->chan_lock); > > response: > - bh_unlock_sock(parent); > + release_sock(parent); > > sendresp: > rsp.scid = cpu_to_le16(scid); > @@ -2727,19 +2727,11 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd > break; > > default: > - /* don't delete l2cap channel if sk is owned by user */ > - if (sock_owned_by_user(sk)) { > - l2cap_state_change(chan, BT_DISCONN); > - __clear_chan_timer(chan); > - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); > - break; > - } > - > l2cap_chan_del(chan, ECONNREFUSED); > break; > } > > - bh_unlock_sock(sk); > + release_sock(sk); > return 0; > } > > @@ -2861,7 +2853,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr > } > > unlock: > - bh_unlock_sock(sk); > + release_sock(sk); > return 0; > } > > @@ -2968,7 +2960,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr > } > > done: > - bh_unlock_sock(sk); > + release_sock(sk); > return 0; > } > > @@ -2997,17 +2989,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd > > sk->sk_shutdown = SHUTDOWN_MASK; > > - /* don't delete l2cap channel if sk is owned by user */ > - if (sock_owned_by_user(sk)) { > - l2cap_state_change(chan, BT_DISCONN); > - __clear_chan_timer(chan); > - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); > - bh_unlock_sock(sk); > - return 0; > - } > - > l2cap_chan_del(chan, ECONNRESET); > - bh_unlock_sock(sk); > + release_sock(sk); > > chan->ops->close(chan->data); > return 0; > @@ -3031,17 +3014,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd > > sk = chan->sk; > > - /* don't delete l2cap channel if sk is owned by user */ > - if (sock_owned_by_user(sk)) { > - l2cap_state_change(chan, BT_DISCONN); > - __clear_chan_timer(chan); > - __set_chan_timer(chan, L2CAP_DISC_TIMEOUT); > - bh_unlock_sock(sk); > - return 0; > - } > - > l2cap_chan_del(chan, 0); > - bh_unlock_sock(sk); > + release_sock(sk); > > chan->ops->close(chan->data); > return 0; > @@ -4284,7 +4258,7 @@ drop: > > done: > if (sk) > - bh_unlock_sock(sk); > + release_sock(sk); > > return 0; > } > @@ -4300,7 +4274,7 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str > > sk = chan->sk; > > - bh_lock_sock(sk); > + lock_sock(sk); > > BT_DBG("sk %p, len %d", sk, skb->len); > > @@ -4318,7 +4292,7 @@ drop: > > done: > if (sk) > - bh_unlock_sock(sk); > + release_sock(sk); > return 0; > } > > @@ -4333,7 +4307,7 @@ static inline int l2cap_att_channel(struct l2cap_conn *conn, __le16 cid, struct > > sk = chan->sk; > > - bh_lock_sock(sk); > + lock_sock(sk); > > BT_DBG("sk %p, len %d", sk, skb->len); > > @@ -4351,7 +4325,7 @@ drop: > > done: > if (sk) > - bh_unlock_sock(sk); > + release_sock(sk); > return 0; > } > > @@ -4656,11 +4630,11 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl > BT_ERR("Frame exceeding recv MTU (len %d, " > "MTU %d)", len, > chan->imtu); > - bh_unlock_sock(sk); > + release_sock(sk); > l2cap_conn_unreliable(conn, ECOMM); > goto drop; > } > - bh_unlock_sock(sk); > + release_sock(sk); > } > > /* Allocate skb for the complete frame (with header) */ > -- > 1.7.6.4 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue 2011-12-17 21:29 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Gustavo F. Padovan @ 2011-12-19 11:05 ` Andrei Emeltchenko 2011-12-19 12:59 ` Ulisses Furquim 1 sibling, 1 reply; 34+ messages in thread From: Andrei Emeltchenko @ 2011-12-19 11:05 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, On Sat, Dec 17, 2011 at 07:29:26PM -0200, Gustavo F. Padovan wrote: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > L2CAP timers also need to run in process context. As the works in l2cap > are small we are using the system worqueue. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > include/net/bluetooth/l2cap.h | 17 +++++----- > net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++----------------------- > 2 files changed, 40 insertions(+), 47 deletions(-) ... > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 0369a9b..89cda6d 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -213,20 +213,18 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn) > return 0; > } > > -static void l2cap_set_timer(struct l2cap_chan *chan, struct timer_list *timer, long timeout) > +static void l2cap_set_timer(struct l2cap_chan *chan, struct delayed_work *work, long timeout) > { > BT_DBG("chan %p state %d timeout %ld", chan, chan->state, timeout); > > - if (!mod_timer(timer, jiffies + msecs_to_jiffies(timeout))) > - chan_hold(chan); > + cancel_delayed_work_sync(work); > + > + schedule_delayed_work(work, timeout); > } > > -static void l2cap_clear_timer(struct l2cap_chan *chan, struct timer_list *timer) > +static void l2cap_clear_timer(struct delayed_work *work) > { > - BT_DBG("chan %p state %d", chan, chan->state); > - > - if (timer_pending(timer) && del_timer(timer)) > - chan_put(chan); > + cancel_delayed_work_sync(work); > } Do you think we do not need to use chan_hold / chan_put? Best regards Andrei Emeltchenko ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue 2011-12-19 11:05 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Andrei Emeltchenko @ 2011-12-19 12:59 ` Ulisses Furquim 0 siblings, 0 replies; 34+ messages in thread From: Ulisses Furquim @ 2011-12-19 12:59 UTC (permalink / raw) To: Andrei Emeltchenko, Gustavo F. Padovan, linux-bluetooth Hi Andrei, On Mon, Dec 19, 2011 at 9:05 AM, Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> wrote: > Hi Gustavo, > > On Sat, Dec 17, 2011 at 07:29:26PM -0200, Gustavo F. Padovan wrote: >> From: "Gustavo F. Padovan" <padovan@profusion.mobi> >> >> L2CAP timers also need to run in process context. As the works in l2cap >> are small we are using the system worqueue. >> >> Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> >> --- >> include/net/bluetooth/l2cap.h | 17 +++++----- >> net/bluetooth/l2cap_core.c | 70 ++++++++++++++++++----------------------- >> 2 files changed, 40 insertions(+), 47 deletions(-) > > ... > >> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c >> index 0369a9b..89cda6d 100644 >> --- a/net/bluetooth/l2cap_core.c >> +++ b/net/bluetooth/l2cap_core.c >> @@ -213,20 +213,18 @@ static u16 l2cap_alloc_cid(struct l2cap_conn *conn) >> return 0; >> } >> >> -static void l2cap_set_timer(struct l2cap_chan *chan, struct timer_list *timer, long timeout) >> +static void l2cap_set_timer(struct l2cap_chan *chan, struct delayed_work *work, long timeout) >> { >> BT_DBG("chan %p state %d timeout %ld", chan, chan->state, timeout); >> >> - if (!mod_timer(timer, jiffies + msecs_to_jiffies(timeout))) >> - chan_hold(chan); >> + cancel_delayed_work_sync(work); >> + >> + schedule_delayed_work(work, timeout); >> } >> >> -static void l2cap_clear_timer(struct l2cap_chan *chan, struct timer_list *timer) >> +static void l2cap_clear_timer(struct delayed_work *work) >> { >> - BT_DBG("chan %p state %d", chan, chan->state); >> - >> - if (timer_pending(timer) && del_timer(timer)) >> - chan_put(chan); >> + cancel_delayed_work_sync(work); >> } > > > Do you think we do not need to use chan_hold / chan_put? It's good you spotted it too. It's a known issue Padovan is looking at. Thanks. Best regards, -- Ulisses Furquim ProFUSION embedded systems http://profusion.mobi Mobile: +55 19 9250 0942 Skype: ulissesffs ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 00/22] Bluetooth: change tasklets to workqueue 2011-12-17 21:29 [RFC 00/22] Bluetooth: change tasklets to workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 01/22] Bluetooth: Process recv path in a workqueue instead of a tasklet Gustavo F. Padovan @ 2011-12-17 21:34 ` Gustavo Padovan 2011-12-17 22:17 ` Marcel Holtmann 2 siblings, 0 replies; 34+ messages in thread From: Gustavo Padovan @ 2011-12-17 21:34 UTC (permalink / raw) To: linux-bluetooth * Gustavo F. Padovan <padovan@profusion.mobi> [2011-12-17 19:29:20 -0200]: > From: "Gustavo F. Padovan" <padovan@profusion.mobi> > > Here are the patches to run the receive path in workqueue. With these patches > l2cap and rfcomm seems to be working fine, but some more work still needs to be > done. You can also check the ongoing work at http://git.kernel.org/?p=linux/kernel/git/padovan/bluetooth-testing.git Gustavo ^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [RFC 00/22] Bluetooth: change tasklets to workqueue 2011-12-17 21:29 [RFC 00/22] Bluetooth: change tasklets to workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 01/22] Bluetooth: Process recv path in a workqueue instead of a tasklet Gustavo F. Padovan 2011-12-17 21:34 ` [RFC 00/22] Bluetooth: change tasklets " Gustavo Padovan @ 2011-12-17 22:17 ` Marcel Holtmann 2 siblings, 0 replies; 34+ messages in thread From: Marcel Holtmann @ 2011-12-17 22:17 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Hi Gustavo, > Here are the patches to run the receive path in workqueue. With these patches > l2cap and rfcomm seems to be working fine, but some more work still needs to be > done. so besides my two comments, this looks fine to me. Acked-by: Marcel Holtmann <marcel@holtmann.org> Regards Marcel ^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2012-01-26 13:20 UTC | newest] Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-12-17 21:29 [RFC 00/22] Bluetooth: change tasklets to workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 01/22] Bluetooth: Process recv path in a workqueue instead of a tasklet Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 02/22] Bluetooth: Replace spin_lock by mutex in hci_dev Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 03/22] Bluetooth: Use delayed_work for connection timeout Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 04/22] Bluetooth: Use delayed work for advertisiment cache timeout Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 05/22] Bluetooth: hci_conn_auto_accept() doesn't need locking Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 08/22] Bluetooth: Remove sk_backlog usage from L2CAP Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 09/22] Bluetooth: move hci_task_lock to mutex Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 10/22] Bluetooth: convert chan_lock " Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 12/22] Bluetooth: convert conn hash to RCU Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 13/22] Bluetooth: Don't disable tasklets to call hdev->notify() Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 14/22] Bluetooth: Move command task to workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 15/22] Bluetooth: convert tx_task " Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 16/22] Bluetooth: convert info timer to delayed_work Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 17/22] Bluetooth: remove power_on work_struct Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 18/22] Bluetooth: invert locking order in connect path Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 20/22] Bluetooth: move power_off to system workqueue Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 21/22] Bluetooth: Use new alloc_workqueue() Gustavo F. Padovan 2011-12-17 21:29 ` [RFC 22/22] Bluetooth: Remove work_add and work_del from hci_sysfs Gustavo F. Padovan 2011-12-17 22:15 ` [RFC 19/22] Bluetooth: Change l2cap chan_list to use RCU Marcel Holtmann 2011-12-19 10:42 ` Andrei Emeltchenko 2011-12-19 13:53 ` Gustavo Padovan 2011-12-17 22:13 ` [RFC 17/22] Bluetooth: remove power_on work_struct Marcel Holtmann 2012-01-26 13:20 ` [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list Andrei Emeltchenko 2011-12-19 9:58 ` [RFC 10/22] Bluetooth: convert chan_lock to mutex Andrei Emeltchenko 2011-12-19 9:53 ` [RFC 07/22] Bluetooth: Don't use spin_lock socket lock anymore Andrei Emeltchenko 2011-12-19 11:05 ` [RFC 06/22] Bluetooth: Move L2CAP timers to workqueue Andrei Emeltchenko 2011-12-19 12:59 ` Ulisses Furquim 2011-12-17 21:34 ` [RFC 00/22] Bluetooth: change tasklets " Gustavo Padovan 2011-12-17 22:17 ` Marcel Holtmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.