[PATCH v3] ath10k: move mgmt descriptor limit handle under mgmt_tx

[Michal Kazior]

On 8 March 2016 at 13:54, Rajkumar Manoharan <rmanohar at codeaurora.org> wrote:
> On , Michal Kazior wrote:
>>
>> On 8 March 2016 at 12:54, Rajkumar Manoharan <rmanohar at qti.qualcomm.com>
>> wrote:
>> [...]
>>>
>>> +int ath10k_htt_tx_mgmt_inc_pending(struct ath10k_htt *htt, bool is_mgmt,
>>> +                                  bool is_presp)
>>>  {
>>>         struct ath10k *ar = htt->ar;
>>>
>>>         lockdep_assert_held(&htt->tx_lock);
>>>
>>> +       if (!is_mgmt)
>>> +               return 0;
>>> +
>>
>>
>>>         if (htt->num_pending_tx >= htt->max_num_pending_tx)
>>>                 return -EBUSY;
>>
>>
>> This busy here will be hit when you use last tx slot because
>> tx_inc_pending() is called before tx_mgmt_inc_pending().
>>
> Yeah.. now this condition is not needed as ath10k_htt_tx_mgmt_inc_pending
> will be called only when tx_inc_pending succeeds. will remove this.
>
>>
>> [...]
>>>
>>>         skb_cb = ATH10K_SKB_CB(msdu);
>>>         txq = skb_cb->txq;
>>>         artxq = (void *)txq->drv_priv;
>>>
>>> -       if (unlikely(skb_cb->flags & ATH10K_SKB_F_MGMT) &&
>>> -           ar->hw_params.max_probe_resp_desc_thres)
>>> -               limit_mgmt_desc = true;
>>> -
>>
>>
>> Oh wait.. I didn't pay attention before, but this looks rather ugly.
>> With this you will actually make the num_pending_mgmt_tx spin
>> endlessly upwards on firmware with HTT 3.0 (and uses TX_FRM for mgmt
>> tx and TX_COMPL_IND for completions), no?
>>
> Oops.. num_pending_mgmt_tx should be incremented only for MGMT_TX. Thats
> why it is moved out of txrx_tx_unref.

Hmm.. in that case I think you should change is_htt in mac_op_tx() to
compare only against TX_HTT_MGMT (instead of both TX_HTT and
TX_HTT_MGMT).

Note, the is_mgmt should stay as it is possible for the TX_HTT_MGMT to
be used for non-mgmt frames in some cases. This also implies you need
to check for ATH10K_SKB_F_MGMT on the mgmt-tx-completion event as well
(you might get to handle NullFunc there).


Michał