Bluetooth: Run page scan updates through hdev->req_workqueue
Since Add/Remove Device perform the page scan updates independently
from the HCI command completion we've introduced a potential race when
multiple mgmt commands are queued. Doing the page scan updates through
the req_workqueue ensures that the state changes are performed in a
race-free manner.
At the same time, to make the request helper more widely usable,
extend it to also cover Inquiry Scan changes since those are behind
the same HCI command. This is also reflected in the new name of the
API as well as the work struct name.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 55ce209..eda809a 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -329,6 +329,7 @@
struct work_struct discov_update;
struct work_struct bg_scan_update;
+ struct work_struct scan_update;
struct delayed_work le_scan_disable;
struct delayed_work le_scan_restart;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index d57c11c..703e37f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2176,7 +2176,7 @@
hci_send_cmd(hdev, HCI_OP_READ_REMOTE_FEATURES,
sizeof(cp), &cp);
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
}
/* Set packet type for incoming connection */
@@ -2362,7 +2362,7 @@
if (test_bit(HCI_CONN_FLUSH_KEY, &conn->flags))
hci_remove_link_key(hdev, &conn->dst);
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
}
params = hci_conn_params_lookup(hdev, &conn->dst, conn->dst_type);
diff --git a/net/bluetooth/hci_request.c b/net/bluetooth/hci_request.c
index e639671..78c026b 100644
--- a/net/bluetooth/hci_request.c
+++ b/net/bluetooth/hci_request.c
@@ -637,7 +637,7 @@
return false;
}
-void __hci_update_page_scan(struct hci_request *req)
+void __hci_req_update_scan(struct hci_request *req)
{
struct hci_dev *hdev = req->hdev;
u8 scan;
@@ -657,22 +657,29 @@
else
scan = SCAN_DISABLED;
- if (test_bit(HCI_PSCAN, &hdev->flags) == !!(scan & SCAN_PAGE))
- return;
-
if (hci_dev_test_flag(hdev, HCI_DISCOVERABLE))
scan |= SCAN_INQUIRY;
+ if (test_bit(HCI_PSCAN, &hdev->flags) == !!(scan & SCAN_PAGE) &&
+ test_bit(HCI_ISCAN, &hdev->flags) == !!(scan & SCAN_INQUIRY))
+ return;
+
hci_req_add(req, HCI_OP_WRITE_SCAN_ENABLE, 1, &scan);
}
-void hci_update_page_scan(struct hci_dev *hdev)
+static int update_scan(struct hci_request *req, unsigned long opt)
{
- struct hci_request req;
+ hci_dev_lock(req->hdev);
+ __hci_req_update_scan(req);
+ hci_dev_unlock(req->hdev);
+ return 0;
+}
- hci_req_init(&req, hdev);
- __hci_update_page_scan(&req);
- hci_req_run(&req, NULL);
+static void scan_update_work(struct work_struct *work)
+{
+ struct hci_dev *hdev = container_of(work, struct hci_dev, scan_update);
+
+ hci_req_sync(hdev, update_scan, 0, HCI_CMD_TIMEOUT, NULL);
}
/* This function controls the background scanning based on hdev->pend_le_conns
@@ -1270,6 +1277,7 @@
{
INIT_WORK(&hdev->discov_update, discov_update);
INIT_WORK(&hdev->bg_scan_update, bg_scan_update);
+ INIT_WORK(&hdev->scan_update, scan_update_work);
INIT_DELAYED_WORK(&hdev->le_scan_disable, le_scan_disable_work);
INIT_DELAYED_WORK(&hdev->le_scan_restart, le_scan_restart_work);
}
@@ -1280,6 +1288,7 @@
cancel_work_sync(&hdev->discov_update);
cancel_work_sync(&hdev->bg_scan_update);
+ cancel_work_sync(&hdev->scan_update);
cancel_delayed_work_sync(&hdev->le_scan_disable);
cancel_delayed_work_sync(&hdev->le_scan_restart);
}
diff --git a/net/bluetooth/hci_request.h b/net/bluetooth/hci_request.h
index 6b9e59f..cc82755 100644
--- a/net/bluetooth/hci_request.h
+++ b/net/bluetooth/hci_request.h
@@ -61,8 +61,12 @@
/* Returns true if HCI commands were queued */
bool hci_req_stop_discovery(struct hci_request *req);
-void hci_update_page_scan(struct hci_dev *hdev);
-void __hci_update_page_scan(struct hci_request *req);
+static inline void hci_req_update_scan(struct hci_dev *hdev)
+{
+ queue_work(hdev->req_workqueue, &hdev->scan_update);
+}
+
+void __hci_req_update_scan(struct hci_request *req);
int hci_update_random_address(struct hci_request *req, bool require_privacy,
u8 *own_addr_type);
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3d9d2e4..0d20e13 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1810,7 +1810,7 @@
* entries.
*/
hci_req_init(&req, hdev);
- __hci_update_page_scan(&req);
+ __hci_req_update_scan(&req);
update_class(&req);
hci_req_run(&req, NULL);
@@ -2058,7 +2058,7 @@
if (conn_changed || discov_changed) {
new_settings(hdev, cmd->sk);
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
if (discov_changed)
mgmt_update_adv_data(hdev);
hci_update_background_scan(hdev);
@@ -2092,7 +2092,7 @@
return err;
if (changed) {
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
hci_update_background_scan(hdev);
return new_settings(hdev, sk);
}
@@ -5041,7 +5041,7 @@
hci_req_init(&req, hdev);
write_fast_connectable(&req, false);
- __hci_update_page_scan(&req);
+ __hci_req_update_scan(&req);
/* Since only the advertising data flags will change, there
* is no need to update the scan response data.
@@ -5927,7 +5927,7 @@
if (err)
goto unlock;
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
goto added;
}
@@ -6024,7 +6024,7 @@
goto unlock;
}
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
device_removed(sk, hdev, &cp->addr.bdaddr,
cp->addr.type);
@@ -6089,7 +6089,7 @@
kfree(b);
}
- hci_update_page_scan(hdev);
+ hci_req_update_scan(hdev);
list_for_each_entry_safe(p, tmp, &hdev->le_conn_params, list) {
if (p->auto_connect == HCI_AUTO_CONN_DISABLED)
@@ -7397,7 +7397,7 @@
write_fast_connectable(&req, true);
else
write_fast_connectable(&req, false);
- __hci_update_page_scan(&req);
+ __hci_req_update_scan(&req);
update_class(&req);
update_name(&req);
update_eir(&req);