linux-kernel - Re: [PATCH v3] ath10k: skip the wait for completion to recovery in shutdown path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 1 Mar 2021 17:51:05 -0800
From:   Abhishek Kumar <kuabhs@...omium.org>
To:     Youghandhar Chintala <youghand@...eaurora.org>
Cc:     ath10k <ath10k@...ts.infradead.org>,
        ath10k-review.external@....qualcomm.com,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Brian Norris <briannorris@...omium.org>
Subject: Re: [PATCH v3] ath10k: skip the wait for completion to recovery in
 shutdown path

This patch seems to address the comments on v2. Overall this patch LGTM.

Reviewed-by: Abhishek Kumar <kuabhs@...omium.org>

On Tue, Feb 23, 2021 at 6:29 AM Youghandhar Chintala
<youghand@...eaurora.org> wrote:
>
> Currently in the shutdown callback we wait for recovery to complete
> before freeing up the resources. This results in additional two seconds
> delay during the shutdown and thereby increase the shutdown time.
>
> As an attempt to take less time during shutdown, remove the wait for
> recovery completion in the shutdown callback and added an API to freeing
> the reosurces in which they were common for shutdown and removing
> the module.
>
> Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
>
> Signed-off-by: Youghandhar Chintala <youghand@...eaurora.org>
> Change-Id: I65bc27b5adae1fedc7f7b367ef13aafbd01f8c0c
> ---
> Changes from v2:
> -Corrected commit text and added common API for freeing the
>  resources for shutdown and unloading the module
> ---
>  drivers/net/wireless/ath/ath10k/snoc.c | 29 ++++++++++++++++++--------
>  1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
> index 84666f72bdfa..70b3f2bd1c81 100644
> --- a/drivers/net/wireless/ath/ath10k/snoc.c
> +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> @@ -1781,17 +1781,11 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
>         return ret;
>  }
>
> -static int ath10k_snoc_remove(struct platform_device *pdev)
> +static int ath10k_snoc_free_resources(struct ath10k *ar)
>  {
> -       struct ath10k *ar = platform_get_drvdata(pdev);
>         struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
>
> -       ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc remove\n");
> -
> -       reinit_completion(&ar->driver_recovery);
> -
> -       if (test_bit(ATH10K_SNOC_FLAG_RECOVERY, &ar_snoc->flags))
> -               wait_for_completion_timeout(&ar->driver_recovery, 3 * HZ);
> +       ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc free resources\n");
>
>         set_bit(ATH10K_SNOC_FLAG_UNREGISTERING, &ar_snoc->flags);
>
> @@ -1805,12 +1799,29 @@ static int ath10k_snoc_remove(struct platform_device *pdev)
>         return 0;
>  }
>
> +static int ath10k_snoc_remove(struct platform_device *pdev)
> +{
> +       struct ath10k *ar = platform_get_drvdata(pdev);
> +       struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
> +
> +       ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc remove\n");
> +
> +       reinit_completion(&ar->driver_recovery);
> +
> +       if (test_bit(ATH10K_SNOC_FLAG_RECOVERY, &ar_snoc->flags))
> +               wait_for_completion_timeout(&ar->driver_recovery, 3 * HZ);
> +
> +       ath10k_snoc_free_resources(ar);
> +
> +       return 0;
> +}
> +
>  static void ath10k_snoc_shutdown(struct platform_device *pdev)
>  {
>         struct ath10k *ar = platform_get_drvdata(pdev);
>
>         ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc shutdown\n");
> -       ath10k_snoc_remove(pdev);
> +       ath10k_snoc_free_resources(ar);
>  }
>
>  static struct platform_driver ath10k_snoc_driver = {
> --
> 2.29.0
>