Re: [PATCH 3/3] mac80211: optimize aggregation session timeout handling

[Felix Fietkau <nbd@openwrt.org>]

On 2012-03-19 11:50 AM, Helmut Schaa wrote:
> On Mon, Mar 19, 2012 at 11:36 AM, Felix Fietkau <nbd@openwrt.org> wrote:
>> On 2012-03-19 10:29 AM, Helmut Schaa wrote:
>>> Hi,
>>>
>>> On Mon, Mar 19, 2012 at 9:39 AM, Johannes Berg
>>> <johannes@sipsolutions.net> wrote:
>>>> On Sun, 2012-03-18 at 12:13 +0100, Felix Fietkau wrote:
>>>>> On 2012-03-18 11:17 AM, Johannes Berg wrote:
>>>>> > On Sun, 2012-03-18 at 00:00 +0100, Felix Fietkau wrote:
>>>>> >> Calling mod_timer from the rx/tx hotpath is somewhat expensive, and the
>>>>> >> timeout doesn't need to be so precise.
>>>>> >>
>>>>> >> Switch to a different strategy: Schedule the timer initially, store jiffies
>>>>> >> of all last rx/tx activity which would previously modify the timer, and
>>>>> >> let the timer re-arm itself after checking the last rx/tx timestamp.
>>>>> >
>>>>> > I don't like this. It's not the optimisation you think it is on other
>>>>> > ("embedded") systems where firing a timer is more expensive.
>>>>> >
>>>>> > You're trading power consumption against CPU utilisation by causing the
>>>>> > timer to wake up.
>>>>> I considered that was well, but didn't think one wakeup every 5 seconds
>>>>> or so would be significant. Would you take the patch if I change the
>>>>> timer to be deferrable, so that it doesn't cause wakeups by itself?
>>>>
>>>> I'm not really convinced, for making them deferrable we should analyse
>>>> the consequences of that more carefully, for example it seems possible
>>>> that the system wakes up to send a packet, and then the first thing that
>>>> happens is a few aggregation handshakes ... that wastes a lot of time
>>>> and power.
>>>
>>> I like the idea of getting rid of the mod_timer overhead. Looking at the timer
>>> code, if the timer value is unchanged mod_timer is not that expensive.
>>>
>>> So, instead of calling mod_timer for every successive frame with a slightly
>>> different timeout we could just use round_jiffies to round the timeout to the
>>> next full second. This would in most cases take the quick path through
>>> mod_timer and only update the timer once every second.
>>>
>>> See code (untested, not even compile tested) below.
>> I would still like to avoid the overhead of apply_slack(), which is
>> called early by mod_timer(). It was visible in both CPU cycles and
>> icache misses when I did some profiling under high tx load.
> 
> Indeed, however, I don't know the timer code at all. Seems like the default
> slack for a timer is 0.4%. Setting the slack to 0 with set_timer_slack
> should allow a shorter path through apply_slack. Not sure if that's sufficient
> already.
Looking at the code, it appears that this would not be sufficient.

- Felix