Hi Andre,
On Thu, Feb 14, 2013, Andre Guedes 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.
> > + if (!transaction) {
> > + err = -ENOMEM;
> > + goto unlock;
> > + }
> > +
> > + memset(transaction, 0, sizeof(*transaction));
>
> We can also use kzalloc instead of kmalloc, making this memset unnecessary.
Good point. Fixed in v2.
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
