2021-02-12 01:35:01

by Miao-chen Chou

[permalink] [raw]
Subject: Re: [PATCH v4] Bluetooth: Keep MSFT ext info throughout a hci_dev's life cycle

Hi Marcel,

A friendly ping on this patch. :)

Regards,
Miao

On Wed, Jan 27, 2021 at 9:17 AM Miao-chen Chou <[email protected]> wrote:
>
> This moves msft_do_close() from hci_dev_do_close() to
> hci_unregister_dev() to avoid clearing MSFT extension info. This also
> re-reads MSFT info upon every msft_do_open() even if MSFT extension has
> been initialized.
>
> The following test steps were performed.
> (1) boot the test device and verify the MSFT support debug log in syslog
> (2) restart bluetoothd and verify msft_do_close() doesn't get invoked
> and msft_do_open re-reads the MSFT support.
>
> Signed-off-by: Miao-chen Chou <[email protected]>
> Reviewed-by: Abhishek Pandit-Subedi <[email protected]>
> Reviewed-by: Archie Pusaka <[email protected]>
> Reviewed-by: Alain Michaud <[email protected]>
> ---
>
> Changes in v4:
> - Re-read the MSFT data instead of skipping if it's initiated already
>
> Changes in v3:
> - Remove the accepted commits from the series
>
> net/bluetooth/hci_core.c | 4 ++--
> net/bluetooth/msft.c | 21 ++++++++++++++++++---
> 2 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index eeafed2efc0da..8056f0d4ae172 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1764,8 +1764,6 @@ int hci_dev_do_close(struct hci_dev *hdev)
>
> hci_sock_dev_event(hdev, HCI_DEV_DOWN);
>
> - msft_do_close(hdev);
> -
> if (hdev->flush)
> hdev->flush(hdev);
>
> @@ -3844,6 +3842,8 @@ void hci_unregister_dev(struct hci_dev *hdev)
> unregister_pm_notifier(&hdev->suspend_notifier);
> cancel_work_sync(&hdev->suspend_prepare);
>
> + msft_do_close(hdev);
> +
> hci_dev_do_close(hdev);
>
> if (!test_bit(HCI_INIT, &hdev->flags) &&
> diff --git a/net/bluetooth/msft.c b/net/bluetooth/msft.c
> index 8579bfeb28364..4465d018280eb 100644
> --- a/net/bluetooth/msft.c
> +++ b/net/bluetooth/msft.c
> @@ -73,16 +73,31 @@ static bool read_supported_features(struct hci_dev *hdev,
>
> void msft_do_open(struct hci_dev *hdev)
> {
> - struct msft_data *msft;
> + struct msft_data *msft = NULL;
>
> if (hdev->msft_opcode == HCI_OP_NOP)
> return;
>
> bt_dev_dbg(hdev, "Initialize MSFT extension");
>
> - msft = kzalloc(sizeof(*msft), GFP_KERNEL);
> - if (!msft)
> + /* If MSFT data exists, reset its members */
> + if (hdev->msft_data) {
> + msft = hdev->msft_data;
> + hdev->msft_data = NULL;
> +
> + msft->features = 0;
> + kfree(msft->evt_prefix);
> + msft->evt_prefix = NULL;
> + msft->evt_prefix_len = 0;
> +
> + } else {
> + msft = kzalloc(sizeof(*msft), GFP_KERNEL);
> + }
> +
> + if (!msft) {
> + bt_dev_err(hdev, "Failed to init MSFT extension");
> return;
> + }
>
> if (!read_supported_features(hdev, msft)) {
> kfree(msft);
> --
> 2.30.0.280.ga3ce27912f-goog
>