| Message ID | 20200604212842.v2.1.I0ec31d716619532fc007eac081e827a204ba03de@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Marcel Holtmann |
| Headers | show |
| Series | [v2] Bluetooth: Allow suspend even when preparation has failed | expand |
On Thu, 4 Jun 2020 21:28:50 -0700 Abhishek Pandit-Subedi wrote: > It is preferable to allow suspend even when Bluetooth has problems > preparing for sleep. When Bluetooth fails to finish preparing for > suspend, log the error and allow the suspend notifier to continue > instead. > > To also make it clearer why suspend failed, change bt_dev_dbg to > bt_dev_err when handling the suspend timeout. > > Fixes: dd522a7429b07e ("Bluetooth: Handle LE devices during suspend") > Reported-by: Len Brown <len.brown@intel.com> > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > --- > To verify this is properly working, I added an additional change to > hci_suspend_wait_event to always return -16. This validates that suspend > continues even when an error has occurred during the suspend > preparation. > > Example on Chromebook: > [ 55.834524] PM: Syncing filesystems ... done. > [ 55.841930] PM: Preparing system for sleep (s2idle) > [ 55.940492] Bluetooth: hci_core.c:hci_suspend_notifier() hci0: Suspend notifier action (3) failed: -16 > [ 55.940497] Freezing user space processes ... (elapsed 0.001 seconds) done. > [ 55.941692] OOM killer disabled. > [ 55.941693] Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done. > [ 55.942632] PM: Suspending system (s2idle) > > I ran this through a suspend_stress_test in the following scenarios: > * Peer classic device connected: 50+ suspends > * No devices connected: 100 suspends > * With the above test case returning -EBUSY: 50 suspends > > I also ran this through our automated testing for suspend and wake on > BT from suspend continues to work. > > > Changes in v2: > - Added fixes and reported-by tags Building W=1 C=1 gcc-10: In file included from ../net/bluetooth/hci_core.c:38: ../net/bluetooth/hci_core.c: In function ‘hci_suspend_notifier’: ../include/net/bluetooth/bluetooth.h:182:9: warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘long unsigned int’ [-Wformat=] 182 | BT_ERR("%s: " fmt, (hdev)->name, ##__VA_ARGS__) | ^~~~~~ ../include/net/bluetooth/bluetooth.h:169:33: note: in definition of macro ‘BT_ERR’ 169 | #define BT_ERR(fmt, ...) bt_err(fmt "\n", ##__VA_ARGS__) | ^~~ ../net/bluetooth/hci_core.c:3368:3: note: in expansion of macro ‘bt_dev_err’ 3368 | bt_dev_err(hdev, "Suspend notifier action (%x) failed: %d", | ^~~~~~~~~~
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index dbe2d79f233fba..54da48441423e0 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -3289,10 +3289,10 @@ static int hci_suspend_wait_event(struct hci_dev *hdev) WAKE_COND, SUSPEND_NOTIFIER_TIMEOUT); if (ret == 0) { - bt_dev_dbg(hdev, "Timed out waiting for suspend"); + 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_dbg(hdev, "Bit %d is set", i); + bt_dev_err(hdev, "Suspend timeout bit: %d", i); clear_bit(i, hdev->suspend_tasks); } @@ -3360,12 +3360,15 @@ static int hci_suspend_notifier(struct notifier_block *nb, unsigned long action, ret = hci_change_suspend_state(hdev, BT_RUNNING); } - /* If suspend failed, restore it to running */ - if (ret && action == PM_SUSPEND_PREPARE) - hci_change_suspend_state(hdev, BT_RUNNING); - done: - return ret ? notifier_from_errno(-EBUSY) : NOTIFY_STOP; + /* We always allow suspend even if suspend preparation failed and + * attempt to recover in resume. + */ + if (ret) + bt_dev_err(hdev, "Suspend notifier action (%x) failed: %d", + action, ret); + + return NOTIFY_STOP; } /* Alloc HCI device */
It is preferable to allow suspend even when Bluetooth has problems preparing for sleep. When Bluetooth fails to finish preparing for suspend, log the error and allow the suspend notifier to continue instead. To also make it clearer why suspend failed, change bt_dev_dbg to bt_dev_err when handling the suspend timeout. Fixes: dd522a7429b07e ("Bluetooth: Handle LE devices during suspend") Reported-by: Len Brown <len.brown@intel.com> Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- To verify this is properly working, I added an additional change to hci_suspend_wait_event to always return -16. This validates that suspend continues even when an error has occurred during the suspend preparation. Example on Chromebook: [ 55.834524] PM: Syncing filesystems ... done. [ 55.841930] PM: Preparing system for sleep (s2idle) [ 55.940492] Bluetooth: hci_core.c:hci_suspend_notifier() hci0: Suspend notifier action (3) failed: -16 [ 55.940497] Freezing user space processes ... (elapsed 0.001 seconds) done. [ 55.941692] OOM killer disabled. [ 55.941693] Freezing remaining freezable tasks ... (elapsed 0.000 seconds) done. [ 55.942632] PM: Suspending system (s2idle) I ran this through a suspend_stress_test in the following scenarios: * Peer classic device connected: 50+ suspends * No devices connected: 100 suspends * With the above test case returning -EBUSY: 50 suspends I also ran this through our automated testing for suspend and wake on BT from suspend continues to work. Changes in v2: - Added fixes and reported-by tags net/bluetooth/hci_core.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)