Hi Ruihan,
On Tue, May 2, 2023 at 7:57 AM Ruihan Li <lrh2000@xxxxxxxxxx> wrote:
>
> Previously, hci_conn_unlink was implemented as a recursion function. To
> unlink physical connections (e.g. ACL/LE), it calls itself to unlink all
> its logical channels (e.g. SCO/eSCO/ISO).
>
> Recursion is not required. This patch refactors hci_conn_unlink into two
> functions, where hci_conn_unlink_parent takes a physical connection,
> checks out all its logical channels, and calls hci_conn_unlink_child for
> each logical channel to unlink it.
>
> Signed-off-by: Ruihan Li <lrh2000@xxxxxxxxxx>
> ---
> net/bluetooth/hci_conn.c | 55 +++++++++++++++++++++++-----------------
> 1 file changed, 32 insertions(+), 23 deletions(-)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index de553e062..243d68a64 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -1074,34 +1074,13 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
> return conn;
> }
>
> -static void hci_conn_unlink(struct hci_conn *conn)
> +static void hci_conn_unlink_parent(struct hci_conn *conn)
> {
> struct hci_dev *hdev = conn->hdev;
>
> bt_dev_dbg(hdev, "hcon %p", conn);
>
> - if (!conn->parent) {
> - struct hci_link *link, *t;
> -
> - list_for_each_entry_safe(link, t, &conn->link_list, list) {
> - struct hci_conn *child = link->conn;
> -
> - hci_conn_unlink(child);
> -
> - /* Due to race, SCO connection might be not established
> - * yet at this point. Delete it now, otherwise it is
> - * possible for it to be stuck and can't be deleted.
> - */
> - if ((child->type == SCO_LINK ||
> - child->type == ESCO_LINK) &&
> - child->handle == HCI_CONN_HANDLE_UNSET)
> - hci_conn_del(child);
> - }
> -
> - return;
> - }
> -
> - if (!conn->link)
> + if (WARN_ON(!conn->link))
> return;
>
> list_del_rcu(&conn->link->list);
> @@ -1115,6 +1094,36 @@ static void hci_conn_unlink(struct hci_conn *conn)
> conn->link = NULL;
> }
>
> +static void hci_conn_unlink_children(struct hci_conn *conn)
> +{
> + struct hci_dev *hdev = conn->hdev;
> + struct hci_link *link, *t;
> +
> + bt_dev_dbg(hdev, "hcon %p", conn);
> +
> + list_for_each_entry_safe(link, t, &conn->link_list, list) {
> + struct hci_conn *child = link->conn;
> +
> + hci_conn_unlink_parent(child);
> +
> + /* Due to race, SCO connection might be not established
> + * yet at this point. Delete it now, otherwise it is
> + * possible for it to be stuck and can't be deleted.
> + */
> + if (child->type == SCO_LINK || child->type == ESCO_LINK)
> + if (child->handle == HCI_CONN_HANDLE_UNSET)
> + hci_conn_del(child);
> + }
This is not quite right, when we are unlinking the children's hci_conn
it shall only unlink itself from the parent not everything.
> +}
> +
> +static void hci_conn_unlink(struct hci_conn *conn)
> +{
> + if (conn->parent)
> + hci_conn_unlink_parent(conn);
> + else
> + hci_conn_unlink_children(conn);
> +}
> +
> void hci_conn_del(struct hci_conn *conn)
> {
> struct hci_dev *hdev = conn->hdev;
> --
> 2.40.0
>
--
Luiz Augusto von Dentz
