Re: [PATCH 02/12] Bluetooth: Add basic start/complete HCI transaction functions

[Szymon Janc <szymon.janc@tieto.com>]

Hi Johan,

On Friday 15 of February 2013 10:22:36 Johan Hedberg wrote:
> > > +int hci_start_transaction(struct hci_dev *hdev)
> > > +{
> > > +       struct hci_transaction *transaction;
> > > +       int err;
> > > +
> > > +       hci_transaction_lock(hdev);
> > > +
> > > +       /* We can't start a new transaction if there's another one in
> > > +        * progress of being built.
> > > +        */
> > > +       if (hdev->build_transaction) {
> > > +               err = -EBUSY;
> > > +               goto unlock;
> > > +       }
> > > +
> > > +       transaction = kmalloc(sizeof(*transaction), GFP_ATOMIC);
> > 
> > I've failed to see why we need GFP_ATOMIC here. As this code is not
> > running in any atomic section, we can allocate memory using
> > GFP_KERNEL.
> 
> Since one of the intentions of this API is to create an async version of
> hci_request() I think it's better to keep GFP_ATOMIC here. One situation
> where you couldn't for sure use hci_request() is if you're in an atomic
> section and then a HCI request would be the only other alternative.

You lock mutex in hci_start_transaction so it is non-atomic anyway..

-- 
BR
Szymon Janc