Thread: [Madwifi-devel] simple patch to samplerate algo
Status: Beta
Brought to you by:
otaku
From: Dan H. <dha...@cs...> - 2009-08-20 12:32:00
|
The variable cw stores the current contention window range in a variable that should always be (2^k - 1). It starts as ATH_DEFAULT_CWMIN (http://madwifi-project.org/browser/madwifi/trunk/ath_rate/sample/sample.c#L173 ) which is 15. This invariant should be maintained when growing the contention window. However, the code that updates cw (http://madwifi-project.org/browser/madwifi/trunk/ath_rate/sample/sample.c#L257 ) is cw = MIN(ATH_DEFAULT_CWMAX, (cw + 1) * 2); which will proceed 15, 32, 66, 134, 270, ... instead of 15, 31, 63, 127, 255, ... . The correct code is cw = MIN(ATH_DEFAULT_CWMAX, (cw + 1) * 2 - 1); or perhaps more simply cw = MIN(ATH_DEFAULT_CWMAX, cw * 2 + 1); or more efficiently, assuming a dumb compiler, cw = MIN(ATH_DEFAULT_CWMAX, (cw << 1) | 1); . Thanks, Dan |
From: Pavel R. <pr...@gn...> - 2009-09-16 02:28:44
|
On Thu, 2009-08-20 at 14:15 +0200, Dan Halperin wrote: > The variable cw stores the current contention window range in a > variable that should always be (2^k - 1). It starts as > ATH_DEFAULT_CWMIN (http://madwifi-project.org/browser/madwifi/trunk/ath_rate/sample/sample.c#L173 > ) which is 15. > > This invariant should be maintained when growing the contention > window. However, the code that updates cw (http://madwifi-project.org/browser/madwifi/trunk/ath_rate/sample/sample.c#L257 > ) is > cw = MIN(ATH_DEFAULT_CWMAX, (cw + 1) * 2); > which will proceed 15, 32, 66, 134, 270, ... instead of 15, 31, 63, > 127, 255, ... . The correct code is > cw = MIN(ATH_DEFAULT_CWMAX, (cw + 1) * 2 - 1); > or perhaps more simply > cw = MIN(ATH_DEFAULT_CWMAX, cw * 2 + 1); > or more efficiently, assuming a dumb compiler, > cw = MIN(ATH_DEFAULT_CWMAX, (cw << 1) | 1); Sorry for delay. You are right. The later expression is what HAL uses in ath_hal_setTxQProps(). And the comment says that it should be power of two minus one. The same code is used in the Minstrel algorithm. Also, the code is present in the Minstrel implementation in the kernel. Since MadWifi doesn't have that kind of manpower and expertise as the kernel, I'm going to submit the patch to the kernel first. Once it's applied, I'll apply it to MadWifi. I used that approach with the warning fix in ath_info, which also applied to ath5k. Generally, if a fix applies to the kernel, please get it merged into the kernel first. I don't want to be in the position when MadWifi has a fix that free drivers don't have. Besides, the expertise of the kernel developers is too valuable to be ignored. -- Regards, Pavel Roskin |
From: Dan H. <dha...@cs...> - 2009-09-16 02:51:00
|
> Sorry for delay. You are right. The later expression is what HAL > uses > in ath_hal_setTxQProps(). And the comment says that it should be > power > of two minus one. Thanks for checking out the code. I certainly understand the delay! > The same code is used in the Minstrel algorithm. Also, the code is > present in the Minstrel implementation in the kernel. > > Since MadWifi doesn't have that kind of manpower and expertise as the > kernel, I'm going to submit the patch to the kernel first. Once it's > applied, I'll apply it to MadWifi. > > I used that approach with the warning fix in ath_info, which also > applied to ath5k. > > Generally, if a fix applies to the kernel, please get it merged into > the > kernel first. I don't want to be in the position when MadWifi has a > fix > that free drivers don't have. Besides, the expertise of the kernel > developers is too valuable to be ignored. Absolutely agree; I wasn't aware that this code was also present in the kernel's Minstrel algorithm, I was just looking at SampleRate. Thanks again, Dan |