Hi Luiz, > hci_cmd_sync_queue can be called multiple times, each adding a > hci_cmd_sync_work_entry, before hci_cmd_sync_work is run so this makes > sure they are all dequeued properly otherwise it creates a backlog of > entries that are never run. > > Link: https://lore.kernel.org/all/CAJCQCtSeUtHCgsHXLGrSTWKmyjaQDbDNpP4rb0i+RE+L2FTXSA@xxxxxxxxxxxxxx/T/ > Fixes: 6a98e3836fa20 ("Bluetooth: Add helper for serialized HCI command execution") > Tested-by: Chris Clayton <chris2553@xxxxxxxxxxxxxx> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx> > --- > net/bluetooth/hci_sync.c | 64 ++++++++++++++++++++++++---------------- > 1 file changed, 38 insertions(+), 26 deletions(-) > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c > index d146d4efae43..121df2dcf2f1 100644 > --- a/net/bluetooth/hci_sync.c > +++ b/net/bluetooth/hci_sync.c > @@ -273,43 +273,55 @@ int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen, > } > EXPORT_SYMBOL(__hci_cmd_sync_status); > > -static void hci_cmd_sync_work(struct work_struct *work) > +static void hci_cmd_sync_work_entry_run(struct hci_dev *hdev, > + struct hci_cmd_sync_work_entry *entry) > { > - struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_sync_work); > - struct hci_cmd_sync_work_entry *entry; > hci_cmd_sync_work_func_t func; > hci_cmd_sync_work_destroy_t destroy; > void *data; > + int err; > > - bt_dev_dbg(hdev, ""); > + bt_dev_dbg(hdev, "entry %p", entry); > > - mutex_lock(&hdev->cmd_sync_work_lock); > - entry = list_first_entry(&hdev->cmd_sync_work_list, > - struct hci_cmd_sync_work_entry, list); > - if (entry) { > - list_del(&entry->list); > - func = entry->func; > - data = entry->data; > - destroy = entry->destroy; > - kfree(entry); > - } else { > - func = NULL; > - data = NULL; > - destroy = NULL; > - } > - mutex_unlock(&hdev->cmd_sync_work_lock); > + func = entry->func; > + data = entry->data; > + destroy = entry->destroy; > + kfree(entry); > > - if (func) { > - int err; > + if (!func) > + return; > > - hci_req_sync_lock(hdev); > + hci_req_sync_lock(hdev); > + > + err = func(hdev, data); > > - err = func(hdev, data); > + if (destroy) > + destroy(hdev, data, err); > > - if (destroy) > - destroy(hdev, data, err); > + hci_req_sync_unlock(hdev); > +} > + > +static void hci_cmd_sync_work(struct work_struct *work) > +{ > + struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_sync_work); > + struct hci_cmd_sync_work_entry *entry; > + > + bt_dev_dbg(hdev, ""); > + > + /* Dequeue all entries and run them */ > + while (1) { > + mutex_lock(&hdev->cmd_sync_work_lock); > + entry = list_first_entry_or_null(&hdev->cmd_sync_work_list, > + struct hci_cmd_sync_work_entry, > + list); > + if (entry) > + list_del(&entry->list); > + mutex_unlock(&hdev->cmd_sync_work_lock); > + > + if (!entry) > + break; > > - hci_req_sync_unlock(hdev); > + hci_cmd_sync_work_entry_run(hdev, entry); > } > } > I was dead serious with my example on how to do this. Scrap the hci_cmd_sync_work_entry_run and do this all in one while loop. Regards Marcel
