Hi,
On Fri, Feb 17, 2012 at 1:00 PM, Johan Hedberg <johan.hedberg@xxxxxxxxx> wrote:
>
> Hi,
>
> On Fri, Feb 17, 2012, Johan Hedberg wrote:
> > > > > The locking doesn't look right to me above. hci_do_inquiry should be
> > > > > called with the lock held. I think it might be simpler if you make
> > > > > mgmt_interleaved_discovery() require the caller to hold the lock.
> > > >
> > > > Yes, you're right. I just realized hci_do_inquiry now calls inquiry_
> > > > cache_flush which requires hdev->lock held. I'll fix this too. Thanks.
> > >
> > > please keep the lock inside mgmt_interleaved_discovery() for now. We
> > > have enough locking crazy. I don't wanna add to it by making the caller
> > > deal with it right now.
> >
> > The calling code looks like this:
> >
> > +
> > + if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED) {
> > + mgmt_interleaved_discovery(hdev);
> > + } else {
> > + hci_dev_lock(hdev);
> > + hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> > + hci_dev_unlock(hdev);
> > + }
> >
> > I was thinking that if mgmt_interleaved_discovery required the lock to
> > be held this code would become simpler:
> >
> > hci_dev_lock(hdev);
> >
> > if (hdev->discovery.type == DISCOV_TYPE_INTERLEAVED)
> > mgmt_interleaved_discovery(hdev);
> > else
> > hci_discovery_set_state(hdev, DISCOVERY_STOPPED);
> >
> > hci_dev_unlock(hdev);
>
> Furthermore, almost all calls from hci_core.c or hci_event.c into mgmt.c
> (that pass hdev as a parameter) have hdev locked so this would just be
> maintaining consistency.
Makes sense to me. Any reason not be this way, Marcel?
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
