| Message ID | 20240404024521.120349-2-dimitris.on.linux@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | V2: Fix busy loop when disabling BT | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
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=841263 ---Test result--- Test Summary: CheckPatch PASS 0.45 seconds GitLint PASS 0.31 seconds BuildEll PASS 25.64 seconds BluezMake PASS 1663.82 seconds MakeCheck PASS 13.49 seconds MakeDistcheck PASS 190.09 seconds CheckValgrind PASS 247.23 seconds CheckSmatch PASS 348.43 seconds bluezmakeextell PASS 118.28 seconds IncrementalBuild PASS 1436.09 seconds ScanBuild PASS 991.93 seconds --- Regards, Linux Bluetooth
Hi Dimitris, On Wed, Apr 3, 2024 at 10:46 PM Dimitris <dimitris.on.linux@gmail.com> wrote: > > Splitting the service state test out of btd_device_is_connected > and using the state-specific test from adapter_remove_connection. > > This intends to fix a busy loop that happens when BT is disabled > from userspace with e.g. "rfkill block bluetooth". > --- > src/adapter.c | 2 +- > src/device.c | 7 ++++++- > src/device.h | 1 + > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/src/adapter.c b/src/adapter.c > index 4bcc464de..0b7aab4b5 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -7486,7 +7486,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter, > device_cancel_authentication(device, TRUE); > > /* If another bearer is still connected */ > - if (btd_device_is_connected(device)) > + if (btd_device_state_is_connected(device)) Perhaps btd_device_bearer_is_connected would be a better name. > return; > > adapter->connections = g_slist_remove(adapter->connections, device); > diff --git a/src/device.c b/src/device.c > index 5e74633c6..123b1b796 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -3273,13 +3273,18 @@ 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) > + if (btd_device_state_is_connected(dev)) > return true; > > return find_service_with_state(dev->services, > BTD_SERVICE_STATE_CONNECTED); I guess the problem is that some service is indicating it is still connected though? > } > > +bool btd_device_state_is_connected(struct btd_device *dev) > +{ > + return dev->bredr_state.connected || dev->le_state.connected; > +} > + > static void clear_temporary_timer(struct btd_device *dev) > { > if (dev->temporary_timer) { > diff --git a/src/device.h b/src/device.h > index d4e70b7ef..e3191f2a4 100644 > --- a/src/device.h > +++ b/src/device.h > @@ -104,6 +104,7 @@ void device_set_rssi(struct btd_device *device, int8_t rssi); > void device_set_tx_power(struct btd_device *device, int8_t tx_power); > void device_set_flags(struct btd_device *device, uint8_t flags); > bool btd_device_is_connected(struct btd_device *dev); > +bool btd_device_state_is_connected(struct btd_device *dev); > uint8_t btd_device_get_bdaddr_type(struct btd_device *dev); > bool device_is_retrying(struct btd_device *device); > void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type, > -- > 2.44.0 > >
On 4/4/24 07:59, Luiz Augusto von Dentz wrote: >> diff --git a/src/adapter.c b/src/adapter.c >> index 4bcc464de..0b7aab4b5 100644 >> --- a/src/adapter.c >> +++ b/src/adapter.c >> @@ -7486,7 +7486,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter, >> device_cancel_authentication(device, TRUE); >> >> /* If another bearer is still connected */ >> - if (btd_device_is_connected(device)) >> + if (btd_device_state_is_connected(device)) > > Perhaps btd_device_bearer_is_connected would be a better name. Thanks, I'll rename. > I guess the problem is that some service is indicating it is still > connected though? Newbie question for sure, but: As this is happening in the code path for "disabling bluetooth", shouldn't services already be disconnected here? D.
Hi Dimitris, On Thu, Apr 4, 2024 at 11:52 AM Dimitris <dimitris.on.linux@gmail.com> wrote: > > On 4/4/24 07:59, Luiz Augusto von Dentz wrote: > > >> diff --git a/src/adapter.c b/src/adapter.c > >> index 4bcc464de..0b7aab4b5 100644 > >> --- a/src/adapter.c > >> +++ b/src/adapter.c > >> @@ -7486,7 +7486,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter, > >> device_cancel_authentication(device, TRUE); > >> > >> /* If another bearer is still connected */ > >> - if (btd_device_is_connected(device)) > >> + if (btd_device_state_is_connected(device)) > > > > Perhaps btd_device_bearer_is_connected would be a better name. > > Thanks, I'll rename. > > > I guess the problem is that some service is indicating it is still > > connected though? > > Newbie question for sure, but: As this is happening in the code path for > "disabling bluetooth", shouldn't services already be disconnected here? That is exactly what I would like to know, why is there a service still indicating it is connected if the controller is rfkilled, so while this should break it back to the old behavior we still need to fix the service that is causing the problem so perhaps we need to print its profile/drive name and figure out what is the driver that is causing it. > D.
Hi Luiz, On 4/4/24 09:16, Luiz Augusto von Dentz wrote: >>> I guess the problem is that some service is indicating it is still >>> connected though? >> >> Newbie question for sure, but: As this is happening in the code path for >> "disabling bluetooth", shouldn't services already be disconnected here? > > That is exactly what I would like to know, why is there a service > still indicating it is connected if the controller is rfkilled, so > while this should break it back to the old behavior we still need to > fix the service that is causing the problem so perhaps we need to > print its profile/drive name and figure out what is the driver that is > causing it. > I added a debug kludge: > diff --git a/src/device.c b/src/device.c > index 74dd67a09..c461a6a3a 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -344,7 +344,15 @@ static GSList *find_service_with_state(GSList *list, > struct btd_service *service = l->data; > > if (btd_service_get_state(service) == state) > + { > + char name[256]; > + device_get_name(btd_service_get_device(service), name, 256); > + info("Found service: err: %d, device: %s", > + btd_service_get_error(service), > + name > + ); > return l; > + } > } > > return NULL; > @@ -3282,7 +3290,12 @@ bool btd_device_is_connected(struct btd_device *dev) > > bool btd_device_bearer_is_connected(struct btd_device *dev) > { > - return dev->bredr_state.connected || dev->le_state.connected; > + if(dev->bredr_state.connected || dev->le_state.connected) > + return true; > + else { > + find_service_with_state(dev->services, BTD_SERVICE_STATE_CONNECTED); > + return false; > + }; > } > > static void clear_temporary_timer(struct btd_device *dev) And it seems that every device I have available triggers this: MX Master 3 mouse, Google Pixel Buds Pro, Google Pixel 7. Ran experiments with each one of the devices being connected when running rfkill block: > Apr 04 11:06:31 bluetoothd[331222]: Found service: err: 0, device: MX Master 3 > Apr 04 11:06:46 bluetoothd[331222]: Found service: err: 0, device: MX Master 3 > Apr 04 11:06:58 bluetoothd[331222]: Found service: err: 0, device: MX Master 3 > Apr 04 11:07:28 bluetoothd[331222]: Found service: err: 0, device: coral buds > Apr 04 11:07:29 bluetoothd[331222]: Found service: err: 0, device: coral buds > Apr 04 11:07:29 bluetoothd[331222]: Found service: err: 0, device: coral buds > Apr 04 11:07:29 bluetoothd[331222]: src/profile.c:ext_io_disconnected() Unable to get io data for Hands-Free Voice gateway: getpeername: Transport endpoint is not connected (107) > Apr 04 11:08:01 bluetoothd[331222]: Found service: err: 0, device: coral buds > Apr 04 11:08:01 bluetoothd[331222]: src/profile.c:ext_io_disconnected() Unable to get io data for Hands-Free Voice gateway: getpeername: Transport endpoint is not connected (107) > Apr 04 11:08:40 bluetoothd[331222]: Found service: err: 0, device: coral buds > Apr 04 11:08:40 bluetoothd[331222]: src/profile.c:ext_io_disconnected() Unable to get io data for Hands-Free Voice gateway: getpeername: Transport endpoint is not connected (107) > Apr 04 11:09:47 bluetoothd[331222]: Found service: err: 0, device: p7 > Apr 04 11:09:47 bluetoothd[331222]: src/profile.c:ext_io_disconnected() Unable to get io data for Hands-Free unit: getpeername: Transport endpoint is not connected (107) The BT adapter is a Mediatek MT7922 WiFi/BT M2 adapter, seems to be using the btmtk driver. In parallel, I've sent a V3 of the "bring back previous behavior" patch with the new function named btd_device_bearer_is_connected. D.
diff --git a/src/adapter.c b/src/adapter.c index 4bcc464de..0b7aab4b5 100644 --- a/src/adapter.c +++ b/src/adapter.c @@ -7486,7 +7486,7 @@ static void adapter_remove_connection(struct btd_adapter *adapter, device_cancel_authentication(device, TRUE); /* If another bearer is still connected */ - if (btd_device_is_connected(device)) + if (btd_device_state_is_connected(device)) return; adapter->connections = g_slist_remove(adapter->connections, device); diff --git a/src/device.c b/src/device.c index 5e74633c6..123b1b796 100644 --- a/src/device.c +++ b/src/device.c @@ -3273,13 +3273,18 @@ 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) + if (btd_device_state_is_connected(dev)) return true; return find_service_with_state(dev->services, BTD_SERVICE_STATE_CONNECTED); } +bool btd_device_state_is_connected(struct btd_device *dev) +{ + return dev->bredr_state.connected || dev->le_state.connected; +} + static void clear_temporary_timer(struct btd_device *dev) { if (dev->temporary_timer) { diff --git a/src/device.h b/src/device.h index d4e70b7ef..e3191f2a4 100644 --- a/src/device.h +++ b/src/device.h @@ -104,6 +104,7 @@ void device_set_rssi(struct btd_device *device, int8_t rssi); void device_set_tx_power(struct btd_device *device, int8_t tx_power); void device_set_flags(struct btd_device *device, uint8_t flags); bool btd_device_is_connected(struct btd_device *dev); +bool btd_device_state_is_connected(struct btd_device *dev); uint8_t btd_device_get_bdaddr_type(struct btd_device *dev); bool device_is_retrying(struct btd_device *device); void device_bonding_complete(struct btd_device *device, uint8_t bdaddr_type,