From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergey Ryazanov Date: Mon, 1 Dec 2014 01:11:56 +0300 Subject: [ath9k-devel] queue priority mapping In-Reply-To: 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 2014-12-01 0:57 GMT+03:00 Thomas H?hn : > Hi Sergey, > Hi Thomas, > You are right, the wrong mapping was caused by the call order. > Felix has already sent 3 patches to fix that bug. So lets test. > Do you know any issues caused by this queues misuse? I actually do not know what to test. > Greetings from Berlin > Thomas > > > > >> Am 30.11.2014 um 19:18 schrieb Sergey Ryazanov : >> >> 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 >> _______________________________________________ >> ath9k-devel mailing list >> ath9k-devel at lists.ath9k.org >> https://lists.ath9k.org/mailman/listinfo/ath9k-devel > -- BR, Sergey ? ?????????? ??????????? ??????? ??????