Re: [PATCH] Bluetooth: Fix missing hci_dev_lock/unlock

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Jaganath & Marcel,

On Wed, Dec 10, 2014, JAGANATH KANAKKASSERY wrote:
> >> 	err = hci_dev_do_open(hdev);
> >> 	if (err < 0) {
> >> +		hci_dev_lock(hdev);
> >> 		mgmt_set_powered_failed(hdev, err);
> >> +		hci_dev_unlock(hdev);
> >> 		return;
> >> 	}
> 
> >I wonder is some of the mgmt_ function should just take the hci_dev
> >lock. Are there cases where we don't want them to take the look?
> 
> There are many mgmt_functions called from hci_event.c which don't
> require lock.  You prefer moving the lock inside
> mgmt_set_powered_failed()?

This should be kept consistent imo. Either all mgmt_* functions called
from hci_core/event.c should be responsible for taking the lock
themselves or none should be. I think right now the general rule is that
hci_core/event.c take the lock first, and this is mainly because a lot
of code that needed calling into mgmt.c was already holding the lock.
I.e. moving the taking into mgmt.c would for many places have required
awkward looking hdev_unlock(); mgmt_foo(); hdev_lock(); constructions.

Johan