Hi Dmitry,
A couple of other quick notes follow. I'll make and send on those
other patches sometime this week as I get time.
On Fri, Mar 02, 2012 at 02:41:23AM +0100, Dmitry Tarnyagin wrote:
> + /* minstrel is buggy a little bit, so distille
> + * incoming rates first. */
Can you elaborate a bit on the bugs? It may be worth fixing them
in minstrel so that everyone benefits. From your code it looks
like you want to reorder them from fastest->slowest, eliminate
duplicate rates, and change retry counts to respect total retry
limits. I agree that the last two are problematic and should
probably be fixed in minstrel.
Changing the rate order may have adverse affects (I'm not sure).
Minstrel often puts 'probe' rates in early MRR slots to update
its statistics, these may well be lower than the current rate.
> + /* Sort rates in descending order. */
> + for (i = 1; i < count; ++i) {
> + if (rates[i].idx < 0) {
> + count = i;
> + break;
> + }
> + if (rates[i].idx > rates[i - 1].idx) {
> + struct ieee80211_tx_rate tmp = rates[i - 1];
> + rates[i - 1] = rates[i];
> + rates[i] = tmp;
> + }
> + }
This sort seems to be missing an inner loop. E.g. if input was 2-3-5-4,
I think you would end up with 2-5-4-3 instead of 5-4-3-2.
> + /* ">> 1" is an equivalent of "/ 2", but faster */
> + int mid_rate = (rates[0].idx + 4) >> 1;
(This is true, but it's only by a couple of instructions for signed
division since compilers still change divide-by-power-of-two to shifts.
Unsigned /2 typically has the same code as >> 1. I tend to prefer /2
for readability unless it's a super hot path, but that's your call.)
--
Bob Copeland %% www.bobcopeland.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
