Hi Andrei,
On Tue, Nov 08, 2011, Andrei Emeltchenko wrote:
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -549,8 +549,11 @@ int hci_dev_open(__u16 dev)
> > hci_dev_hold(hdev);
> > set_bit(HCI_UP, &hdev->flags);
> > hci_notify(hdev, HCI_DEV_UP);
> > - if (!test_bit(HCI_SETUP, &hdev->flags))
> > + if (!test_bit(HCI_SETUP, &hdev->flags)) {
> > + hci_dev_lock_bh(hdev);
> > mgmt_powered(hdev, 1);
> > + hci_dev_unlock_bh(hdev);
>
> Shall we acquire lock before test_bit here and below?
Once the HCI_SETUP flag has been cleared it never gets set again for a
hci_dev, so in the above case it doesn't really make a difference
whether we lock before or after the test.
> > @@ -1561,8 +1566,11 @@ void hci_unregister_dev(struct hci_dev *hdev)
> > kfree_skb(hdev->reassembly[i]);
> >
> > if (!test_bit(HCI_INIT, &hdev->flags) &&
> > - !test_bit(HCI_SETUP, &hdev->flags))
> > + !test_bit(HCI_SETUP, &hdev->flags)) {
> > + hci_dev_lock_bh(hdev);
> > mgmt_index_removed(hdev);
> > + hci_dev_unlock_bh(hdev);
> > + }
The same applies for the above. The only question there is about
HCI_INIT. Since we're inside hci_unregister_dev and have already called
hci_dev_do_close I don't think there's any problem here. Furthermore,
the whole test for HCI_INIT looks unnecessary to me since testing for
HCI_SETUP should be enough. I might send a separate patch for that later
(it's anyway unrelated to this locking patch).
Johan
--
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
