Bugs in wake-queue logic.

[Michal Kazior]

On 05/12/2015, Ben Greear <greearb at candelatech.com> wrote:
> On 12/05/2015 05:42 AM, Michal Kazior wrote:
>> On 04/12/2015, Ben Greear <greearb at candelatech.com> wrote:
>>> So, after tweaking a firmware image to actually be able to use
>>> all tx-buffers, then queues can actually be stopped on the host
>>> now.
>>>
>>> I'm now getting splats related to tx-queue being
>>> out of range.
>>>
>>> Why are we using vdev_id as the queue-id below?
>>>
>>> void ath10k_mac_vif_tx_unlock(struct ath10k_vif *arvif, int reason)
>>> {
>>>           struct ath10k *ar = arvif->ar;
>>>
>>>           lockdep_assert_held(&ar->htt.tx_lock);
>>>
>>>           WARN_ON(reason >= BITS_PER_LONG);
>>>           arvif->tx_paused &= ~BIT(reason);
>>>
>>>           if (ar->tx_paused)
>>>                   return;
>>>
>>>           if (arvif->tx_paused)
>>>                   return;
>>>
>>>           ieee80211_wake_queue(ar->hw, arvif->vdev_id);
>>
>> I guess you could try replacing arvif->vdev_id with
>> arvif->vif->cab_queue. This along with vif->hw_queue[] share the same
>> queue number associated with vdev_id. Refer to the add_interface()
>> implementation, please.
>>
>> You don't need to increase mac80211's max_queues unless you intend to
>> support a similar host queue control via firmware events like qca6174.
>
> What about the comments about tx-locking?  Does that only apply to qca6174?

The code applies the logic to all chips but only qca6174 generates the
events that can stop queues per vif. The qca6174 doesn't have any
iface combinations with 16 vdevs so the logic works fine in all cases
as far as upstream is concerned.


> For host queue control, are firmware events really needed, or can we
> just keep track of buffered pkts per peer and/or per vdev and use
> that on the host?

It's possible. However you probably have your 16+ vdev setup in mind.
This in itself won't work as-is because of the modulo/overlap.

You'll probably need to add some kind of mechanism to avoid waking up
a queue when multiple vdevs share one.


Michal