Re: [PATCH] ath10k_htt_rx_amsdu_allowed(): use ath10k_dbg()

[Ryan Hsu]

On 07/19/2017 11:24 AM, Gabriel Craciunescu wrote:

To make it consistent, maybe rename the patch title starting with ath10k:

> From: Gabriel Craciunescu <nix.or@gmail.com>
>
>   Each time we get disconencted from AP we get flooded with messages like:
>
>   ...
>   ath10k_pci :03:00.0: no channel configured; ignoring frame(s)!
>   
>   ath10k_warn: 155 callbacks suppressed
>   ...
>
>   Use ath10k_dbg() here too.

You don't need an indentation on every line of the commit.

> Signed-off-by: Gabriel Craciunescu <nix.or@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
> b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 398dda978d6e..75d9b59b7e63 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1514,7 +1514,7 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k 
> *ar,
>*/
>  
>   if (!rx_status->freq) {
> - ath10k_warn(ar, "no channel configured; ignoring frame(s)!\n");
> + ath10k_dbg(ar, ATH10K_DBG_HTT, "no channel configured; ignoring 
> frame(s)!\n");
>   return false;
>   }
>  

-- 
Ryan Hsu

Re: ath10k_htt_rx_amsdu_allowed() noise

[Ryan Hsu]

On 07/16/2017 03:56 PM, Gabriel C wrote:

>
> Can ath10k_warn() be ath10k_dbg() there ? Maybe this ?
>

Looks good to me.

>
> From d4138d936635ca7b69ed7f7b0cda4914f0f07917 Mon Sep 17 00:00:00 2001
> From: Gabriel Craciunescu <nix.or@gmail.com>
> Date: Mon, 17 Jul 2017 00:45:29 +0200
> Subject: [PATCH] ath10k_htt_rx_amsdu_allowed(): change ath10k_warn() to 
> th10k_dbg()
>
> Signed-off-by: Gabriel Craciunescu <nix.or@gmail.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
> b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index 398dda978d6e..ad0306cd6ee1 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -1514,7 +1514,7 @@ static bool ath10k_htt_rx_amsdu_allowed(struct ath10k 
> *ar,
>  */
>
> if (!rx_status->freq) {
> -   ath10k_warn(ar, "no channel configured; ignoriframe(s)!\n");
> +   ath10k_dbg(ar, ATH10K_DBG_HTT, "no channel configured, 
> ignoring frame(s)!\n");
> return false;
> }
>

Can you send this as patch for reviewing, please?

-- 
Ryan Hsu

Re: WARN_ON_ONCE(work > weight) in napi_poll()

[Ryan Hsu]

On 07/11/2017 06:19 PM, Igor Mitsyanko wrote:

> On 07/11/2017 10:28 AM, Andrey Ryabinin wrote:
>>
>> It gave me this:
>>
>> [118648.825347] #1 quota too big 72 64 16
>> [118648.825351] #2 quota too big 72 64 16
>> [118648.825471] [ cut here ]
>> [118648.825484] WARNING: CPU: 0 PID: 0 at ../net/core/dev.c:5274 
>> net_rx_action+0x258/0x360
>>
>> So this means that we didn't met the condition bellow, i.e. 
>> skb_queue_empty() returned true.
>>
>>  ath10k_htt_txrx_compl_task():
>>
>>  if ((quota > ATH10K_NAPI_QUOTA_LIMIT) &&
>>  !skb_queue_empty(>rx_in_ord_compl_q)) {
>>  resched_napi = true;
>>  goto exit;
>>  }
>>
>>> Also WLAN.RM.2.0-00180-QCARMSWPZ-1 firmware is a bit old, could you also 
>>> update firmware to give it a try?
>>> https://github.com/kvalo/ath10k-firmware/tree/master/QCA6174/hw3.0/4.4
>>>
>>
>> Will try.
>>
>
> Maybe ath10k_htt_rx_in_ord_ind() has to accept "budget_left" parameter and 
> use it to limit number of processed MSDUs in queued AMSDU and saving rest for 
> later (NAPI has to be rescheduled in this case).
> It seems natural that this problem happens with current logic, in case AMSDU 
> in Rx queue has more elements then left in budget.

Thanks, likely in current logic, it does have chance to exceed the budget while 
dequeuing from the last list.

Can you give it a try this one? for QCA6174 reorder is offload, so this should 
be good enough for your case to test, will have to check non-offload reorder 
case... but let me know if you're seeing something different

--
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 398dda9..e8697a1 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -1735,7 +1735,8 @@ static void ath10k_htt_rx_delba(struct ath10k *ar, struct 
htt_resp *resp)
 }
 
 static int ath10k_htt_rx_extract_amsdu(struct sk_buff_head *list,
-   struct sk_buff_head *amsdu)
+   struct sk_buff_head *amsdu,
+   int budget_left)
 {
 struct sk_buff *msdu;
 struct htt_rx_desc *rxd;
@@ -1746,8 +1747,9 @@ static int ath10k_htt_rx_extract_amsdu(struct 
sk_buff_head *list,
 if (WARN_ON(!skb_queue_empty(amsdu)))
 return -EINVAL;
 
-while ((msdu = __skb_dequeue(list))) {
+while ((msdu = __skb_dequeue(list)) && budget_left) {
 __skb_queue_tail(amsdu, msdu);
+budget_left--;
 
 rxd = (void *)msdu->data - sizeof(*rxd);
 if (rxd->msdu_end.common.info0 &
@@ -1838,7 +1840,8 @@ static int ath10k_htt_rx_h_rx_offload(struct ath10k *ar,
 return num_msdu;
 }
 
-static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb)
+static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, struct sk_buff *skb,
+int budget_left)
 {
 struct ath10k_htt *htt = >htt;
 struct htt_resp *resp = (void *)skb->data;
@@ -1895,9 +1898,9 @@ static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, 
struct sk_buff *skb)
 if (offload)
 num_msdus = ath10k_htt_rx_h_rx_offload(ar, );
 
-while (!skb_queue_empty()) {
+while (!skb_queue_empty() && budget_left) {
 __skb_queue_head_init();
-ret = ath10k_htt_rx_extract_amsdu(, );
+ret = ath10k_htt_rx_extract_amsdu(, , budget_left);
 switch (ret) {
 case 0:
 /* Note: The in-order indication may report interleaved
@@ -1907,6 +1910,7 @@ static int ath10k_htt_rx_in_ord_ind(struct ath10k *ar, 
struct sk_buff *skb)
  * should still give an idea about rx rate to the user.
  */
 num_msdus += skb_queue_len();
+budget_left -= skb_queue_len();
 ath10k_htt_rx_h_ppdu(ar, , status, vdev_id);
 ath10k_htt_rx_h_filter(ar, , status);
 ath10k_htt_rx_h_mpdu(ar, , status);
@@ -2549,7 +2553,8 @@ int ath10k_htt_txrx_compl_task(struct ath10k *ar, int 
budget)
 }
 
 spin_lock_bh(>rx_ring.lock);
-num_rx_msdus = ath10k_htt_rx_in_ord_ind(ar, skb);
+num_rx_msdus = ath10k_htt_rx_in_ord_ind(ar, skb,
+(budget - quota));
 spin_unlock_bh(>rx_ring.lock);
 if (num_rx_msdus < 0) {
 resched_napi = true;
--

-- 
Ryan Hsu

Re: WARN_ON_ONCE(work > weight) in napi_poll()

[Ryan Hsu]

On 07/04/2017 08:59 AM, Andrey Ryabinin wrote:

> On 07/04/2017 04:49 PM, Kalle Valo wrote:
>> Andrey Ryabinin <aryabi...@virtuozzo.com> writes:
>>
>>> I occasionally hit WARN_ON_ONCE(work > weight); in napi_poll() on a
>>> laptop with ath10k card.
>>>
>>>
>>> [37207.593370] [ cut here ]
>>> [37207.593380] WARNING: CPU: 0 PID: 7 at ../net/core/dev.c:5274
>>> net_rx_action+0x258/0x360
>>> [37207.593381] Modules linked in: snd_hda_codec_realtek snd_soc_skl
>>> snd_hda_codec_generic snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp
>>> snd_hda_ext_core snd_soc_sst_match snd_soc_core rtsx_pci_sdmmc
>>> mmc_core snd_pcm_dmaengine x86_pkg_temp_thermal snd_hda_intel uvcvideo
>>> kvm_intel videobuf2_vmalloc kvm snd_hda_codec snd_hwdep btusb
>>> videobuf2_memops btintel snd_hda_core videobuf2_v4l2 bluetooth
>>> irqbypass snd_pcm videobuf2_core crc32c_intel videodev mei_me mei
>>> rtsx_pci intel_lpss_pci intel_lpss_acpi intel_vbtn intel_lpss mfd_core
>>> tpm_tis intel_hid tpm_tis_core tpm efivarfs
>>> [37207.593430] CPU: 0 PID: 7 Comm: ksoftirqd/0 Not tainted 4.11.7 #28
>>> [37207.593432] Hardware name: Dell Inc. XPS 13 9360/0839Y6, BIOS 1.3.5 
>>> 05/08/2017
>> What firmware and hardware versions exactly? The dmesg output when
>> ath10k loads is preferred. As you are using XPS 13 I'm guessing it's one
>> of QCA6174 family.
>>
> Yes it's qca6174.
>
> $ dmesg |grep ath10
> [0.624828] ath10k_pci :3a:00.0: enabling device ( -> 0002)
> [0.626370] ath10k_pci :3a:00.0: pci irq msi oper_irq_mode 2 irq_mode 
> 0 reset_mode 0
> [0.837862] ath10k_pci :3a:00.0: qca6174 hw3.2 target 0x0503 
> chip_id 0x00340aff sub 1a56:1535
> [0.837863] ath10k_pci :3a:00.0: kconfig debug 0 debugfs 1 tracing 0 
> dfs 0 testmode 0
> [0.838388] ath10k_pci :3a:00.0: firmware ver 
> WLAN.RM.2.0-00180-QCARMSWPZ-1 api 4 features wowlan,ignore-otp,no-4addr-pad 
> crc32 75dee6c5
> [0.900606] ath10k_pci :3a:00.0: board_file api 2 bmi_id N/A crc32 
> 19644295
> [3.020681] ath10k_pci :3a:00.0: htt-ver 3.26 wmi-op 4 htt-op 3 cal 
> otp max-sta 32 raw 0 hwcrypto 1
> [9.574087] ath10k_pci :3a:00.0 wlp58s0: renamed from wlan0
>

I can't reproduce of this past few days, not sure if this is due to the amsdu 
packets received from AP, would you mind share what Ap you're using?
And if there any specific steps you're doing?

Also WLAN.RM.2.0-00180-QCARMSWPZ-1 firmware is a bit old, could you also update 
firmware to give it a try?
https://github.com/kvalo/ath10k-firmware/tree/master/QCA6174/hw3.0/4.4

-- 
Ryan Hsu