Re: [RFC 11/22] Bluetooth: Use RCU to manipulate chan_list
From: Andrei Emeltchenko <hidden>
Date: 2012-01-26 13:20:20
Hi Gustavo, On Sat, Dec 17, 2011 at 07:29:31PM -0200, Gustavo F. Padovan wrote:
From: "Gustavo F. Padovan" <redacted> 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 <redacted>
...
quoted hunk ↗ jump to hunk
@@ -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