Hi Gustavo,
On Sat, Dec 17, 2011 at 07:29:31PM -0200, Gustavo F. Padovan wrote:
> From: "Gustavo F. Padovan" <padovan@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
...
> @@ -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
--
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
