Kan Yan <kyan@xxxxxxxxxx> writes: > Hi Toke, > > There is an updated version of AQL in the chromiumos tree implemented > in the mac80211 driver, instead of in the ath10k driver as the > original version: > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1703105/7 > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1703106/6 Ah, that's awesome! Thank you for brining this up :) > It is based on a more recent kernel (4.14) and integrated with the > airtime fairness tx scheduler in mac80211. This version has been > tested rather extensively. I intended to use it as the basis for my > effort to bring AQL upstream, but get sidetracked by other things. I > can clean it up and send a patchset next week if you think that is the > right path. Yes, please do! AFAICT, the main difference is that your version keeps the airtime calculation itself in the driver, while mine passes up the rate and lets mac80211 do the calculation of airtime. Other than that, the differences are minor, no? I'm not actually sure which approach is best; I suspect doing all the accounting in mac80211 will help with integrating this into drivers that use minstrel; we can just add a hook in that and be done with it. Whereas if the driver has to do the accounting, we would need to add that to each driver (mt76, iwl(?)). But of course, doing things in mac80211 depends on stuffing even more stuff into the already overloaded cb field; and I'm not actually entirely sure what I've done with that will actually work. WDYT? In any case, if you post your series we'll have something to contrast against, which I think will be useful to help us converge on something we can all be happy with. Of course we'll also have to eventually integrate this with the other series that Yibo recently re-posted (the virtual time scheduler). I think that will be relatively straight forward, except I'm not sure your atomic patches will work when we also have to update the rbtree. Any thoughts on that series in general? > Sorry for the long delay and slack off on the upstream effort. Hehe, no worries. I only posted this because Dave finally bugged me into doing something about this at LPC. And hey, we're making progress now, so that's good! :) > There is some concern in this thread regarding the accuracy of the > estimated airtime using the last reported TX rate. It is indeed a > rather crude method and did not include retries in the calculation. > Besides, there are lags between firmware changing rate and host driver > get the rate update. The 16us IFS overhead is only correct for 5G and > it is actually 10us for 2.4 G. However, that hardly matters. The goal > of AQL is to prevent the firmware/hardware queue from getting bloated > or starved. There is a lot of headroom in the queue length limit (8-10 > ms) to tolerate inaccuracy in the estimate airtime. AQL doesn't > control the fine grained TX packet scheduling. It is handled by the > airtime fairness scheduler and ultimately firmware. Yeah, this was basically the point I was trying to make; this limit doesn't need to be that accurate, we just need a rough estimate. If we want to get the latency even lower later, we're better off fiddling with the queue limit value than trying to improve the airtime estimate. > There are two TX airtimes in the newer version (chromiumos 4.14 > kernel): The estimated airtime for frames pending in the queue and the > airtime reported by the firmware for the frame transmitted, which > should be accurate as the firmware supposed to take retries and > aggregation into account. The airtime fairness scheduler that does the > TX packet scheduling should use the TX airtime reported by the > firmware. That's the reason why the original implementation in the > ChromiumOS tree tries to factor in aggregation size when estimate the > airtime overhead and the later version doesn't even bother with that. Yup, makes sense. Looking at the version you linked to, though, it seems you're calling ieee80211_sta_register_airtime() with the estimated value as well? So are you double-accounting airtime, or are you adjusting for the accurate values somewhere else I don't see in that series? -Toke
