[BlueZ,1/1] Revert "device: Consider service state on device_is_connected"

Commit Message

Dimitris April 3, 2024, 8:52 p.m. UTC

This reverts commit 44d3f67277f83983e1e9697eda7b9aeb40ca231d.
---
 src/device.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

[e-mail] April 3, 2024, 10:39 p.m. UTC | #1

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=841223

---Test result---

Test Summary:
CheckPatch                    FAIL      0.63 seconds
GitLint                       PASS      0.32 seconds
BuildEll                      PASS      24.58 seconds
BluezMake                     PASS      1713.67 seconds
MakeCheck                     PASS      13.57 seconds
MakeDistcheck                 PASS      176.41 seconds
CheckValgrind                 PASS      243.68 seconds
CheckSmatch                   PASS      345.98 seconds
bluezmakeextell               PASS      117.79 seconds
IncrementalBuild              PASS      1567.24 seconds
ScanBuild                     PASS      965.21 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,1/1] Revert "device: Consider service state on device_is_connected"
WARNING:UNKNOWN_COMMIT_ID: Unknown commit id '44d3f67277f83983e1e9697eda7b9aeb40ca231d', maybe rebased or not pulled?
#94: 
This reverts commit 44d3f67277f83983e1e9697eda7b9aeb40ca231d.

/github/workspace/src/src/13616707.patch total: 0 errors, 1 warnings, 12 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.

/github/workspace/src/src/13616707.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




---
Regards,
Linux Bluetooth

Paul Menzel April 4, 2024, 6:24 a.m. UTC | #2

Dear Dimitris,


Am 03.04.24 um 22:52 schrieb Dimitris:
> This reverts commit 44d3f67277f83983e1e9697eda7b9aeb40ca231d.

Please document the reason for the revert.

> ---
>   src/device.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)

[…]


Kind regards,

Paul

Dimitris April 4, 2024, 6:35 a.m. UTC | #3

Hi Paul,

> Please document the reason for the revert.

I've since done a narrower change; instead of the revert I'm only 
factoring out the state check that avoids the busy loop at "rfkill block 
bluetooth" time.

Updated cover letter and patch under the "V2" part of the thread:

https://lore.kernel.org/linux-bluetooth/[e-mail]/T/#u

Issue steps to reproduce from e.g. 
https://github.com/bluez/bluez/issues/785 :

- Connect at least one device. Tried this with either one of: Logitech 
MX Master 3, Google Pixel Buds Pro.
- Run rfkill block bluetooth
- bluetoothd takes a whole core, GNOME quick settings and status still 
shows bluetooth as "active". I have to kill -9 the process to get the 
block to really complete.
- Turning off the connected device does not break the loop.

Busy loop stack looks like:

> #0  adapter_remove_connection (adapter=0x55a17e6889d0, device=0x55a17e698d30, bdaddr_type=2 '\002') at src/adapter.c:7476
> #1  0x000055a17e55979f in adapter_stop (adapter=0x55a17e6889d0) at src/adapter.c:7527
> #2  settings_changed (settings=<optimized out>, adapter=0x55a17e6889d0) at src/adapter.c:622
> #3  new_settings_callback (index=<optimized out>, length=<optimized out>, param=<optimized out>, user_data=0x55a17e6889d0) at src/adapter.c:705
> #4  0x000055a17e59981e in queue_foreach (user_data=0x7ffe49a7ef20, function=0x55a17e591e60 <notify_handler>, queue=0x55a17e683280) at src/shared/queue.c:207
> #5  queue_foreach (user_data=0x7ffe49a7ef20, function=0x55a17e591e60 <notify_handler>, queue=0x55a17e683280) at src/shared/queue.c:190
> #6  process_notify (param=<optimized out>, length=<optimized out>, index=<optimized out>, event=<optimized out>, mgmt=0x55a17e682f30) at src/shared/mgmt.c:349
> #7  can_read_data (io=<optimized out>, user_data=0x55a17e682f30) at src/shared/mgmt.c:409
> #8  0x000055a17e5bb679 in watch_callback (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>) at src/shared/io-glib.c:157
> #9  0x00007f876edd4e5c in g_main_context_dispatch_unlocked.lto_priv () from target:/lib64/libglib-2.0.so.0
> #10 0x00007f876ee2ff18 in g_main_context_iterate_unlocked.isra () from target:/lib64/libglib-2.0.so.0
> #11 0x00007f876edd6447 in g_main_loop_run () from target:/lib64/libglib-2.0.so.0
> #12 0x000055a17e4f1d64 in mainloop_run () at src/shared/mainloop-glib.c:66
> #13 mainloop_run_with_signal (func=0x55a17e544740 <signal_callback>, user_data=0x0) at src/shared/mainloop-notify.c:188
> #14 main (argc=<optimized out>, argv=<optimized out>) at src/main.c:1455

First time submitter here, should I resubmit/start a new thread for this 
"V2" approach?

Thanks

D.

Patch

diff --git a/src/device.c b/src/device.c
index 5e74633c6..de5f94c7d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -3273,11 +3273,7 @@  uint8_t btd_device_get_bdaddr_type(struct btd_device *dev)
 
 bool btd_device_is_connected(struct btd_device *dev)
 {
-	if (dev->bredr_state.connected || dev->le_state.connected)
-		return true;
-
-	return find_service_with_state(dev->services,
-						BTD_SERVICE_STATE_CONNECTED);
+	return dev->bredr_state.connected || dev->le_state.connected;
 }
 
 static void clear_temporary_timer(struct btd_device *dev)