* [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established @ 2025-07-02 3:12 Yang Li via B4 Relay 2025-07-02 13:00 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 5+ messages in thread From: Yang Li via B4 Relay @ 2025-07-02 3:12 UTC (permalink / raw) To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz Cc: linux-bluetooth, linux-kernel, Yang Li From: Yang Li <yang.li@amlogic.com> Hold PA sync connection to prevent cleanup during hci_conn_timeout. Signed-off-by: Yang Li <yang.li@amlogic.com> --- net/bluetooth/hci_event.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index e817c4a77f97..85ae1608e028 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -6968,6 +6968,9 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data, if (!ev->status) { conn->state = BT_CONNECTED; + /* Prevent cleanup in hci_conn_timeout. */ + hci_conn_hold(conn); + set_bit(HCI_CONN_BIG_SYNC, &bis->flags); hci_debugfs_create_conn(conn); hci_conn_add_sysfs(conn); --- base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe change-id: 20250702-hold_pa_sync-91ca7a81adf0 Best regards, -- Yang Li <yang.li@amlogic.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established 2025-07-02 3:12 [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established Yang Li via B4 Relay @ 2025-07-02 13:00 ` Luiz Augusto von Dentz 2025-07-03 8:44 ` Yang Li 0 siblings, 1 reply; 5+ messages in thread From: Luiz Augusto von Dentz @ 2025-07-02 13:00 UTC (permalink / raw) To: yang.li; +Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel Hi, On Tue, Jul 1, 2025 at 11:12 PM Yang Li via B4 Relay <devnull+yang.li.amlogic.com@kernel.org> wrote: > > From: Yang Li <yang.li@amlogic.com> > > Hold PA sync connection to prevent cleanup during hci_conn_timeout. You will need to do a better job explaining what is the issue here. > Signed-off-by: Yang Li <yang.li@amlogic.com> > --- > net/bluetooth/hci_event.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > index e817c4a77f97..85ae1608e028 100644 > --- a/net/bluetooth/hci_event.c > +++ b/net/bluetooth/hci_event.c > @@ -6968,6 +6968,9 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data, > > if (!ev->status) { > conn->state = BT_CONNECTED; > + /* Prevent cleanup in hci_conn_timeout. */ > + hci_conn_hold(conn); > + > set_bit(HCI_CONN_BIG_SYNC, &bis->flags); > hci_debugfs_create_conn(conn); > hci_conn_add_sysfs(conn); > > --- > base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe > change-id: 20250702-hold_pa_sync-91ca7a81adf0 > > Best regards, > -- > Yang Li <yang.li@amlogic.com> > > -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established 2025-07-02 13:00 ` Luiz Augusto von Dentz @ 2025-07-03 8:44 ` Yang Li 2025-07-03 13:21 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 5+ messages in thread From: Yang Li @ 2025-07-03 8:44 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel Hi Luiz, > [ EXTERNAL EMAIL ] > > Hi, > > On Tue, Jul 1, 2025 at 11:12 PM Yang Li via B4 Relay > <devnull+yang.li.amlogic.com@kernel.org> wrote: >> From: Yang Li <yang.li@amlogic.com> >> >> Hold PA sync connection to prevent cleanup during hci_conn_timeout. > You will need to do a better job explaining what is the issue here. Since the PA sync connection is set to BT_CONNECTED in hci_le_big_sync_established_evt, if its status is BT_CONNECTED when hci_abort_conn_sync is called, hci_disconnect_sync() will be executed, which will cause the PA sync connection to be deleted. int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason) { ... switch (conn->state) { case BT_CONNECTED: case BT_CONFIG: err = hci_disconnect_sync(hdev, conn, reason); break; ... stack trace as below: [ 55.154495][0 T1966 d.] CPU: 0 PID: 1966 Comm: kworker/u9:0 Tainted: G O 6.6.77 #104 [ 55.155721][0 T1966 d.] Hardware name: Amlogic (DT) [ 55.156336][0 T1966 d.] Workqueue: hci0 hci_cmd_sync_work [ 55.157018][0 T1966 d.] Call trace: [ 55.157461][0 T1966 d.] dump_backtrace+0x94/0xec [ 55.158056][0 T1966 d.] show_stack+0x18/0x24 [ 55.158607][0 T1966 d.] dump_stack_lvl+0x48/0x60 [ 55.159205][0 T1966 d.] dump_stack+0x18/0x24 [ 55.159756][0 T1966 d.] hci_conn_del+0x1c/0x12c [ 55.160341][0 T1966 d.] hci_conn_failed+0xdc/0x150 [ 55.160958][0 T1966 d.] hci_abort_conn_sync+0x204/0x388 [ 55.161630][0 T1966 d.] abort_conn_sync+0x58/0x80 [ 55.162237][0 T1966 d.] hci_cmd_sync_work+0x94/0x100 [ 55.162877][0 T1966 d.] process_one_work+0x168/0x444 [ 55.163516][0 T1966 d.] worker_thread+0x378/0x3f4 [ 55.164122][0 T1966 d.] kthread+0x108/0x10c [ 55.164664][0 T1966 d.] ret_from_fork+0x10/0x20 [ 55.165408][0 T1966 d.] hci0 hcon 000000004f36962c handle 3841 #PA sync connection btmon trace: < HCI Command: Disconnect (0x01|0x0006) plen 3 #75 [hci0] 14.640630 Handle: 3841 Reason: Remote User Terminated Connection (0x13) > HCI Event: Command Status (0x0f) plen 4 #76 [hci0] 14.642103 Disconnect (0x01|0x0006) ncmd 1 Status: Invalid HCI Command Parameters (0x12) So the current question is whether the PA sync connection, which is marked as BT_CONNECTED, really needs to be disconnected. If it does need to be disconnected, then the PA sync terminate command must be executed. However, in my opinion, the PA sync connection should not be disconnected. > >> Signed-off-by: Yang Li <yang.li@amlogic.com> >> --- >> net/bluetooth/hci_event.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c >> index e817c4a77f97..85ae1608e028 100644 >> --- a/net/bluetooth/hci_event.c >> +++ b/net/bluetooth/hci_event.c >> @@ -6968,6 +6968,9 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data, >> >> if (!ev->status) { >> conn->state = BT_CONNECTED; >> + /* Prevent cleanup in hci_conn_timeout. */ >> + hci_conn_hold(conn); >> + >> set_bit(HCI_CONN_BIG_SYNC, &bis->flags); >> hci_debugfs_create_conn(conn); >> hci_conn_add_sysfs(conn); >> >> --- >> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe >> change-id: 20250702-hold_pa_sync-91ca7a81adf0 >> >> Best regards, >> -- >> Yang Li <yang.li@amlogic.com> >> >> > > -- > Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established 2025-07-03 8:44 ` Yang Li @ 2025-07-03 13:21 ` Luiz Augusto von Dentz 2025-07-03 14:11 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 5+ messages in thread From: Luiz Augusto von Dentz @ 2025-07-03 13:21 UTC (permalink / raw) To: Yang Li; +Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel Hi, On Thu, Jul 3, 2025 at 4:45 AM Yang Li <yang.li@amlogic.com> wrote: > > Hi Luiz, > > [ EXTERNAL EMAIL ] > > > > Hi, > > > > On Tue, Jul 1, 2025 at 11:12 PM Yang Li via B4 Relay > > <devnull+yang.li.amlogic.com@kernel.org> wrote: > >> From: Yang Li <yang.li@amlogic.com> > >> > >> Hold PA sync connection to prevent cleanup during hci_conn_timeout. > > You will need to do a better job explaining what is the issue here. > > > Since the PA sync connection is set to BT_CONNECTED in > hci_le_big_sync_established_evt, if its status is BT_CONNECTED when > hci_abort_conn_sync is called, hci_disconnect_sync() will be executed, > which will cause the PA sync connection to be deleted. > > int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 > reason) > { > ... > switch (conn->state) { > case BT_CONNECTED: > case BT_CONFIG: > err = hci_disconnect_sync(hdev, conn, reason); > break; > ... > > stack trace as below: > > [ 55.154495][0 T1966 d.] CPU: 0 PID: 1966 Comm: kworker/u9:0 Tainted: > G O 6.6.77 #104 > [ 55.155721][0 T1966 d.] Hardware name: Amlogic (DT) > [ 55.156336][0 T1966 d.] Workqueue: hci0 hci_cmd_sync_work > [ 55.157018][0 T1966 d.] Call trace: > [ 55.157461][0 T1966 d.] dump_backtrace+0x94/0xec > [ 55.158056][0 T1966 d.] show_stack+0x18/0x24 > [ 55.158607][0 T1966 d.] dump_stack_lvl+0x48/0x60 > [ 55.159205][0 T1966 d.] dump_stack+0x18/0x24 > [ 55.159756][0 T1966 d.] hci_conn_del+0x1c/0x12c > [ 55.160341][0 T1966 d.] hci_conn_failed+0xdc/0x150 > [ 55.160958][0 T1966 d.] hci_abort_conn_sync+0x204/0x388 > [ 55.161630][0 T1966 d.] abort_conn_sync+0x58/0x80 > [ 55.162237][0 T1966 d.] hci_cmd_sync_work+0x94/0x100 > [ 55.162877][0 T1966 d.] process_one_work+0x168/0x444 > [ 55.163516][0 T1966 d.] worker_thread+0x378/0x3f4 > [ 55.164122][0 T1966 d.] kthread+0x108/0x10c > [ 55.164664][0 T1966 d.] ret_from_fork+0x10/0x20 > [ 55.165408][0 T1966 d.] hci0 hcon 000000004f36962c handle 3841 #PA > sync connection > > > btmon trace: > > < HCI Command: Disconnect (0x01|0x0006) plen 3 #75 [hci0] > 14.640630 > Handle: 3841 > Reason: Remote User Terminated Connection (0x13) > > HCI Event: Command Status (0x0f) plen 4 #76 [hci0] > 14.642103 > Disconnect (0x01|0x0006) ncmd 1 > Status: Invalid HCI Command Parameters (0x12) > Ok, so this must be fixed and not worked around. > So the current question is whether the PA sync connection, which is > marked as BT_CONNECTED, really needs to be disconnected. > If it does need to be disconnected, then the PA sync terminate command > must be executed. > However, in my opinion, the PA sync connection should not be disconnected. Why not? The fact that it is not being cleanup properly doesn't mean it shouldn't be terminated, otherwise we will have a dangling hci_conn. > > > >> Signed-off-by: Yang Li <yang.li@amlogic.com> > >> --- > >> net/bluetooth/hci_event.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > >> index e817c4a77f97..85ae1608e028 100644 > >> --- a/net/bluetooth/hci_event.c > >> +++ b/net/bluetooth/hci_event.c > >> @@ -6968,6 +6968,9 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data, > >> > >> if (!ev->status) { > >> conn->state = BT_CONNECTED; > >> + /* Prevent cleanup in hci_conn_timeout. */ > >> + hci_conn_hold(conn); > >> + > >> set_bit(HCI_CONN_BIG_SYNC, &bis->flags); > >> hci_debugfs_create_conn(conn); > >> hci_conn_add_sysfs(conn); > >> > >> --- > >> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe > >> change-id: 20250702-hold_pa_sync-91ca7a81adf0 > >> > >> Best regards, > >> -- > >> Yang Li <yang.li@amlogic.com> > >> > >> > > > > -- > > Luiz Augusto von Dentz -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established 2025-07-03 13:21 ` Luiz Augusto von Dentz @ 2025-07-03 14:11 ` Luiz Augusto von Dentz 0 siblings, 0 replies; 5+ messages in thread From: Luiz Augusto von Dentz @ 2025-07-03 14:11 UTC (permalink / raw) To: Yang Li; +Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel Hi, On Thu, Jul 3, 2025 at 9:21 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi, > > On Thu, Jul 3, 2025 at 4:45 AM Yang Li <yang.li@amlogic.com> wrote: > > > > Hi Luiz, > > > [ EXTERNAL EMAIL ] > > > > > > Hi, > > > > > > On Tue, Jul 1, 2025 at 11:12 PM Yang Li via B4 Relay > > > <devnull+yang.li.amlogic.com@kernel.org> wrote: > > >> From: Yang Li <yang.li@amlogic.com> > > >> > > >> Hold PA sync connection to prevent cleanup during hci_conn_timeout. > > > You will need to do a better job explaining what is the issue here. > > > > > > Since the PA sync connection is set to BT_CONNECTED in > > hci_le_big_sync_established_evt, if its status is BT_CONNECTED when > > hci_abort_conn_sync is called, hci_disconnect_sync() will be executed, > > which will cause the PA sync connection to be deleted. > > > > int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 > > reason) > > { > > ... > > switch (conn->state) { > > case BT_CONNECTED: > > case BT_CONFIG: > > err = hci_disconnect_sync(hdev, conn, reason); > > break; > > ... > > > > stack trace as below: > > > > [ 55.154495][0 T1966 d.] CPU: 0 PID: 1966 Comm: kworker/u9:0 Tainted: > > G O 6.6.77 #104 > > [ 55.155721][0 T1966 d.] Hardware name: Amlogic (DT) > > [ 55.156336][0 T1966 d.] Workqueue: hci0 hci_cmd_sync_work > > [ 55.157018][0 T1966 d.] Call trace: > > [ 55.157461][0 T1966 d.] dump_backtrace+0x94/0xec > > [ 55.158056][0 T1966 d.] show_stack+0x18/0x24 > > [ 55.158607][0 T1966 d.] dump_stack_lvl+0x48/0x60 > > [ 55.159205][0 T1966 d.] dump_stack+0x18/0x24 > > [ 55.159756][0 T1966 d.] hci_conn_del+0x1c/0x12c > > [ 55.160341][0 T1966 d.] hci_conn_failed+0xdc/0x150 > > [ 55.160958][0 T1966 d.] hci_abort_conn_sync+0x204/0x388 > > [ 55.161630][0 T1966 d.] abort_conn_sync+0x58/0x80 > > [ 55.162237][0 T1966 d.] hci_cmd_sync_work+0x94/0x100 > > [ 55.162877][0 T1966 d.] process_one_work+0x168/0x444 > > [ 55.163516][0 T1966 d.] worker_thread+0x378/0x3f4 > > [ 55.164122][0 T1966 d.] kthread+0x108/0x10c > > [ 55.164664][0 T1966 d.] ret_from_fork+0x10/0x20 > > [ 55.165408][0 T1966 d.] hci0 hcon 000000004f36962c handle 3841 #PA > > sync connection > > > > > > btmon trace: > > > > < HCI Command: Disconnect (0x01|0x0006) plen 3 #75 [hci0] > > 14.640630 > > Handle: 3841 > > Reason: Remote User Terminated Connection (0x13) > > > HCI Event: Command Status (0x0f) plen 4 #76 [hci0] > > 14.642103 > > Disconnect (0x01|0x0006) ncmd 1 > > Status: Invalid HCI Command Parameters (0x12) > > > > Ok, so this must be fixed and not worked around. > > > So the current question is whether the PA sync connection, which is > > marked as BT_CONNECTED, really needs to be disconnected. > > If it does need to be disconnected, then the PA sync terminate command > > must be executed. > > However, in my opinion, the PA sync connection should not be disconnected. > > Why not? The fact that it is not being cleanup properly doesn't mean > it shouldn't be terminated, otherwise we will have a dangling > hci_conn. Looks like Ive messed up with the fix to mark the BIS as connected, it marked the PA instead which is probably wrong and perhaps it is why you were saying it shouldn't be terminated since the PA sync is terminated when the last BIS is terminated. Anyway I will be fixing up the PA being set to connected rather than BIS, so please rebase. > > > > > >> Signed-off-by: Yang Li <yang.li@amlogic.com> > > >> --- > > >> net/bluetooth/hci_event.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c > > >> index e817c4a77f97..85ae1608e028 100644 > > >> --- a/net/bluetooth/hci_event.c > > >> +++ b/net/bluetooth/hci_event.c > > >> @@ -6968,6 +6968,9 @@ static void hci_le_big_sync_established_evt(struct hci_dev *hdev, void *data, > > >> > > >> if (!ev->status) { > > >> conn->state = BT_CONNECTED; > > >> + /* Prevent cleanup in hci_conn_timeout. */ > > >> + hci_conn_hold(conn); > > >> + > > >> set_bit(HCI_CONN_BIG_SYNC, &bis->flags); > > >> hci_debugfs_create_conn(conn); > > >> hci_conn_add_sysfs(conn); > > >> > > >> --- > > >> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe > > >> change-id: 20250702-hold_pa_sync-91ca7a81adf0 > > >> > > >> Best regards, > > >> -- > > >> Yang Li <yang.li@amlogic.com> > > >> > > >> > > > > > > -- > > > Luiz Augusto von Dentz > > > > -- > Luiz Augusto von Dentz -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-07-03 14:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-07-02 3:12 [PATCH] Bluetooth: hci_event: Hold PA sync conn when BIG sync established Yang Li via B4 Relay 2025-07-02 13:00 ` Luiz Augusto von Dentz 2025-07-03 8:44 ` Yang Li 2025-07-03 13:21 ` Luiz Augusto von Dentz 2025-07-03 14:11 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®