| Message ID | 20220720163548.kernel.v1.1.I4058a198aa4979ee74a219fe6e315fad1184d78d@changeid (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix get clock info | expand |
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 >
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;
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(-)