2011-12-22 18:56:02

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 01/12] Bluetooth: convert SCO socket timeout function to workqueue

From: "Gustavo F. Padovan" <[email protected]>

As a consequence of the move of the Bluetooth core to the process context
we need all important timer to process context too. Thus we will not need
to deal with different context inside the Bluetooth core anymore.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/sco.h | 1 +
net/bluetooth/sco.c | 35 +++++++++++++++++++++--------------
2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index 1e35c43..fc1cf9d 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -74,6 +74,7 @@ struct sco_pinfo {
struct bt_sock bt;
__u32 flags;
struct sco_conn *conn;
+ struct delayed_work timer;
};

#endif /* __SCO_H */
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 0d59e61..8057a4b 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -68,9 +68,10 @@ static void sco_sock_close(struct sock *sk);
static void sco_sock_kill(struct sock *sk);

/* ---- SCO timers ---- */
-static void sco_sock_timeout(unsigned long arg)
+static void sco_sock_timeout(struct work_struct *work)
{
- struct sock *sk = (struct sock *) arg;
+ struct sock *sk = (void *)container_of(work, struct sco_pinfo,
+ timer.work);

BT_DBG("sock %p state %d", sk, sk->sk_state);

@@ -83,16 +84,21 @@ static void sco_sock_timeout(unsigned long arg)
sock_put(sk);
}

-static void sco_sock_set_timer(struct sock *sk, long timeout)
+static inline void sco_set_timer(struct sock *sk, long timeout)
{
BT_DBG("sock %p state %d timeout %ld", sk, sk->sk_state, timeout);
- sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
+
+ if (!__cancel_delayed_work(&sco_pi(sk)->timer))
+ sock_hold(sk);
+ schedule_delayed_work(&sco_pi(sk)->timer, timeout);
}

-static void sco_sock_clear_timer(struct sock *sk)
+static inline void sco_clear_timer(struct sock *sk)
{
BT_DBG("sock %p state %d", sk, sk->sk_state);
- sk_stop_timer(sk, &sk->sk_timer);
+
+ if (__cancel_delayed_work(&sco_pi(sk)->timer))
+ sock_put(sk);
}

/* ---- SCO connections ---- */
@@ -149,7 +155,7 @@ static int sco_conn_del(struct hci_conn *hcon, int err)
sk = sco_chan_get(conn);
if (sk) {
bh_lock_sock(sk);
- sco_sock_clear_timer(sk);
+ sco_clear_timer(sk);
sco_chan_del(sk, err);
bh_unlock_sock(sk);
sco_sock_kill(sk);
@@ -217,11 +223,11 @@ static int sco_connect(struct sock *sk)
goto done;

if (hcon->state == BT_CONNECTED) {
- sco_sock_clear_timer(sk);
+ sco_clear_timer(sk);
sk->sk_state = BT_CONNECTED;
} else {
sk->sk_state = BT_CONNECT;
- sco_sock_set_timer(sk, sk->sk_sndtimeo);
+ sco_set_timer(sk, sk->sk_sndtimeo);
}

done:
@@ -372,7 +378,8 @@ static void __sco_sock_close(struct sock *sk)
case BT_CONFIG:
if (sco_pi(sk)->conn) {
sk->sk_state = BT_DISCONN;
- sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT);
+ sco_set_timer(sk,
+ msecs_to_jiffies(SCO_DISCONN_TIMEOUT));
hci_conn_put(sco_pi(sk)->conn->hcon);
sco_pi(sk)->conn->hcon = NULL;
} else
@@ -393,7 +400,7 @@ static void __sco_sock_close(struct sock *sk)
/* Must be called on unlocked socket. */
static void sco_sock_close(struct sock *sk)
{
- sco_sock_clear_timer(sk);
+ sco_clear_timer(sk);
lock_sock(sk);
__sco_sock_close(sk);
release_sock(sk);
@@ -435,7 +442,7 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock, int pro
sk->sk_protocol = proto;
sk->sk_state = BT_OPEN;

- setup_timer(&sk->sk_timer, sco_sock_timeout, (unsigned long)sk);
+ INIT_DELAYED_WORK(&sco_pi(sk)->timer, sco_sock_timeout);

bt_sock_link(&sco_sk_list, sk);
return sk;
@@ -771,7 +778,7 @@ static int sco_sock_shutdown(struct socket *sock, int how)
lock_sock(sk);
if (!sk->sk_shutdown) {
sk->sk_shutdown = SHUTDOWN_MASK;
- sco_sock_clear_timer(sk);
+ sco_clear_timer(sk);
__sco_sock_close(sk);

if (sock_flag(sk, SOCK_LINGER) && sk->sk_lingertime)
@@ -853,7 +860,7 @@ static void sco_conn_ready(struct sco_conn *conn)
sco_conn_lock(conn);

if (sk) {
- sco_sock_clear_timer(sk);
+ sco_clear_timer(sk);
bh_lock_sock(sk);
sk->sk_state = BT_CONNECTED;
sk->sk_state_change(sk);
--
1.7.6.4



2011-12-27 17:37:25

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 05/12] Bluetooth: Don't use *_bh locks anymore

Hi Marcel,

* Marcel Holtmann <[email protected]> [2011-12-22 11:49:18 -0800]:

> Hi Gustavo,
>
> > Those locks are not shared between interrupt and process context anymore,
> > so remove the part that disable interruption. We are stiff safe because
> > preeption is disabled.
> >
> > Signed-off-by: Gustavo F. Padovan <[email protected]>
> > ---
> > net/bluetooth/l2cap_core.c | 24 ++++++++++++------------
> > 1 files changed, 12 insertions(+), 12 deletions(-)
>
> any reason why this can not be converted into RCU usage?

Yes, when the element is added to the list its psm and cid both equals to 0.
In a following call the psm or cid is set, but at this point, we can't let a
reader read an old version of these value, this may cause two socket to
succeed on trying to connect to the same psm for example. spin lock is the
only safe choice here I think.

Gustavo

2011-12-22 19:54:41

by Ulisses Furquim

[permalink] [raw]
Subject: Re: [PATCH 05/12] Bluetooth: Don't use *_bh locks anymore

Hi Marcel,

On Thu, Dec 22, 2011 at 5:49 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Gustavo,
>
>> Those locks are not shared between interrupt and process context anymore=
,
>> so remove the part that disable interruption. We are stiff safe because
>> preeption is disabled.
>>
>> Signed-off-by: Gustavo F. Padovan <[email protected]>
>> ---
>> =A0net/bluetooth/l2cap_core.c | =A0 24 ++++++++++++------------
>> =A01 files changed, 12 insertions(+), 12 deletions(-)
>
> any reason why this can not be converted into RCU usage?

Indeed the chan_list_lock may be easily converted to RCU, IMO.

Regards,

--=20
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs

2011-12-22 19:49:18

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 05/12] Bluetooth: Don't use *_bh locks anymore

Hi Gustavo,

> Those locks are not shared between interrupt and process context anymore,
> so remove the part that disable interruption. We are stiff safe because
> preeption is disabled.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 24 ++++++++++++------------
> 1 files changed, 12 insertions(+), 12 deletions(-)

any reason why this can not be converted into RCU usage?

Regards

Marcel



2011-12-22 19:49:39

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 09/12] Bluetooth: remove *_bh usage from hci_dev_list and hci_cb_list

Hi Marcel,

* Marcel Holtmann <[email protected]> [2011-12-22 11:45:21 -0800]:

> Hi Gustavo,
>
> > They don't need to disable interruption anymore, we only run in interrupt
> > context now.
>
> I assume you mean process context here.

Yes. I left some typos in the messages. Ulisses helped me here to fix all of
them.

Gustavo

2011-12-22 19:48:13

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 12/12] Bluetooth: Don't disable interruptions on RFCOMM

Hi Gustavo,

> As everything is now process context, disable interruptions is now
> pointless.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/rfcomm/sock.c | 12 ++++++------
> net/bluetooth/rfcomm/tty.c | 16 ++++++++--------
> 2 files changed, 14 insertions(+), 14 deletions(-)

are you sure here? Have you considered TTY callers?

Regards

Marcel



2011-12-22 19:47:12

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 10/12] Bluetooth: Don't disable interrupt when locking the queue

Hi Gustavo,

> We run everything in iterrupt context now.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/hci_core.c | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-22 19:45:21

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 09/12] Bluetooth: remove *_bh usage from hci_dev_list and hci_cb_list

Hi Gustavo,

> They don't need to disable interruption anymore, we only run in interrupt
> context now.

I assume you mean process context here.

>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 16 ++++++++--------
> net/bluetooth/hci_conn.c | 4 ++--
> net/bluetooth/hci_core.c | 24 ++++++++++++------------
> 3 files changed, 22 insertions(+), 22 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-22 19:44:17

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 08/12] Bluetooth: Remove local_bh_disable() from hci_sock.c

Hi Gustavo,

> Everything is in process context now, we do not need such a call.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/hci_sock.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)

this should hold true even without any of the other preceding patches.
Or did I overlook something?

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-22 19:43:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 07/12] Bluetooth: Remove lock from inquiry_cache

Hi Gustavo,

> It was never used, so removing it.
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> include/net/bluetooth/hci_core.h | 7 -------
> 1 files changed, 0 insertions(+), 7 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-22 19:42:49

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 04/12] Bluetooth: add debug output to l2cap_ack_timeout()

Hi Gustavo,

> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/l2cap_core.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)

Acked-by: Marcel Holtmann <[email protected]>

Regards

Marcel



2011-12-22 18:56:13

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 12/12] Bluetooth: Don't disable interruptions on RFCOMM

From: "Gustavo F. Padovan" <[email protected]>

As everything is now process context, disable interruptions is now
pointless.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/rfcomm/sock.c | 12 ++++++------
net/bluetooth/rfcomm/tty.c | 16 ++++++++--------
2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index aea2bdd..f066678 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -370,7 +370,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
goto done;
}

- write_lock_bh(&rfcomm_sk_list.lock);
+ write_lock(&rfcomm_sk_list.lock);

if (sa->rc_channel && __rfcomm_get_sock_by_addr(sa->rc_channel, &sa->rc_bdaddr)) {
err = -EADDRINUSE;
@@ -381,7 +381,7 @@ static int rfcomm_sock_bind(struct socket *sock, struct sockaddr *addr, int addr
sk->sk_state = BT_BOUND;
}

- write_unlock_bh(&rfcomm_sk_list.lock);
+ write_unlock(&rfcomm_sk_list.lock);

done:
release_sock(sk);
@@ -455,7 +455,7 @@ static int rfcomm_sock_listen(struct socket *sock, int backlog)

err = -EINVAL;

- write_lock_bh(&rfcomm_sk_list.lock);
+ write_lock(&rfcomm_sk_list.lock);

for (channel = 1; channel < 31; channel++)
if (!__rfcomm_get_sock_by_addr(channel, src)) {
@@ -464,7 +464,7 @@ static int rfcomm_sock_listen(struct socket *sock, int backlog)
break;
}

- write_unlock_bh(&rfcomm_sk_list.lock);
+ write_unlock(&rfcomm_sk_list.lock);

if (err < 0)
goto done;
@@ -982,7 +982,7 @@ static int rfcomm_sock_debugfs_show(struct seq_file *f, void *p)
struct sock *sk;
struct hlist_node *node;

- read_lock_bh(&rfcomm_sk_list.lock);
+ read_lock(&rfcomm_sk_list.lock);

sk_for_each(sk, node, &rfcomm_sk_list.head) {
seq_printf(f, "%s %s %d %d\n",
@@ -991,7 +991,7 @@ static int rfcomm_sock_debugfs_show(struct seq_file *f, void *p)
sk->sk_state, rfcomm_pi(sk)->channel);
}

- read_unlock_bh(&rfcomm_sk_list.lock);
+ read_unlock(&rfcomm_sk_list.lock);

return 0;
}
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index fa8f4de5..6583f1c 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -205,7 +205,7 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
if (!dev)
return -ENOMEM;

- write_lock_bh(&rfcomm_dev_lock);
+ write_lock(&rfcomm_dev_lock);

if (req->dev_id < 0) {
dev->id = 0;
@@ -290,7 +290,7 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
__module_get(THIS_MODULE);

out:
- write_unlock_bh(&rfcomm_dev_lock);
+ write_unlock(&rfcomm_dev_lock);

if (err < 0)
goto free;
@@ -327,9 +327,9 @@ static void rfcomm_dev_del(struct rfcomm_dev *dev)
if (atomic_read(&dev->opened) > 0)
return;

- write_lock_bh(&rfcomm_dev_lock);
+ write_lock(&rfcomm_dev_lock);
list_del_init(&dev->list);
- write_unlock_bh(&rfcomm_dev_lock);
+ write_unlock(&rfcomm_dev_lock);

rfcomm_dev_put(dev);
}
@@ -473,7 +473,7 @@ static int rfcomm_get_dev_list(void __user *arg)

di = dl->dev_info;

- read_lock_bh(&rfcomm_dev_lock);
+ read_lock(&rfcomm_dev_lock);

list_for_each_entry(dev, &rfcomm_dev_list, list) {
if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
@@ -488,7 +488,7 @@ static int rfcomm_get_dev_list(void __user *arg)
break;
}

- read_unlock_bh(&rfcomm_dev_lock);
+ read_unlock(&rfcomm_dev_lock);

dl->dev_num = n;
size = sizeof(*dl) + n * sizeof(*di);
@@ -766,9 +766,9 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
rfcomm_dlc_unlock(dev->dlc);

if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags)) {
- write_lock_bh(&rfcomm_dev_lock);
+ write_lock(&rfcomm_dev_lock);
list_del_init(&dev->list);
- write_unlock_bh(&rfcomm_dev_lock);
+ write_unlock(&rfcomm_dev_lock);

rfcomm_dev_put(dev);
}
--
1.7.6.4


2011-12-22 18:56:12

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 11/12] Bluetooth: Remove *_bh from socket list lock.

From: "Gustavo F. Padovan" <[email protected]>

We don't need it anymore.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/af_bluetooth.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index cdcfcab..ef92864 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -156,17 +156,17 @@ static int bt_sock_create(struct net *net, struct socket *sock, int proto,

void bt_sock_link(struct bt_sock_list *l, struct sock *sk)
{
- write_lock_bh(&l->lock);
+ write_lock(&l->lock);
sk_add_node(sk, &l->head);
- write_unlock_bh(&l->lock);
+ write_unlock(&l->lock);
}
EXPORT_SYMBOL(bt_sock_link);

void bt_sock_unlink(struct bt_sock_list *l, struct sock *sk)
{
- write_lock_bh(&l->lock);
+ write_lock(&l->lock);
sk_del_node_init(sk);
- write_unlock_bh(&l->lock);
+ write_unlock(&l->lock);
}
EXPORT_SYMBOL(bt_sock_unlink);

--
1.7.6.4


2011-12-22 18:56:11

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 10/12] Bluetooth: Don't disable interrupt when locking the queue

From: "Gustavo F. Padovan" <[email protected]>

We run everything in iterrupt context now.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/hci_core.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 4f0ff01..6d38d80 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1960,7 +1960,7 @@ static void hci_queue_acl(struct hci_conn *conn, struct sk_buff_head *queue,
skb_shinfo(skb)->frag_list = NULL;

/* Queue all fragments atomically */
- spin_lock_bh(&queue->lock);
+ spin_lock(&queue->lock);

__skb_queue_tail(queue, skb);

@@ -1978,7 +1978,7 @@ static void hci_queue_acl(struct hci_conn *conn, struct sk_buff_head *queue,
__skb_queue_tail(queue, skb);
} while (list);

- spin_unlock_bh(&queue->lock);
+ spin_unlock(&queue->lock);
}
}

--
1.7.6.4


2011-12-22 18:56:10

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 09/12] Bluetooth: remove *_bh usage from hci_dev_list and hci_cb_list

From: "Gustavo F. Padovan" <[email protected]>

They don't need to disable interruption anymore, we only run in interrupt
context now.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/hci_core.h | 16 ++++++++--------
net/bluetooth/hci_conn.c | 4 ++--
net/bluetooth/hci_core.c | 24 ++++++++++++------------
3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 37d7cb8..5e2e984 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -801,13 +801,13 @@ static inline void hci_auth_cfm(struct hci_conn *conn, __u8 status)

encrypt = (conn->link_mode & HCI_LM_ENCRYPT) ? 0x01 : 0x00;

- read_lock_bh(&hci_cb_list_lock);
+ read_lock(&hci_cb_list_lock);
list_for_each(p, &hci_cb_list) {
struct hci_cb *cb = list_entry(p, struct hci_cb, list);
if (cb->security_cfm)
cb->security_cfm(conn, status, encrypt);
}
- read_unlock_bh(&hci_cb_list_lock);
+ read_unlock(&hci_cb_list_lock);
}

static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status,
@@ -823,26 +823,26 @@ static inline void hci_encrypt_cfm(struct hci_conn *conn, __u8 status,

hci_proto_encrypt_cfm(conn, status, encrypt);

- read_lock_bh(&hci_cb_list_lock);
+ read_lock(&hci_cb_list_lock);
list_for_each(p, &hci_cb_list) {
struct hci_cb *cb = list_entry(p, struct hci_cb, list);
if (cb->security_cfm)
cb->security_cfm(conn, status, encrypt);
}
- read_unlock_bh(&hci_cb_list_lock);
+ read_unlock(&hci_cb_list_lock);
}

static inline void hci_key_change_cfm(struct hci_conn *conn, __u8 status)
{
struct list_head *p;

- read_lock_bh(&hci_cb_list_lock);
+ read_lock(&hci_cb_list_lock);
list_for_each(p, &hci_cb_list) {
struct hci_cb *cb = list_entry(p, struct hci_cb, list);
if (cb->key_change_cfm)
cb->key_change_cfm(conn, status);
}
- read_unlock_bh(&hci_cb_list_lock);
+ read_unlock(&hci_cb_list_lock);
}

static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
@@ -850,13 +850,13 @@ static inline void hci_role_switch_cfm(struct hci_conn *conn, __u8 status,
{
struct list_head *p;

- read_lock_bh(&hci_cb_list_lock);
+ read_lock(&hci_cb_list_lock);
list_for_each(p, &hci_cb_list) {
struct hci_cb *cb = list_entry(p, struct hci_cb, list);
if (cb->role_switch_cfm)
cb->role_switch_cfm(conn, status, role);
}
- read_unlock_bh(&hci_cb_list_lock);
+ read_unlock(&hci_cb_list_lock);
}

int hci_register_cb(struct hci_cb *hcb);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 401d8ea..3db4324 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -487,7 +487,7 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)

BT_DBG("%s -> %s", batostr(src), batostr(dst));

- read_lock_bh(&hci_dev_list_lock);
+ read_lock(&hci_dev_list_lock);

list_for_each_entry(d, &hci_dev_list, list) {
if (!test_bit(HCI_UP, &d->flags) || test_bit(HCI_RAW, &d->flags))
@@ -512,7 +512,7 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src)
if (hdev)
hdev = hci_dev_hold(hdev);

- read_unlock_bh(&hci_dev_list_lock);
+ read_unlock(&hci_dev_list_lock);
return hdev;
}
EXPORT_SYMBOL(hci_get_route);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 22c8331..4f0ff01 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -844,7 +844,7 @@ int hci_get_dev_list(void __user *arg)

dr = dl->dev_req;

- read_lock_bh(&hci_dev_list_lock);
+ read_lock(&hci_dev_list_lock);
list_for_each_entry(hdev, &hci_dev_list, list) {
if (test_and_clear_bit(HCI_AUTO_OFF, &hdev->flags))
cancel_delayed_work(&hdev->power_off);
@@ -858,7 +858,7 @@ int hci_get_dev_list(void __user *arg)
if (++n >= dev_num)
break;
}
- read_unlock_bh(&hci_dev_list_lock);
+ read_unlock(&hci_dev_list_lock);

dl->dev_num = n;
size = sizeof(*dl) + n * sizeof(*dr);
@@ -1458,7 +1458,7 @@ int hci_register_dev(struct hci_dev *hdev)
*/
id = (hdev->dev_type == HCI_BREDR) ? 0 : 1;

- write_lock_bh(&hci_dev_list_lock);
+ write_lock(&hci_dev_list_lock);

/* Find first available device id */
list_for_each(p, &hci_dev_list) {
@@ -1528,7 +1528,7 @@ int hci_register_dev(struct hci_dev *hdev)

atomic_set(&hdev->promisc, 0);

- write_unlock_bh(&hci_dev_list_lock);
+ write_unlock(&hci_dev_list_lock);

hdev->workqueue = alloc_workqueue(hdev->name, WQ_HIGHPRI | WQ_UNBOUND |
WQ_MEM_RECLAIM, 1);
@@ -1561,9 +1561,9 @@ int hci_register_dev(struct hci_dev *hdev)
err_wqueue:
destroy_workqueue(hdev->workqueue);
err:
- write_lock_bh(&hci_dev_list_lock);
+ write_lock(&hci_dev_list_lock);
list_del(&hdev->list);
- write_unlock_bh(&hci_dev_list_lock);
+ write_unlock(&hci_dev_list_lock);

return error;
}
@@ -1576,9 +1576,9 @@ void hci_unregister_dev(struct hci_dev *hdev)

BT_DBG("%p name %s bus %d", hdev, hdev->name, hdev->bus);

- write_lock_bh(&hci_dev_list_lock);
+ write_lock(&hci_dev_list_lock);
list_del(&hdev->list);
- write_unlock_bh(&hci_dev_list_lock);
+ write_unlock(&hci_dev_list_lock);

hci_dev_do_close(hdev);

@@ -1830,9 +1830,9 @@ int hci_register_cb(struct hci_cb *cb)
{
BT_DBG("%p name %s", cb, cb->name);

- write_lock_bh(&hci_cb_list_lock);
+ write_lock(&hci_cb_list_lock);
list_add(&cb->list, &hci_cb_list);
- write_unlock_bh(&hci_cb_list_lock);
+ write_unlock(&hci_cb_list_lock);

return 0;
}
@@ -1842,9 +1842,9 @@ int hci_unregister_cb(struct hci_cb *cb)
{
BT_DBG("%p name %s", cb, cb->name);

- write_lock_bh(&hci_cb_list_lock);
+ write_lock(&hci_cb_list_lock);
list_del(&cb->list);
- write_unlock_bh(&hci_cb_list_lock);
+ write_unlock(&hci_cb_list_lock);

return 0;
}
--
1.7.6.4


2011-12-22 18:56:09

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 08/12] Bluetooth: Remove local_bh_disable() from hci_sock.c

From: "Gustavo F. Padovan" <[email protected]>

Everything is in process context now, we do not need such a call.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/hci_sock.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 189a667..896a72f 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -767,7 +767,6 @@ static int hci_sock_dev_event(struct notifier_block *this, unsigned long event,
/* Detach sockets from device */
read_lock(&hci_sk_list.lock);
sk_for_each(sk, node, &hci_sk_list.head) {
- local_bh_disable();
bh_lock_sock_nested(sk);
if (hci_pi(sk)->hdev == hdev) {
hci_pi(sk)->hdev = NULL;
@@ -778,7 +777,6 @@ static int hci_sock_dev_event(struct notifier_block *this, unsigned long event,
hci_dev_put(hdev);
}
bh_unlock_sock(sk);
- local_bh_enable();
}
read_unlock(&hci_sk_list.lock);
}
--
1.7.6.4


2011-12-22 18:56:08

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 07/12] Bluetooth: Remove lock from inquiry_cache

From: "Gustavo F. Padovan" <[email protected]>

It was never used, so removing it.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/hci_core.h | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 4ff08d6..37d7cb8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -50,7 +50,6 @@ struct inquiry_entry {
};

struct inquiry_cache {
- spinlock_t lock;
__u32 timestamp;
struct inquiry_entry *list;
};
@@ -349,15 +348,9 @@ extern int sco_recv_scodata(struct hci_conn *hcon, struct sk_buff *skb);
#define INQUIRY_CACHE_AGE_MAX (HZ*30) /* 30 seconds */
#define INQUIRY_ENTRY_AGE_MAX (HZ*60) /* 60 seconds */

-#define inquiry_cache_lock(c) spin_lock(&c->lock)
-#define inquiry_cache_unlock(c) spin_unlock(&c->lock)
-#define inquiry_cache_lock_bh(c) spin_lock_bh(&c->lock)
-#define inquiry_cache_unlock_bh(c) spin_unlock_bh(&c->lock)
-
static inline void inquiry_cache_init(struct hci_dev *hdev)
{
struct inquiry_cache *c = &hdev->inq_cache;
- spin_lock_init(&c->lock);
c->list = NULL;
}

--
1.7.6.4


2011-12-22 18:56:07

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 06/12] Bluetooth: Remove *_bh locks from SCO

From: "Gustavo F. Padovan" <[email protected]>

Those locks are not shared between interrupt and process context anymore,
so remove the part that disable interruption. We are stiff safe because
preeption is disabled.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/sco.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 90c72fa..c8afcdb 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -489,7 +489,7 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
goto done;
}

- write_lock_bh(&sco_sk_list.lock);
+ write_lock(&sco_sk_list.lock);

if (bacmp(src, BDADDR_ANY) && __sco_get_sock_by_addr(src)) {
err = -EADDRINUSE;
@@ -499,7 +499,7 @@ static int sco_sock_bind(struct socket *sock, struct sockaddr *addr, int addr_le
sk->sk_state = BT_BOUND;
}

- write_unlock_bh(&sco_sk_list.lock);
+ write_unlock(&sco_sk_list.lock);

done:
release_sock(sk);
@@ -972,14 +972,14 @@ static int sco_debugfs_show(struct seq_file *f, void *p)
struct sock *sk;
struct hlist_node *node;

- read_lock_bh(&sco_sk_list.lock);
+ read_lock(&sco_sk_list.lock);

sk_for_each(sk, node, &sco_sk_list.head) {
seq_printf(f, "%s %s %d\n", batostr(&bt_sk(sk)->src),
batostr(&bt_sk(sk)->dst), sk->sk_state);
}

- read_unlock_bh(&sco_sk_list.lock);
+ read_unlock(&sco_sk_list.lock);

return 0;
}
--
1.7.6.4


2011-12-22 18:56:06

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 05/12] Bluetooth: Don't use *_bh locks anymore

From: "Gustavo F. Padovan" <[email protected]>

Those locks are not shared between interrupt and process context anymore,
so remove the part that disable interruption. We are stiff safe because
preeption is disabled.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap_core.c | 24 ++++++++++++------------
1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index cd7bb3d..a1a50d2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -165,7 +165,7 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
{
int err;

- write_lock_bh(&chan_list_lock);
+ write_lock(&chan_list_lock);

if (psm && __l2cap_global_chan_by_addr(psm, src)) {
err = -EADDRINUSE;
@@ -190,17 +190,17 @@ int l2cap_add_psm(struct l2cap_chan *chan, bdaddr_t *src, __le16 psm)
}

done:
- write_unlock_bh(&chan_list_lock);
+ write_unlock(&chan_list_lock);
return err;
}

int l2cap_add_scid(struct l2cap_chan *chan, __u16 scid)
{
- write_lock_bh(&chan_list_lock);
+ write_lock(&chan_list_lock);

chan->scid = scid;

- write_unlock_bh(&chan_list_lock);
+ write_unlock(&chan_list_lock);

return 0;
}
@@ -289,9 +289,9 @@ struct l2cap_chan *l2cap_chan_create(struct sock *sk)

chan->sk = sk;

- write_lock_bh(&chan_list_lock);
+ write_lock(&chan_list_lock);
list_add(&chan->global_l, &chan_list);
- write_unlock_bh(&chan_list_lock);
+ write_unlock(&chan_list_lock);

INIT_DELAYED_WORK(&chan->chan_timer, l2cap_chan_timeout);

@@ -306,9 +306,9 @@ struct l2cap_chan *l2cap_chan_create(struct sock *sk)

void l2cap_chan_destroy(struct l2cap_chan *chan)
{
- write_lock_bh(&chan_list_lock);
+ write_lock(&chan_list_lock);
list_del(&chan->global_l);
- write_unlock_bh(&chan_list_lock);
+ write_unlock(&chan_list_lock);

l2cap_chan_put(chan);
}
@@ -543,14 +543,14 @@ static u8 l2cap_get_ident(struct l2cap_conn *conn)
* 200 - 254 are used by utilities like l2ping, etc.
*/

- spin_lock_bh(&conn->lock);
+ spin_lock(&conn->lock);

if (++conn->tx_ident > 128)
conn->tx_ident = 1;

id = conn->tx_ident;

- spin_unlock_bh(&conn->lock);
+ spin_unlock(&conn->lock);

return id;
}
@@ -4702,7 +4702,7 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
{
struct l2cap_chan *c;

- read_lock_bh(&chan_list_lock);
+ read_lock(&chan_list_lock);

list_for_each_entry(c, &chan_list, global_l) {
struct sock *sk = c->sk;
@@ -4715,7 +4715,7 @@ static int l2cap_debugfs_show(struct seq_file *f, void *p)
c->sec_level, c->mode);
}

- read_unlock_bh(&chan_list_lock);
+ read_unlock(&chan_list_lock);

return 0;
}
--
1.7.6.4


2011-12-22 18:56:05

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 04/12] Bluetooth: add debug output to l2cap_ack_timeout()

From: "Gustavo F. Padovan" <[email protected]>

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap_core.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 944c189..cd7bb3d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1985,6 +1985,8 @@ static void l2cap_ack_timeout(struct work_struct *work)
struct l2cap_chan *chan = container_of(work, struct l2cap_chan,
ack_timer.work);

+ BT_DBG("chan %p", chan);
+
lock_sock(chan->sk);
l2cap_send_ack(chan);
release_sock(chan->sk);
--
1.7.6.4


2011-12-22 18:56:04

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 03/12] Bluetooth: Use only mutex lock_sock() in SCO

From: "Gustavo F. Padovan" <[email protected]>

We are now working only in process context here. So it makes sense use
only mutex lock_sock().

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/sco.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 36fe9eb..90c72fa 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -75,10 +75,10 @@ static void sco_sock_timeout(struct work_struct *work)

BT_DBG("sock %p state %d", sk, sk->sk_state);

- bh_lock_sock(sk);
+ lock_sock(sk);
sk->sk_err = ETIMEDOUT;
sk->sk_state_change(sk);
- bh_unlock_sock(sk);
+ release_sock(sk);

sco_sock_kill(sk);
sock_put(sk);
@@ -154,10 +154,10 @@ static int sco_conn_del(struct hci_conn *hcon, int err)
/* Kill socket */
sk = sco_chan_get(conn);
if (sk) {
- bh_lock_sock(sk);
+ lock_sock(sk);
sco_clear_timer(sk);
sco_chan_del(sk, err);
- bh_unlock_sock(sk);
+ release_sock(sk);
sco_sock_kill(sk);
}

@@ -861,21 +861,21 @@ static void sco_conn_ready(struct sco_conn *conn)

if (sk) {
sco_clear_timer(sk);
- bh_lock_sock(sk);
+ lock_sock(sk);
sk->sk_state = BT_CONNECTED;
sk->sk_state_change(sk);
- bh_unlock_sock(sk);
+ release_sock(sk);
} else {
parent = sco_get_sock_listen(conn->src);
if (!parent)
goto done;

- bh_lock_sock(parent);
+ lock_sock(parent);

sk = sco_sock_alloc(sock_net(parent), NULL,
BTPROTO_SCO, GFP_ATOMIC);
if (!sk) {
- bh_unlock_sock(parent);
+ release_sock(parent);
goto done;
}

@@ -892,7 +892,7 @@ static void sco_conn_ready(struct sco_conn *conn)
/* Wake up parent */
parent->sk_data_ready(parent, 1);

- bh_unlock_sock(parent);
+ release_sock(parent);
}

done:
--
1.7.6.4


2011-12-22 18:56:03

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 02/12] Bluetooth: Change SCO conn lock to mutex

From: "Gustavo F. Padovan" <[email protected]>

As part of the moving to process context, we will need to change
bh_lock_sock() to lock_sock() in the SCO code, but first the SCO
connection lock need to be moved to mutex otherwise we can not use
lock_sock() in a region locked by it. (i.e., we can't sleep inside a
spinlock critical session)

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/sco.h | 6 +++---
net/bluetooth/sco.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/sco.h b/include/net/bluetooth/sco.h
index fc1cf9d..1bd0d62 100644
--- a/include/net/bluetooth/sco.h
+++ b/include/net/bluetooth/sco.h
@@ -58,14 +58,14 @@ struct sco_conn {
bdaddr_t *dst;
bdaddr_t *src;

- spinlock_t lock;
+ struct mutex lock;
struct sock *sk;

unsigned int mtu;
};

-#define sco_conn_lock(c) spin_lock(&c->lock);
-#define sco_conn_unlock(c) spin_unlock(&c->lock);
+#define sco_conn_lock(c) mutex_lock(&c->lock);
+#define sco_conn_unlock(c) mutex_unlock(&c->lock);

/* ----- SCO socket info ----- */
#define sco_pi(sk) ((struct sco_pinfo *) sk)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 8057a4b..36fe9eb 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -114,7 +114,7 @@ static struct sco_conn *sco_conn_add(struct hci_conn *hcon, __u8 status)
if (!conn)
return NULL;

- spin_lock_init(&conn->lock);
+ mutex_init(&conn->lock);

hcon->sco_data = conn;
conn->hcon = hcon;
--
1.7.6.4