* [PATCH v4] device: Remove device after all bearers are disconnected @ 2024-09-27 2:38 Cheng Jiang 2024-09-27 4:42 ` [v4] " bluez.test.bot 2024-09-27 14:03 ` [PATCH v4] " Luiz Augusto von Dentz 0 siblings, 2 replies; 4+ messages in thread From: Cheng Jiang @ 2024-09-27 2:38 UTC (permalink / raw) To: linux-bluetooth; +Cc: quic_jiaymao For a combo mode remote, both BR/EDR and BLE may be connected. RemoveDevice should be handled after all bearers are dropped, otherwise, remove device is not performed in adapter_remove_connection. --- src/device.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/device.c b/src/device.c index f8f61e643..4efd755a0 100644 --- a/src/device.c +++ b/src/device.c @@ -3492,8 +3492,27 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, DBusMessage *msg = device->disconnects->data; if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, - "RemoveDevice")) + "RemoveDevice")) { + + /* Don't handle the RemoveDevice msg if device is + * connected. For a dual-mode remote, both BR/EDR + * and BLE may be connected, RemoveDevice should + * be handled after all bearers are disconnects. + * Otherwise, if msg is removed, but not all + * connection are dropped, this function returns + * before *remove is updated, then after all + * connections are dropped, but device->disconnects + * is NULL, remove_device is not updated. Consequently + * *remove is not set to true. The device is not removed + * for adapter in adapter_remove_connection. Need + * to wait for temporary device timeout to remove + * the device. + */ + if (device->bredr_state.connected || + device->le_state.connected) + break; remove_device = true; + } g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); device->disconnects = g_slist_remove(device->disconnects, msg); -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [v4] device: Remove device after all bearers are disconnected 2024-09-27 2:38 [PATCH v4] device: Remove device after all bearers are disconnected Cheng Jiang @ 2024-09-27 4:42 ` bluez.test.bot 2024-09-27 14:03 ` [PATCH v4] " Luiz Augusto von Dentz 1 sibling, 0 replies; 4+ messages in thread From: bluez.test.bot @ 2024-09-27 4:42 UTC (permalink / raw) To: linux-bluetooth, quic_chejiang [-- Attachment #1: Type: text/plain, Size: 948 bytes --] 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=893302 ---Test result--- Test Summary: CheckPatch PASS 0.49 seconds GitLint PASS 0.46 seconds BuildEll PASS 24.05 seconds BluezMake PASS 1507.72 seconds MakeCheck PASS 13.23 seconds MakeDistcheck PASS 174.81 seconds CheckValgrind PASS 246.55 seconds CheckSmatch PASS 347.47 seconds bluezmakeextell PASS 117.03 seconds IncrementalBuild PASS 1357.42 seconds ScanBuild PASS 992.35 seconds --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v4] device: Remove device after all bearers are disconnected 2024-09-27 2:38 [PATCH v4] device: Remove device after all bearers are disconnected Cheng Jiang 2024-09-27 4:42 ` [v4] " bluez.test.bot @ 2024-09-27 14:03 ` Luiz Augusto von Dentz 2024-09-29 2:25 ` Cheng Jiang 1 sibling, 1 reply; 4+ messages in thread From: Luiz Augusto von Dentz @ 2024-09-27 14:03 UTC (permalink / raw) To: Cheng Jiang; +Cc: linux-bluetooth, quic_jiaymao Hi Cheng, On Thu, Sep 26, 2024 at 10:39 PM Cheng Jiang <quic_chejiang@quicinc.com> wrote: > > For a combo mode remote, both BR/EDR and BLE may be connected. > RemoveDevice should be handled after all bearers are dropped, > otherwise, remove device is not performed in adapter_remove_connection. > --- > src/device.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/src/device.c b/src/device.c > index f8f61e643..4efd755a0 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -3492,8 +3492,27 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, > DBusMessage *msg = device->disconnects->data; > > if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, > - "RemoveDevice")) > + "RemoveDevice")) { > + > + /* Don't handle the RemoveDevice msg if device is > + * connected. For a dual-mode remote, both BR/EDR > + * and BLE may be connected, RemoveDevice should > + * be handled after all bearers are disconnects. > + * Otherwise, if msg is removed, but not all > + * connection are dropped, this function returns > + * before *remove is updated, then after all > + * connections are dropped, but device->disconnects > + * is NULL, remove_device is not updated. Consequently > + * *remove is not set to true. The device is not removed > + * for adapter in adapter_remove_connection. Need > + * to wait for temporary device timeout to remove > + * the device. > + */ I mean as git description, anyway Im having second thoughts about this change, see below. > + if (device->bredr_state.connected || > + device->le_state.connected) > + break; > remove_device = true; > + } > > g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); > device->disconnects = g_slist_remove(device->disconnects, msg); How we move the block checking for disconnect message after check all bearers have been disconnected: diff --git a/src/device.c b/src/device.c index f8f61e64376c..76d2c859c747 100644 --- a/src/device.c +++ b/src/device.c @@ -3488,18 +3488,6 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, device->connect = NULL; } - while (device->disconnects) { - DBusMessage *msg = device->disconnects->data; - - if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, - "RemoveDevice")) - remove_device = true; - - g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); - device->disconnects = g_slist_remove(device->disconnects, msg); - dbus_message_unref(msg); - } - /* Check paired status of both bearers since it's possible to be /* Check paired status of both bearers since it's possible to be /* Check paired status of both bearers since it's possible to be * paired but not connected via link key to LTK conversion. */ @@ -3539,6 +3527,18 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, g_dbus_emit_property_changed(dbus_conn, device->path, DEVICE_INTERFACE, "Connected"); + while (device->disconnects) { + DBusMessage *msg = device->disconnects->data; + + if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, + "RemoveDevice")) + remove_device = true; + + g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); + device->disconnects = g_slist_remove(device->disconnects, msg); + dbus_message_unref(msg); + } + if (remove_device) *remove = remove_device; > -- > 2.25.1 > > -- Luiz Augusto von Dentz ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v4] device: Remove device after all bearers are disconnected 2024-09-27 14:03 ` [PATCH v4] " Luiz Augusto von Dentz @ 2024-09-29 2:25 ` Cheng Jiang 0 siblings, 0 replies; 4+ messages in thread From: Cheng Jiang @ 2024-09-29 2:25 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth, quic_jiaymao Hi Luiz, On 9/27/2024 10:03 PM, Luiz Augusto von Dentz wrote: > Hi Cheng, > > On Thu, Sep 26, 2024 at 10:39 PM Cheng Jiang <quic_chejiang@quicinc.com> wrote: >> >> For a combo mode remote, both BR/EDR and BLE may be connected. >> RemoveDevice should be handled after all bearers are dropped, >> otherwise, remove device is not performed in adapter_remove_connection. >> --- >> src/device.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/src/device.c b/src/device.c >> index f8f61e643..4efd755a0 100644 >> --- a/src/device.c >> +++ b/src/device.c >> @@ -3492,8 +3492,27 @@ void device_remove_connection(struct btd_device *device, uint8_t bdaddr_type, >> DBusMessage *msg = device->disconnects->data; >> >> if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, >> - "RemoveDevice")) >> + "RemoveDevice")) { >> + >> + /* Don't handle the RemoveDevice msg if device is >> + * connected. For a dual-mode remote, both BR/EDR >> + * and BLE may be connected, RemoveDevice should >> + * be handled after all bearers are disconnects. >> + * Otherwise, if msg is removed, but not all >> + * connection are dropped, this function returns >> + * before *remove is updated, then after all >> + * connections are dropped, but device->disconnects >> + * is NULL, remove_device is not updated. Consequently >> + * *remove is not set to true. The device is not removed >> + * for adapter in adapter_remove_connection. Need >> + * to wait for temporary device timeout to remove >> + * the device. >> + */ > > I mean as git description, anyway Im having second thoughts about this > change, see below. > >> + if (device->bredr_state.connected || >> + device->le_state.connected) >> + break; >> remove_device = true; >> + } >> >> g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); >> device->disconnects = g_slist_remove(device->disconnects, msg); > > How we move the block checking for disconnect message after check all > bearers have been disconnected: > > diff --git a/src/device.c b/src/device.c > index f8f61e64376c..76d2c859c747 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -3488,18 +3488,6 @@ void device_remove_connection(struct btd_device > *device, uint8_t bdaddr_type, > device->connect = NULL; > } > > - while (device->disconnects) { > - DBusMessage *msg = device->disconnects->data; > - > - if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, > - "RemoveDevice")) > - remove_device = true; > - > - g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); > - device->disconnects = g_slist_remove(device->disconnects, msg); > - dbus_message_unref(msg); > - } > - > /* Check paired status of both bearers since it's possible to be > /* Check paired status of both bearers since it's possible to be > /* Check paired status of both bearers since it's possible to be > * paired but not connected via link key to LTK conversion. > */ > @@ -3539,6 +3527,18 @@ void device_remove_connection(struct btd_device > *device, uint8_t bdaddr_type, > g_dbus_emit_property_changed(dbus_conn, device->path, > DEVICE_INTERFACE, "Connected"); > > + while (device->disconnects) { > + DBusMessage *msg = device->disconnects->data; > + > + if (dbus_message_is_method_call(msg, ADAPTER_INTERFACE, > + "RemoveDevice")) > + remove_device = true; > + > + g_dbus_send_reply(dbus_conn, msg, DBUS_TYPE_INVALID); > + device->disconnects = g_slist_remove(device->disconnects, msg); > + dbus_message_unref(msg); > + } > + > if (remove_device) > *remove = remove_device; > > Agree with you. Update the patch (v5). Thank you! >> -- >> 2.25.1 >> >> > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-09-29 2:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-27 2:38 [PATCH v4] device: Remove device after all bearers are disconnected Cheng Jiang 2024-09-27 4:42 ` [v4] " bluez.test.bot 2024-09-27 14:03 ` [PATCH v4] " Luiz Augusto von Dentz 2024-09-29 2:25 ` Cheng Jiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox