* [kernel PATCH v1 0/1] Fix get clock info
@ 2022-07-20 23:36 Zhengping Jiang
2022-07-20 23:36 ` [kernel PATCH v1 1/1] Bluetooth: " Zhengping Jiang
0 siblings, 1 reply; 3+ messages in thread
From: Zhengping Jiang @ 2022-07-20 23:36 UTC (permalink / raw)
To: linux-bluetooth, marcel
Cc: Zhengping Jiang, David S. Miller, Eric Dumazet, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, Paolo Abeni, linux-kernel,
netdev
Similar to get conn info function, the function to get clock info also
need to set connection data.
Fixes: 5a75013746640 ("Bluetooth: hci_sync: Convert MGMT_OP_GET_CLOCK_INFO")
Changes in v1:
- Fix input connection data
Zhengping Jiang (1):
Bluetooth: Fix get clock info
net/bluetooth/mgmt.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply [flat|nested] 3+ messages in thread
* [kernel PATCH v1 1/1] Bluetooth: Fix get clock info
2022-07-20 23:36 [kernel PATCH v1 0/1] Fix get clock info Zhengping Jiang
@ 2022-07-20 23:36 ` Zhengping Jiang
2022-07-21 22:20 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 3+ messages in thread
From: Zhengping Jiang @ 2022-07-20 23:36 UTC (permalink / raw)
To: linux-bluetooth, marcel
Cc: Zhengping Jiang, David S. Miller, Eric Dumazet, Jakub Kicinski,
Johan Hedberg, Luiz Augusto von Dentz, Paolo Abeni, linux-kernel,
netdev
If connection exists, set the connection data before calling
get_clock_info_sync, so it can be verified the connection is still
connected, before retrieving clock info.
Signed-off-by: Zhengping Jiang <jiangzp@google.com>
---
Changes in v1:
- Fix input connection data
net/bluetooth/mgmt.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index ef8371975c4eb..947d700574c54 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6971,11 +6971,16 @@ static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,
}
cmd = mgmt_pending_new(sk, MGMT_OP_GET_CLOCK_INFO, hdev, data, len);
- if (!cmd)
+ if (!cmd) {
err = -ENOMEM;
- else
+ } else {
+ if (conn) {
+ hci_conn_hold(conn);
+ cmd->user_data = hci_conn_get(conn);
+ }
err = hci_cmd_sync_queue(hdev, get_clock_info_sync, cmd,
get_clock_info_complete);
+ }
if (err < 0) {
err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_GET_CLOCK_INFO,
@@ -6984,12 +6989,8 @@ static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,
if (cmd)
mgmt_pending_free(cmd);
- } else if (conn) {
- hci_conn_hold(conn);
- cmd->user_data = hci_conn_get(conn);
}
-
unlock:
hci_dev_unlock(hdev);
return err;
--
2.37.0.170.g444d1eabd0-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [kernel PATCH v1 1/1] Bluetooth: Fix get clock info
2022-07-20 23:36 ` [kernel PATCH v1 1/1] Bluetooth: " Zhengping Jiang
@ 2022-07-21 22:20 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 3+ messages in thread
From: Luiz Augusto von Dentz @ 2022-07-21 22:20 UTC (permalink / raw)
To: Zhengping Jiang
Cc: linux-bluetooth, Marcel Holtmann, David S. Miller, Eric Dumazet,
Jakub Kicinski, Johan Hedberg, Paolo Abeni,
Linux Kernel Mailing List, open list:NETWORKING [GENERAL]
Hi Zhengping,
On Wed, Jul 20, 2022 at 4:36 PM Zhengping Jiang <jiangzp@google.com> wrote:
>
> If connection exists, set the connection data before calling
> get_clock_info_sync, so it can be verified the connection is still
> connected, before retrieving clock info.
>
> Signed-off-by: Zhengping Jiang <jiangzp@google.com>
> ---
>
> Changes in v1:
> - Fix input connection data
>
> net/bluetooth/mgmt.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index ef8371975c4eb..947d700574c54 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -6971,11 +6971,16 @@ static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,
> }
>
> cmd = mgmt_pending_new(sk, MGMT_OP_GET_CLOCK_INFO, hdev, data, len);
> - if (!cmd)
> + if (!cmd) {
> err = -ENOMEM;
> - else
> + } else {
> + if (conn) {
> + hci_conn_hold(conn);
> + cmd->user_data = hci_conn_get(conn);
> + }
> err = hci_cmd_sync_queue(hdev, get_clock_info_sync, cmd,
> get_clock_info_complete);
> + }
Having seconds though if this is actually the right thing to do,
hci_cmd_sync_queue will queue the command so get_clock_info_sync
shouldn't execute immediately if that happens that actually would be
quite a problem since we are holding the hci_dev_lock so if the
callback executes and blocks waiting a command that would likely cause
a deadlock because no command can be completed while hci_dev_lock is
being held, in fact I don't really like the idea of holding hci_conn
reference either since we are doing a lookup by address on
get_clock_info_sync Id probably just remove this code as it seem
unnecessary.
Btw, there are tests for this command in mgmt-tester so if this is
actually attempting to fix a problem Id like to have a test to
reproduce it.
> if (err < 0) {
> err = mgmt_cmd_complete(sk, hdev->id, MGMT_OP_GET_CLOCK_INFO,
> @@ -6984,12 +6989,8 @@ static int get_clock_info(struct sock *sk, struct hci_dev *hdev, void *data,
> if (cmd)
> mgmt_pending_free(cmd);
>
> - } else if (conn) {
> - hci_conn_hold(conn);
> - cmd->user_data = hci_conn_get(conn);
> }
>
> -
> unlock:
> hci_dev_unlock(hdev);
> return err;
> --
> 2.37.0.170.g444d1eabd0-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-07-21 22:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20 23:36 [kernel PATCH v1 0/1] Fix get clock info Zhengping Jiang
2022-07-20 23:36 ` [kernel PATCH v1 1/1] Bluetooth: " Zhengping Jiang
2022-07-21 22:20 ` 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;
as well as URLs for NNTP newsgroup(s).