On 2 December 2016 at 01:24, Ben Greear <greearb@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> wrote:
>>>>>
>>>>> From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>>>>
>>>>> 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ł
