[RFC,1/6] Bluetooth: hci_conn: hci_conn_cleanup is not needed, combine with del

Commit Message

Pauli Virtanen July 26, 2023, 9:25 p.m. UTC

hci_conn_cleanup is no longer needed, so move the code back to
hci_conn_del to keep the hci_conn teardown in a single place.

This undoes commit b958f9a3e877 ("Bluetooth: Fix reference counting for
LE-scan based connections"), but keeps the current order of cleanup
operations.

Signed-off-by: Pauli Virtanen <[e-mail]>
---
 net/bluetooth/hci_conn.c | 78 +++++++++++++++++-----------------------
 1 file changed, 33 insertions(+), 45 deletions(-)

Comments

[e-mail] July 26, 2023, 10:01 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=769854

---Test result---

Test Summary:
CheckPatch                    PASS      5.24 seconds
GitLint                       FAIL      2.16 seconds
SubjectPrefix                 PASS      0.75 seconds
BuildKernel                   PASS      32.99 seconds
CheckAllWarning               PASS      36.22 seconds
CheckSparse                   PASS      42.20 seconds
CheckSmatch                   PASS      112.30 seconds
BuildKernel32                 PASS      31.92 seconds
TestRunnerSetup               PASS      484.75 seconds
TestRunner_l2cap-tester       PASS      23.23 seconds
TestRunner_iso-tester         PASS      44.83 seconds
TestRunner_bnep-tester        PASS      10.64 seconds
TestRunner_mgmt-tester        PASS      216.30 seconds
TestRunner_rfcomm-tester      PASS      16.08 seconds
TestRunner_sco-tester         PASS      16.79 seconds
TestRunner_ioctl-tester       PASS      17.90 seconds
TestRunner_mesh-tester        PASS      13.44 seconds
TestRunner_smp-tester         PASS      14.31 seconds
TestRunner_userchan-tester    PASS      11.30 seconds
IncrementalBuild              PASS      102.70 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[RFC,2/6] Bluetooth: hci_conn: add hci_conn_is_alive

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
22: B2 Line has trailing whitespace: "    "
28: B2 Line has trailing whitespace: "    "
33: B2 Line has trailing whitespace: "    "
37: B2 Line has trailing whitespace: "    "
[RFC,3/6] Bluetooth: hci_sync: add hci_conn_sync_queue and hci_cmd_sync_dev_(un)lock

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (84>80): "[RFC,3/6] Bluetooth: hci_sync: add hci_conn_sync_queue and hci_cmd_sync_dev_(un)lock"
[RFC,4/6] Bluetooth: hci_sync: fix locking in hci_conn_abort and hci_disconnect_all_sync

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (88>80): "[RFC,4/6] Bluetooth: hci_sync: fix locking in hci_conn_abort and hci_disconnect_all_sync"


---
Regards,
Linux Bluetooth

Paul Menzel July 27, 2023, 4:14 a.m. UTC | #2

Dear Pauli,


Thank you for your patch.

You might want to make the commit message summary a statement about the 
action. Maybe:

Combine unneeded hci_conn_cleanup() with hci_conn_del()

Am 26.07.23 um 23:25 schrieb Pauli Virtanen:
> hci_conn_cleanup is no longer needed, so move the code back to

Why is it no longer needed?

> hci_conn_del to keep the hci_conn teardown in a single place.
> 
> This undoes commit b958f9a3e877 ("Bluetooth: Fix reference counting for
> LE-scan based connections"), but keeps the current order of cleanup
> operations.
> 
> Signed-off-by: Pauli Virtanen <[e-mail]>
> ---
>   net/bluetooth/hci_conn.c | 78 +++++++++++++++++-----------------------
>   1 file changed, 33 insertions(+), 45 deletions(-)

[…]


Kind regards,

Paul

Patch

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index cccc2b8b60a8..a71a54a5c8d8 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -139,45 +139,6 @@  static void hci_connect_le_scan_cleanup(struct hci_conn *conn, u8 status)
 	hci_update_passive_scan(hdev);
 }
 
-static void hci_conn_cleanup(struct hci_conn *conn)
-{
-	struct hci_dev *hdev = conn->hdev;
-
-	if (test_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags))
-		hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
-
-	if (test_and_clear_bit(HCI_CONN_FLUSH_KEY, &conn->flags))
-		hci_remove_link_key(hdev, &conn->dst);
-
-	hci_chan_list_flush(conn);
-
-	hci_conn_hash_del(hdev, conn);
-
-	if (conn->cleanup)
-		conn->cleanup(conn);
-
-	if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
-		switch (conn->setting & SCO_AIRMODE_MASK) {
-		case SCO_AIRMODE_CVSD:
-		case SCO_AIRMODE_TRANSP:
-			if (hdev->notify)
-				hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO);
-			break;
-		}
-	} else {
-		if (hdev->notify)
-			hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
-	}
-
-	hci_conn_del_sysfs(conn);
-
-	debugfs_remove_recursive(conn->debugfs);
-
-	hci_dev_put(hdev);
-
-	hci_conn_put(conn);
-}
-
 static void hci_acl_create_connection(struct hci_conn *conn)
 {
 	struct hci_dev *hdev = conn->hdev;
@@ -1127,12 +1088,39 @@  void hci_conn_del(struct hci_conn *conn)
 
 	skb_queue_purge(&conn->data_q);
 
-	/* Remove the connection from the list and cleanup its remaining
-	 * state. This is a separate function since for some cases like
-	 * BT_CONNECT_SCAN we *only* want the cleanup part without the
-	 * rest of hci_conn_del.
-	 */
-	hci_conn_cleanup(conn);
+	if (test_bit(HCI_CONN_PARAM_REMOVAL_PEND, &conn->flags))
+		hci_conn_params_del(conn->hdev, &conn->dst, conn->dst_type);
+
+	if (test_and_clear_bit(HCI_CONN_FLUSH_KEY, &conn->flags))
+		hci_remove_link_key(hdev, &conn->dst);
+
+	hci_chan_list_flush(conn);
+
+	hci_conn_hash_del(hdev, conn);
+
+	if (conn->cleanup)
+		conn->cleanup(conn);
+
+	if (conn->type == SCO_LINK || conn->type == ESCO_LINK) {
+		switch (conn->setting & SCO_AIRMODE_MASK) {
+		case SCO_AIRMODE_CVSD:
+		case SCO_AIRMODE_TRANSP:
+			if (hdev->notify)
+				hdev->notify(hdev, HCI_NOTIFY_DISABLE_SCO);
+			break;
+		}
+	} else {
+		if (hdev->notify)
+			hdev->notify(hdev, HCI_NOTIFY_CONN_DEL);
+	}
+
+	hci_conn_del_sysfs(conn);
+
+	debugfs_remove_recursive(conn->debugfs);
+
+	hci_dev_put(hdev);
+
+	hci_conn_put(conn);
 }
 
 struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type)