From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergey Ryazanov Date: Sun, 30 Nov 2014 22:18:24 +0400 Subject: [ath9k-devel] queue priority mapping In-Reply-To: <91ADAADD-6367-4C24-A965-9248A5D18D40@net.t-labs.tu-berlin.de> References: <5472F604.3020404@fami-braun.de> <21621.50854.350744.269360@gargle.gargle.HOWL> <91ADAADD-6367-4C24-A965-9248A5D18D40@net.t-labs.tu-berlin.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: ath9k-devel@lists.ath9k.org Hi Thomas, 2014-11-30 14:30 GMT+03:00 Thomas H?hn : > From my point of view the call-path of the functions look like this: > > ath9k_init_queues(struct ath_softc *sc) > > -> ath_txq_setup(sc, ATH9K_TX_QUEUE_DATA, i) with subtype-remapping > via > > qi.tqi_subtype = subtype_txq_to_hwq[subtype]; > > -> ath9k_hw_setuptxqueue(ah, qtype, &qi); > > And the subtyperemapping reflects the correct AC class mapping to ath9k hw > queues. tqi_subtype field doesn't define hw queue. Quick grep show only 6 lines in the whole driver, where this field is accessed. Actual hw queue number stored in axq_qnum field of ath_txq as documented in ath9k.h > In xmit.c function ath_txq_setup() defines and initializes the mapping with: > > static const int subtype_txq_to_hwq[] = { > > [IEEE80211_AC_BE] = ATH_TXQ_AC_BE, > [IEEE80211_AC_BK] = ATH_TXQ_AC_BK, > [IEEE80211_AC_VI] = ATH_TXQ_AC_VI, > [IEEE80211_AC_VO] = ATH_TXQ_AC_VO, > }; > > from mac80211.h: > > 101 /** > 102 * enum ieee80211_ac_numbers - AC numbers as used in mac80211 > 103 * @IEEE80211_AC_VO: voice > 104 * @IEEE80211_AC_VI: video > 105 * @IEEE80211_AC_BE: best effort > 106 * @IEEE80211_AC_BK: background > 107 */ > 108 enum ieee80211_ac_numbers { > 109 IEEE80211_AC_VO = 0, > 110 IEEE80211_AC_VI = 1, > 111 IEEE80211_AC_BE = 2, > 112 IEEE80211_AC_BK = 3, > 113 }; > 114 #define IEEE80211_NUM_ACS 4 > > from ath9k/hw.h: > > 218 enum ath_hw_txq_subtype { > 219 ATH_TXQ_AC_BE = 0, > 220 ATH_TXQ_AC_BK = 1, > 221 ATH_TXQ_AC_VI = 2, > 222 ATH_TXQ_AC_VO = 3, > 223 }; > > this translates to the following mapping of AC to hw queues: > > static const int subtype_txq_to_hwq[] = { > [2] = 0, > [3] = 1, > [1] = 2, > [0] = 3, > }; > This table confuse me too. But if you trace where the table and the tqi_subtype field is used, then you face, that this table means nothing for hw queue selection. > > That is how I understand the source code and yes it looks a bit complicated > but function wise seems to be correct. > Or do I miss something here ? > Actual hw queue selection occurs inside the ath9k_hw_setuptxqueue() function (in mac.c). Stripped version looks like: int ath9k_hw_setuptxqueue(struct ath_hw *ah, enum ath9k_tx_queue type, const struct ath9k_tx_queue_info *qinfo) { int q; [stripped] for (q = 0; q < ATH9K_NUM_TX_QUEUES; q++) if (ah->txq[q].tqi_type == ATH9K_TX_QUEUE_INACTIVE) break; [stripped] return q; } So, as Huber wrote, hw queue number really depends on call order. And existing call order gives us the following result: ~# cat /sys/kernel/debug/ieee80211/phy0/ath9k/queues (VO): qnum: 0 qdepth: 0 ampdu-depth: 0 pending: 0 stopped: 0 (VI): qnum: 1 qdepth: 0 ampdu-depth: 0 pending: 0 stopped: 0 (BE): qnum: 2 qdepth: 0 ampdu-depth: 0 pending: 0 stopped: 0 (BK): qnum: 3 qdepth: 0 ampdu-depth: 0 pending: 0 stopped: 0 (CAB): qnum: 8 qdepth: 0 ampdu-depth: 0 pending: 0 stopped: 0 Just in case, I checked ath5k driver and it contains exactly the same code. Let's wait a comment from maintainers. > Greetings from Berlin > Thomas > > > > > Am 27.11.2014 um 13:04 schrieb Hubert Feurstein : > > Hi Sujith, > > 2014-11-26 13:25 GMT+01:00 Sujith Manoharan : > > Hubert Feurstein wrote: > > Well, in fact it does. To understand my post you have to know the > behaviour of ath_txq_setup. The point here is that on the first call > of ath_txq_setup(..ATH9K_TX_QUEUE_DATA..), hw-queue 0 is returned. On > the second call hw-queue 1 is returned, ... . And IEEE80211_AC_VO = 0, > so hw-queue 0 is assigned, which is wrong in my opinion. But the right > place to fix this would be ath_txq_setup itself. > > > The mapping has been changed recently and I think it might be a problem, > since this is what the datasheet says: > > "The mapping of physical DCUs to absolute channel access priorities is fixed > and > cannot be altered by software." > > > Yes I know, and isn't this the reason why there is > sc->tx.txq_map[] in place, to map the AC to the proper HW queue in > software. So sc->tx.txq_map[IEEE80211_AC_VO] should return the > higher-prio-queue 3 and not the low-prio-queue 0. In FreeBSD it is > that way, in Linux it is the opposite, because in Linux > IEEE80211_AC_VO=0 and in FreeBSD WME_AC_VO=3. > > Hubert -- BR, Sergey