Hi Gustavo,
On Thu, Feb 9, 2012 at 4:37 PM, Gustavo Padovan <padovan@xxxxxxxxxxxxxx> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@xxxxxxxxx> [2012-02-09 16:17:23 +0200]:
>
>> From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
>>
>> Usage of RCU list looks not reasonalbe for a number of reasons:
>> our code sleep and we have to use socket spinlocks, some parts
>> of code are updaters thus we need to use mutexes anyway.
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
>> ---
>> net/bluetooth/l2cap_core.c | 108 ++++++++++++++++++++++----------------------
>> 1 files changed, 54 insertions(+), 54 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index 8dfccb3..ae08944 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -77,36 +77,24 @@ static void l2cap_send_disconn_req(struct l2cap_conn *conn,
>>
>> static struct l2cap_chan *__l2cap_get_chan_by_dcid(struct l2cap_conn *conn, u16 cid)
>> {
>> - struct l2cap_chan *c, *r = NULL;
>> -
>> - rcu_read_lock();
>> + struct l2cap_chan *c;
>>
>> - list_for_each_entry_rcu(c, &conn->chan_l, list) {
>> - if (c->dcid == cid) {
>> - r = c;
>> - break;
>> - }
>> + list_for_each_entry(c, &conn->chan_l, list) {
>> + if (c->dcid == cid)
>> + return c;
>> }
>> -
>> - rcu_read_unlock();
>> - return r;
>> + return NULL;
>> }
>>
>> static struct l2cap_chan *__l2cap_get_chan_by_scid(struct l2cap_conn *conn, u16 cid)
>> {
>> - struct l2cap_chan *c, *r = NULL;
>> -
>> - rcu_read_lock();
>> + struct l2cap_chan *c;
>>
>> - list_for_each_entry_rcu(c, &conn->chan_l, list) {
>> - if (c->scid == cid) {
>> - r = c;
>> - break;
>> - }
>> + list_for_each_entry(c, &conn->chan_l, list) {
>> + if (c->scid == cid)
>> + return c;
>> }
>> -
>> - rcu_read_unlock();
>> - return r;
>> + return NULL;
>> }
>>
>> /* Find channel with given SCID.
>> @@ -115,36 +103,34 @@ 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, *r = NULL;
>> -
>> - rcu_read_lock();
>> + struct l2cap_chan *c;
>>
>> - list_for_each_entry_rcu(c, &conn->chan_l, list) {
>> - if (c->ident == ident) {
>> - r = c;
>> - break;
>> - }
>> + list_for_each_entry(c, &conn->chan_l, list) {
>> + if (c->ident == ident)
>> + return c;
>> }
>> -
>> - rcu_read_unlock();
>> - return r;
>> + return NULL;
>> }
>>
>> 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;
>> }
>>
>> @@ -259,6 +245,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
>>
>> BT_DBG("chan %p state %d", chan, chan->state);
>>
>> + mutex_lock(&chan->conn->chan_lock);
>> lock_sock(sk);
>>
>> if (chan->state == BT_CONNECTED || chan->state == BT_CONFIG)
>> @@ -272,6 +259,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
>> l2cap_chan_close(chan, reason);
>>
>> release_sock(sk);
>> + mutex_unlock(&chan->conn->chan_lock);
>>
>> chan->ops->close(chan->data);
>> l2cap_chan_put(chan);
>> @@ -357,7 +345,9 @@ static void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
>>
>> l2cap_chan_hold(chan);
>>
>> - list_add_rcu(&chan->list, &conn->chan_l);
>> + mutex_lock(&conn->chan_lock);
>> + list_add(&chan->list, &conn->chan_l);
>> + mutex_unlock(&conn->chan_lock);
>
> Check the commit that added RCU to our list, I think you are missing to revert
> some parts of it.
He's undoing it a little bit different. Check my comments in the
previous series he sent.
Regards,
--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
