[PATCH] ath10k: enable threaded napi on ath10k driver

[PATCH] ath10k: enable threaded napi on ath10k driver

[Abhishek Kumar]

NAPI poll can be done in threaded context along with soft irq
context. Threaded context can be scheduled efficiently, thus
creating less of bottleneck during Rx processing. This patch is
to enable threaded NAPI on ath10k driver.

Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
Signed-off-by: Abhishek Kumar <kuabhs@chromium.org>
---

 drivers/net/wireless/ath/ath10k/pci.c  | 1 +
 drivers/net/wireless/ath/ath10k/sdio.c | 1 +
 drivers/net/wireless/ath/ath10k/snoc.c | 2 +-
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 4d4e2f91e15c..584307574d99 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1958,6 +1958,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
 
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
 
+	dev_set_threaded(&ar->napi_dev, true);
 	ath10k_core_napi_enable(ar);
 
 	ath10k_pci_irq_enable(ar);
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index 63e1c2d783c5..52ef74d9811a 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -1862,6 +1862,7 @@ static int ath10k_sdio_hif_start(struct ath10k *ar)
 	struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
 	int ret;
 
+	dev_set_threaded(&ar->napi_dev, true);
 	ath10k_core_napi_enable(ar);
 
 	/* Sleep 20 ms before HIF interrupts are disabled.
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 9513ab696fff..e7d12dbb3fa5 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -926,7 +926,7 @@ static int ath10k_snoc_hif_start(struct ath10k *ar)
 	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
 
 	bitmap_clear(ar_snoc->pending_ce_irqs, 0, CE_COUNT_MAX);
-
+	dev_set_threaded(&ar->napi_dev, true);
 	ath10k_core_napi_enable(ar);
 	ath10k_snoc_irq_enable(ar);
 	ath10k_snoc_rx_post(ar);
-- 
2.34.1.173.g76aa8bc2d0-goog

Re: [PATCH] ath10k: enable threaded napi on ath10k driver

[Abhishek Kumar]

Hi All,

This patch is to trigger a discussion on the best approach to enable
threaded NAPI on ath10k. Threaded NAPI feature was added in (net:
extract napi poll functionality to __napi_poll() commit
898f8015ffe74118e7b461827451f2cc6e51035b) and showed good results on
ath10k snoc based solution.

If we come to a consensus with this as the best approach to enable
threaded NAPI on ath10k, then we can moved ahead with the
implementation and enable across sdio and pci, or if there is any
objection then we can discuss it here.

Thanks
Abhishek

On Tue, Dec 14, 2021 at 2:41 PM Abhishek Kumar <kuabhs@chromium.org> wrote:
>
> NAPI poll can be done in threaded context along with soft irq
> context. Threaded context can be scheduled efficiently, thus
> creating less of bottleneck during Rx processing. This patch is
> to enable threaded NAPI on ath10k driver.
>
> Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
> Signed-off-by: Abhishek Kumar <kuabhs@chromium.org>
> ---
>
>  drivers/net/wireless/ath/ath10k/pci.c  | 1 +
>  drivers/net/wireless/ath/ath10k/sdio.c | 1 +
>  drivers/net/wireless/ath/ath10k/snoc.c | 2 +-
>  3 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
> index 4d4e2f91e15c..584307574d99 100644
> --- a/drivers/net/wireless/ath/ath10k/pci.c
> +++ b/drivers/net/wireless/ath/ath10k/pci.c
> @@ -1958,6 +1958,7 @@ static int ath10k_pci_hif_start(struct ath10k *ar)
>
>         ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");
>
> +       dev_set_threaded(&ar->napi_dev, true);
>         ath10k_core_napi_enable(ar);
>
>         ath10k_pci_irq_enable(ar);
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
> index 63e1c2d783c5..52ef74d9811a 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -1862,6 +1862,7 @@ static int ath10k_sdio_hif_start(struct ath10k *ar)
>         struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
>         int ret;
>
> +       dev_set_threaded(&ar->napi_dev, true);
>         ath10k_core_napi_enable(ar);
>
>         /* Sleep 20 ms before HIF interrupts are disabled.
> diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
> index 9513ab696fff..e7d12dbb3fa5 100644
> --- a/drivers/net/wireless/ath/ath10k/snoc.c
> +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> @@ -926,7 +926,7 @@ static int ath10k_snoc_hif_start(struct ath10k *ar)
>         struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
>
>         bitmap_clear(ar_snoc->pending_ce_irqs, 0, CE_COUNT_MAX);
> -
> +       dev_set_threaded(&ar->napi_dev, true);
>         ath10k_core_napi_enable(ar);
>         ath10k_snoc_irq_enable(ar);
>         ath10k_snoc_rx_post(ar);
> --
> 2.34.1.173.g76aa8bc2d0-goog
>

Re: [PATCH] ath10k: enable threaded napi on ath10k driver

[Kalle Valo]

Abhishek Kumar <kuabhs@chromium.org> writes:

> This patch is to trigger a discussion on the best approach to enable
> threaded NAPI on ath10k. Threaded NAPI feature was added in (net:
> extract napi poll functionality to __napi_poll() commit
> 898f8015ffe74118e7b461827451f2cc6e51035b) and showed good results on
> ath10k snoc based solution.
>
> If we come to a consensus with this as the best approach to enable
> threaded NAPI on ath10k, then we can moved ahead with the
> implementation and enable across sdio and pci, or if there is any
> objection then we can discuss it here.

Few tips:

It's a good idea to mark patches like this with "[PATCH RFC]":

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#use_rfc_or_rft_for_patches_not_ready

And you can add a text like the above after the "---" line in the patch
itself.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] ath10k: enable threaded napi on ath10k driver

[Jakub Kicinski]

On Tue, 14 Dec 2021 22:39:36 +0000 Abhishek Kumar wrote:
> NAPI poll can be done in threaded context along with soft irq
> context. Threaded context can be scheduled efficiently, thus
> creating less of bottleneck during Rx processing. This patch is
> to enable threaded NAPI on ath10k driver.

You need to explain in more detail what you mean by "can be scheduled
efficiently". mt76 had an issue where Rx and Tx would use the same IRQ 
and threaded NAPI allowed them to be run on separate cores. What's the
challenge for ath10k HW?

Re: [PATCH] ath10k: enable threaded napi on ath10k driver

[Kalle Valo]

Abhishek Kumar <kuabhs@chromium.org> writes:

> NAPI poll can be done in threaded context along with soft irq
> context. Threaded context can be scheduled efficiently, thus
> creating less of bottleneck during Rx processing. This patch is
> to enable threaded NAPI on ath10k driver.
>
> Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
> Signed-off-by: Abhishek Kumar <kuabhs@chromium.org>
> ---
>
>  drivers/net/wireless/ath/ath10k/pci.c  | 1 +
>  drivers/net/wireless/ath/ath10k/sdio.c | 1 +
>  drivers/net/wireless/ath/ath10k/snoc.c | 2 +-

I think also pci and sdio should be tested.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] ath10k: enable threaded napi on ath10k driver

[Felix Fietkau]

On 2021-12-14 23:39, Abhishek Kumar wrote:
> NAPI poll can be done in threaded context along with soft irq
> context. Threaded context can be scheduled efficiently, thus
> creating less of bottleneck during Rx processing. This patch is
> to enable threaded NAPI on ath10k driver.
> 
> Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
> Signed-off-by: Abhishek Kumar <kuabhs@chromium.org>
The name of the dummy device should be filled in as well. How about
something like this:

--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -3576,6 +3576,9 @@ struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
  		  ath10k_core_set_coverage_class_work);
  
  	init_dummy_netdev(&ar->napi_dev);
+	snprintf(ar->napi_dev.name, sizeof(ar->napi_dev.name), "%s",
+		 wiphy_name(ar->hw->wiphy));
+	ar->napi_dev.threaded = 1;
  
  	ret = ath10k_coredump_create(ar);
  	if (ret)

Re: [PATCH] ath10k: enable threaded napi on ath10k driver

[Dave Taht]

what is your definition "Good results"? I would really love it if I
could get back more flent benchmarks like this one:

https://forum.openwrt.org/t/aql-and-the-ath10k-is-lovely/59002

As the ath10k has cost me more hair and time than I care to think about.