| Message ID | 20210816231150.1478727-1-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Revert "Bluetooth: Shutdown controller after workqueues are flushed or cancelled" | expand |
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=532345
---Test result---
Test Summary:
CheckPatch FAIL 0.55 seconds
GitLint PASS 0.13 seconds
BuildKernel PASS 661.59 seconds
TestRunner: Setup PASS 424.38 seconds
TestRunner: l2cap-tester PASS 2.99 seconds
TestRunner: bnep-tester PASS 2.14 seconds
TestRunner: mgmt-tester PASS 32.86 seconds
TestRunner: rfcomm-tester PASS 2.36 seconds
TestRunner: sco-tester PASS 2.38 seconds
TestRunner: smp-tester FAIL 2.43 seconds
TestRunner: userchan-tester PASS 2.22 seconds
Details
##############################
Test: CheckPatch - FAIL - 0.55 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Revert "Bluetooth: Shutdown controller after workqueues are flushed or cancelled"
WARNING: Unknown commit id '0ea9fd001a14', maybe rebased or not pulled?
#25:
Fixes: 0ea9fd001a14 ("Bluetooth: Shutdown controller after workqueues are flushed or cancelled")
total: 0 errors, 1 warnings, 0 checks, 28 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
"[PATCH] Revert "Bluetooth: Shutdown controller after workqueues are" has style problems, please review.
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - PASS - 0.13 seconds
Run gitlint with rule in .gitlint
##############################
Test: BuildKernel - PASS - 661.59 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 424.38 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 2.99 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.14 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 32.86 seconds
Run test-runner with mgmt-tester
Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.36 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.38 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - FAIL - 2.43 seconds
Run test-runner with smp-tester
Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0
Failed Test Cases
SMP Client - SC Request 2 Failed 0.028 seconds
##############################
Test: TestRunner: userchan-tester - PASS - 2.22 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
Hi Dmitry, > This reverts commit 0ea9fd001a14ebc294f112b0361a4e601551d508. It moved > calling shutdown callback after flushing the queues. In doing so it > disabled calling the shutdown hook completely: shutdown condition > tests for HCI_UP in hdev->flags, which gets cleared now before checking > this condition (see test_and_clear_bit(HCI_UP, ...) call). Thus shutdown > hook was never called. > > This would not be a problem itself and could fixed with just removing > the HCI_UP condition (since if we are this point, we already know that > the HCI device was up before calling hci_dev_do_close(). However the > fact that shutdown hook was not called hid the fact that it is not > proper to call shutdown hook so late in the sequence. The hook would > usually call __hci_cmd_sync()/__hci_cmd_sync_ev(), which would timeout > without running queues. > > Thus I think it is more proper at this moment to revert the commit and > look for a better solution. > > Fixes: 0ea9fd001a14 ("Bluetooth: Shutdown controller after workqueues are flushed or cancelled") > Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > net/bluetooth/hci_core.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) I just merged this patch: commit 0ea53674d07fb6db2dd7a7ec2fdc85a12eb246c2 Author: Kai-Heng Feng <kai.heng.feng@canonical.com> Date: Tue Aug 10 12:53:15 2021 +0800 Bluetooth: Move shutdown callback before flushing tx and rx queue Commit 0ea9fd001a14 ("Bluetooth: Shutdown controller after workqueues are flushed or cancelled") introduced a regression that makes mtkbtsdio driver stops working: Please check if this works for you. Regards Marcel
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index e1a545c8a69f..677d880068a4 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -1721,6 +1721,14 @@ int hci_dev_do_close(struct hci_dev *hdev) BT_DBG("%s %p", hdev->name, hdev); + if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) && + !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && + test_bit(HCI_UP, &hdev->flags)) { + /* Execute vendor specific shutdown routine */ + if (hdev->shutdown) + hdev->shutdown(hdev); + } + cancel_delayed_work(&hdev->power_off); cancel_delayed_work(&hdev->ncmd_timer); @@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev) clear_bit(HCI_INIT, &hdev->flags); } - if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) && - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && - test_bit(HCI_UP, &hdev->flags)) { - /* Execute vendor specific shutdown routine */ - if (hdev->shutdown) - hdev->shutdown(hdev); - } - /* flush cmd work */ flush_work(&hdev->cmd_work);
This reverts commit 0ea9fd001a14ebc294f112b0361a4e601551d508. It moved calling shutdown callback after flushing the queues. In doing so it disabled calling the shutdown hook completely: shutdown condition tests for HCI_UP in hdev->flags, which gets cleared now before checking this condition (see test_and_clear_bit(HCI_UP, ...) call). Thus shutdown hook was never called. This would not be a problem itself and could fixed with just removing the HCI_UP condition (since if we are this point, we already know that the HCI device was up before calling hci_dev_do_close(). However the fact that shutdown hook was not called hid the fact that it is not proper to call shutdown hook so late in the sequence. The hook would usually call __hci_cmd_sync()/__hci_cmd_sync_ev(), which would timeout without running queues. Thus I think it is more proper at this moment to revert the commit and look for a better solution. Fixes: 0ea9fd001a14 ("Bluetooth: Shutdown controller after workqueues are flushed or cancelled") Cc: Kai-Heng Feng <kai.heng.feng@canonical.com> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- net/bluetooth/hci_core.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)