[PATCH 1/3] ath10k: Ensure there are no stale ar->txqs entries.

[Michal Kazior]

On 2 December 2016 at 01:24, Ben Greear <greearb at candelatech.com> wrote:
> On 12/01/2016 02:52 PM, Ben Greear wrote:
>>
>> On 08/19/2016 06:34 AM, Ben Greear wrote:
>>>
>>>
>>>
>>> On 08/18/2016 11:59 PM, Michal Kazior wrote:
>>>>
>>>> On 19 August 2016 at 03:26,  <greearb at candelatech.com> wrote:
>>>>>
>>>>> From: Ben Greear <greearb at candelatech.com>
>>>>>
>>>>> I was seeing kernel crashes due to accessing freed memory
>>>>> while debugging a 9984 firmware that was crashing often.
>>>>>
>>>>> This patch fixes the crashes.  I am not certain if there
>>>>> is a better way or not.
>
>
> Michal, thanks for the help on IRC.  I added this logic:
>
> static void ieee80211_drv_tx(struct ieee80211_local *local,
>                              struct ieee80211_vif *vif,
>                              struct ieee80211_sta *pubsta,
>                              struct sk_buff *skb)
> {
>         struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
>         struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
>         struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
>         struct ieee80211_tx_control control = {
>                 .sta = pubsta,
>         };
>         struct ieee80211_txq *txq = NULL;
>         struct txq_info *txqi;
>         u8 ac;
>
>         if ((info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ||
>             (info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
>                 goto tx_normal;
>
>         if (!ieee80211_is_data(hdr->frame_control))
>                 goto tx_normal;
>
>         if (unlikely(!ieee80211_sdata_running(sdata))) {
>                 WARN_ON_ONCE(1);
>                 goto delete_and_return;
>         }
>
> ...
>
>         if (atomic_read(&sdata->txqs_len[ac]) >=
>             (local->hw.txq_ac_max_pending * 2)) {
>                 /* Must be that something is not paying attention to
>                  * max-pending, like pktgen, so just drop this frame.
>                  */
> delete_and_return:
>                 ieee80211_free_txskb(&local->hw, skb);
>                 return;
>         }
>
>
> But, I still see the txq entries on the ar->txqs list in the
> ath10k_mac_txq_init
> after firmware crash in my test case.  Is this the test you were suggesting?

Yes.

Now that I think about it mac80211 doesn't call anything in driver
during hw_restart that would unref txqs. This means you'll have them
still linked when add_interface/sta_state is called, no?

This means that either:
 (a) txq (re-)init should be smarter in ath10k
 (b) txqs should be purged during hw_restart in ath10k


Michał