On 2018-07-12 16:13, Toke Høiland-Jørgensen wrote:
Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxx> writes:
On 2018-07-11 13:48, Toke Høiland-Jørgensen wrote:
Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxx> writes:
On 2018-07-09 09:37, Toke Høiland-Jørgensen wrote:
[...]
+/**
[...]
Erm, how would this prevent an infinite loop? With this scheme, at
some
point, ieee80211_next_txq() removes the last txq from activeq, and
returns that. Then, when it is called again the next time the driver
loops, it's back to the first case (activeq empty, waitq non-empty);
so
it'll move waitq back as activeq and start over... Only the driver
really knows when it is starting a logical "loop through all active
TXQs".
Oops.. My bad.. The idea is that ieee80211_next_txq process txq from
activeq only and keep processed txqs separately. Having single list
eventually needs tracking mechanism. The point is that once activeq
becomes empty, splice waitq list and return NULL. So that driver can
break from the loop.
ieee80211_next_txq
- if activeq empty,
- move waitq list into activeq
- return NULL
- if activeq not empty
- fetch appropriate txq from activeq
- remove txq from activeq list.
- If txq found, return txq else return NULL
Right, so this would ensure the driver only sees each TXQ once *if it
keeps looping*. But it doesn't necessarily; if the hardware queues fill
up, for instance, it might abort earlier. In that case it would need to
signal mac80211 that it is done for now, which is equivalent to
signalling when it starts a scheduler round.
Hmm... I thought driver will call ieee80211_schedule_txq when it runs
out
of hardware descriptor and break the loop. The serving txq will be added
back to head of activeq list. no?
Also, for airtime fairness, the queues are not scheduled strictly
round-robin, so the dual-list scheme wouldn't work there either...
As you know, ath10k driver will operate in two tx modes (push-only,
push-pull). These modes will be switched dynamically depends on
firmware load or resource availability. In push-pull mode, firmware
will query N number of frames for set of STA, TID.
Ah, so it will look up the TXQ without looping through the list of
pending queues at all? Didn't realise that is what it's doing; yeah,
that would be problematic for airtime fairness :)
So the driver will directly dequeue N number of frames from given txq.
In this case txq ordering alone wont help. I am planning to add below
changes in exiting API and add new API ieee80211_reorder_txq.
ieee80211_txq_get_depth
- return deficit status along with frm_cnt
ieee80211_reorder_txq
- if txq deficit > 0
- return;
- if txq is last
- return
- delete txq from list
- move it to tail
- update deficit by quantum
ath10k_htt_rx_tx_fetch_ind
- get txq deficit status
- if txq deficit > 0
- dequeue skb
- else if deficit < 0
- return NULL
Please share your thoughts.
Hmm, not sure exactly how this would work; seems a little complicated?
Also, I'd rather if drivers were completely oblivious to the deficit;
that is a bit of an implementation detail...
Agree.. Initially I thought of adding deficit check in
ieee80211_tx_dequeue.
But It will be overhead of taking activeq_lock for every skbs. Perhaps
it can be renamed as allowed_to_dequeue instead of deficit.
We could have an ieee80211_txq_may_pull(); or maybe just have
ieee80211_tx_dequeue() return NULL if the deficit is negative?
As I said earlier, checking deficit for every skb will be an overhead.
It should be done once before accessing txq.
the reasonable thing for the driver to do, then, would be to ask
ieee80211_next_txq() for another TXQ to pull from if the current one
doesn't work for whatever reason.
Would that work for push-pull mode?
Not really. Driver shouldn't send other txq data instead of asked one.
In MU-MIMO, firmware will query N packets from given set of {STA,TID}.
So the driver not supposed to send other txq's data.
-Rajkumar
