Skip to content

Commit

Permalink
Bluetooth: hci_sync: Rework hci_suspend_notifier
Browse files Browse the repository at this point in the history 
This makes hci_suspend_notifier use the hci_*_sync which can be
executed synchronously which is allowed in the suspend_notifier and
simplifies a lot of the handling since the status of each command can
be checked inline so no other work need to be scheduled thus can be
performed without using of a state machine.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
  • Loading branch information
@Vudentz @holtmann
Vudentz authored and holtmann committed Oct 29, 2021
1 parent d0b1370 commit 182ee45
Show file tree
Hide file tree
Showing 10 changed files with 650 additions and 638 deletions.
4 changes: 0 additions & 4 deletions include/net/bluetooth/hci_core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,6 @@ struct hci_dev {
bool advertising_paused;

struct notifier_block suspend_notifier;
struct work_struct suspend_prepare;
enum suspended_state suspend_state_next;
enum suspended_state suspend_state;
bool scanning_paused;
Expand All @@ -532,9 +531,6 @@ struct hci_dev {
bdaddr_t wake_addr;
u8 wake_addr_type;

wait_queue_head_t suspend_wait_q;
DECLARE_BITMAP(suspend_tasks, __SUSPEND_NUM_TASKS);

struct hci_conn_hash conn_hash;

struct list_head mgmt_pending;
Expand Down
3 changes: 3 additions & 0 deletions include/net/bluetooth/hci_sync.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,6 @@ int hci_set_powered_sync(struct hci_dev *hdev, u8 val);

int hci_start_discovery_sync(struct hci_dev *hdev);
int hci_stop_discovery_sync(struct hci_dev *hdev);

int hci_suspend_sync(struct hci_dev *hdev);
int hci_resume_sync(struct hci_dev *hdev);
10 changes: 0 additions & 10 deletions net/bluetooth/hci_conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -900,16 +900,6 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)

hci_conn_del(conn);

/* The suspend notifier is waiting for all devices to disconnect and an
* LE connect cancel will result in an hci_le_conn_failed. Once the last
* connection is deleted, we should also wake the suspend queue to
* complete suspend operations.
*/
if (list_empty(&hdev->conn_hash.list) &&
test_and_clear_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks)) {
wake_up(&hdev->suspend_wait_q);
}

/* Since we may have temporarily stopped the background scanning in
* favor of connection establishment, we should restart it.
*/
Expand Down
106 changes: 12 additions & 94 deletions net/bluetooth/hci_core.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2374,61 +2374,6 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
}
}

static void hci_suspend_clear_tasks(struct hci_dev *hdev)
{
int i;

for (i = 0; i < __SUSPEND_NUM_TASKS; i++)
clear_bit(i, hdev->suspend_tasks);

wake_up(&hdev->suspend_wait_q);
}

static int hci_suspend_wait_event(struct hci_dev *hdev)
{
#define WAKE_COND \
(find_first_bit(hdev->suspend_tasks, __SUSPEND_NUM_TASKS) == \
__SUSPEND_NUM_TASKS)

int i;
int ret = wait_event_timeout(hdev->suspend_wait_q,
WAKE_COND, SUSPEND_NOTIFIER_TIMEOUT);

if (ret == 0) {
bt_dev_err(hdev, "Timed out waiting for suspend events");
for (i = 0; i < __SUSPEND_NUM_TASKS; ++i) {
if (test_bit(i, hdev->suspend_tasks))
bt_dev_err(hdev, "Suspend timeout bit: %d", i);
clear_bit(i, hdev->suspend_tasks);
}

ret = -ETIMEDOUT;
} else {
ret = 0;
}

return ret;
}

static void hci_prepare_suspend(struct work_struct *work)
{
struct hci_dev *hdev =
container_of(work, struct hci_dev, suspend_prepare);

hci_dev_lock(hdev);
hci_req_prepare_suspend(hdev, hdev->suspend_state_next);
hci_dev_unlock(hdev);
}

static int hci_change_suspend_state(struct hci_dev *hdev,
enum suspended_state next)
{
hdev->suspend_state_next = next;
set_bit(SUSPEND_PREPARE_NOTIFIER, hdev->suspend_tasks);
queue_work(hdev->req_workqueue, &hdev->suspend_prepare);
return hci_suspend_wait_event(hdev);
}

static void hci_clear_wake_reason(struct hci_dev *hdev)
{
hci_dev_lock(hdev);
Expand Down Expand Up @@ -2565,7 +2510,6 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
INIT_WORK(&hdev->tx_work, hci_tx_work);
INIT_WORK(&hdev->power_on, hci_power_on);
INIT_WORK(&hdev->error_reset, hci_error_reset);
INIT_WORK(&hdev->suspend_prepare, hci_prepare_suspend);

hci_cmd_sync_init(hdev);

Expand All @@ -2576,7 +2520,6 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
skb_queue_head_init(&hdev->raw_q);

init_waitqueue_head(&hdev->req_wait_q);
init_waitqueue_head(&hdev->suspend_wait_q);

INIT_DELAYED_WORK(&hdev->cmd_timer, hci_cmd_timeout);
INIT_DELAYED_WORK(&hdev->ncmd_timer, hci_ncmd_timeout);
Expand Down Expand Up @@ -2729,11 +2672,8 @@ void hci_unregister_dev(struct hci_dev *hdev)

hci_cmd_sync_clear(hdev);

if (!test_bit(HCI_QUIRK_NO_SUSPEND_NOTIFIER, &hdev->quirks)) {
hci_suspend_clear_tasks(hdev);
if (!test_bit(HCI_QUIRK_NO_SUSPEND_NOTIFIER, &hdev->quirks))
unregister_pm_notifier(&hdev->suspend_notifier);
cancel_work_sync(&hdev->suspend_prepare);
}

msft_unregister(hdev);

Expand Down Expand Up @@ -2800,7 +2740,6 @@ EXPORT_SYMBOL(hci_release_dev);
int hci_suspend_dev(struct hci_dev *hdev)
{
int ret;
u8 state = BT_RUNNING;

bt_dev_dbg(hdev, "");

Expand All @@ -2809,40 +2748,17 @@ int hci_suspend_dev(struct hci_dev *hdev)
hci_dev_test_flag(hdev, HCI_UNREGISTER))
return 0;

/* If powering down, wait for completion. */
if (mgmt_powering_down(hdev)) {
set_bit(SUSPEND_POWERING_DOWN, hdev->suspend_tasks);
ret = hci_suspend_wait_event(hdev);
if (ret)
goto done;
}

/* Suspend consists of two actions:
* - First, disconnect everything and make the controller not
* connectable (disabling scanning)
* - Second, program event filter/accept list and enable scan
*/
ret = hci_change_suspend_state(hdev, BT_SUSPEND_DISCONNECT);
if (ret)
goto clear;

state = BT_SUSPEND_DISCONNECT;
/* If powering down don't attempt to suspend */
if (mgmt_powering_down(hdev))
return 0;

/* Only configure accept list if device may wakeup. */
if (hdev->wakeup && hdev->wakeup(hdev)) {
ret = hci_change_suspend_state(hdev, BT_SUSPEND_CONFIGURE_WAKE);
if (!ret)
state = BT_SUSPEND_CONFIGURE_WAKE;
}
hci_req_sync_lock(hdev);
ret = hci_suspend_sync(hdev);
hci_req_sync_unlock(hdev);

clear:
hci_clear_wake_reason(hdev);
mgmt_suspending(hdev, state);
mgmt_suspending(hdev, hdev->suspend_state);

done:
/* We always allow suspend even if suspend preparation failed and
* attempt to recover in resume.
*/
hci_sock_dev_event(hdev, HCI_DEV_SUSPEND);
return ret;
}
Expand All @@ -2864,10 +2780,12 @@ int hci_resume_dev(struct hci_dev *hdev)
if (mgmt_powering_down(hdev))
return 0;

ret = hci_change_suspend_state(hdev, BT_RUNNING);
hci_req_sync_lock(hdev);
ret = hci_resume_sync(hdev);
hci_req_sync_unlock(hdev);

mgmt_resuming(hdev, hdev->wake_reason, &hdev->wake_addr,
hdev->wake_addr_type);
hdev->wake_addr_type);

hci_sock_dev_event(hdev, HCI_DEV_RESUME);
return ret;
Expand Down
71 changes: 53 additions & 18 deletions net/bluetooth/hci_event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2414,9 +2414,14 @@ static void hci_cs_exit_sniff_mode(struct hci_dev *hdev, __u8 status)
static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)
{
struct hci_cp_disconnect *cp;
struct hci_conn_params *params;
struct hci_conn *conn;
bool mgmt_conn;

if (!status)
/* Wait for HCI_EV_DISCONN_COMPLETE if status 0x00 and not suspended
* otherwise cleanup the connection immediately.
*/
if (!status && !hdev->suspended)
return;

cp = hci_sent_cmd_data(hdev, HCI_OP_DISCONNECT);
Expand All @@ -2426,7 +2431,10 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)
hci_dev_lock(hdev);

conn = hci_conn_hash_lookup_handle(hdev, __le16_to_cpu(cp->handle));
if (conn) {
if (!conn)
goto unlock;

if (status) {
mgmt_disconnect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, status);

Expand All @@ -2435,14 +2443,48 @@ static void hci_cs_disconnect(struct hci_dev *hdev, u8 status)
hci_enable_advertising(hdev);
}

/* If the disconnection failed for any reason, the upper layer
* does not retry to disconnect in current implementation.
* Hence, we need to do some basic cleanup here and re-enable
* advertising if necessary.
*/
hci_conn_del(conn);
goto done;
}

mgmt_conn = test_and_clear_bit(HCI_CONN_MGMT_CONNECTED, &conn->flags);

if (conn->type == ACL_LINK) {
if (test_bit(HCI_CONN_FLUSH_KEY, &conn->flags))
hci_remove_link_key(hdev, &conn->dst);
}

params = hci_conn_params_lookup(hdev, &conn->dst, conn->dst_type);
if (params) {
switch (params->auto_connect) {
case HCI_AUTO_CONN_LINK_LOSS:
if (cp->reason != HCI_ERROR_CONNECTION_TIMEOUT)
break;
fallthrough;

case HCI_AUTO_CONN_DIRECT:
case HCI_AUTO_CONN_ALWAYS:
list_del_init(&params->action);
list_add(&params->action, &hdev->pend_le_conns);
break;

default:
break;
}
}

mgmt_device_disconnected(hdev, &conn->dst, conn->type, conn->dst_type,
cp->reason, mgmt_conn);

hci_disconn_cfm(conn, cp->reason);

done:
/* If the disconnection failed for any reason, the upper layer
* does not retry to disconnect in current implementation.
* Hence, we need to do some basic cleanup here and re-enable
* advertising if necessary.
*/
hci_conn_del(conn);
unlock:
hci_dev_unlock(hdev);
}

Expand Down Expand Up @@ -3047,14 +3089,6 @@ static void hci_disconn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb)

hci_conn_del(conn);

/* The suspend notifier is waiting for all devices to disconnect so
* clear the bit from pending tasks and inform the wait queue.
*/
if (list_empty(&hdev->conn_hash.list) &&
test_and_clear_bit(SUSPEND_DISCONNECTING, hdev->suspend_tasks)) {
wake_up(&hdev->suspend_wait_q);
}

unlock:
hci_dev_unlock(hdev);
}
Expand Down Expand Up @@ -5575,8 +5609,9 @@ static struct hci_conn *check_pending_le_conn(struct hci_dev *hdev,
if (adv_type != LE_ADV_IND && adv_type != LE_ADV_DIRECT_IND)
return NULL;

/* Ignore if the device is blocked */
if (hci_bdaddr_list_lookup(&hdev->reject_list, addr, addr_type))
/* Ignore if the device is blocked or hdev is suspended */
if (hci_bdaddr_list_lookup(&hdev->reject_list, addr, addr_type) ||
hdev->suspended)
return NULL;

/* Most controller will fail if we try to create new connections
Expand Down
Loading

0 comments on commit 182ee45

Please sign in to comment.