Hi,
On Mon, Mar 19, 2012 at 9:39 AM, Johannes Berg
<johannes@xxxxxxxxxxxxxxxx> 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.
Helmut
---
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 44627c8..25c1621 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -53,6 +53,7 @@ struct ieee80211_local;
#define IEEE80211_FRAGMENT_MAX 4
#define TU_TO_EXP_TIME(x) (jiffies + usecs_to_jiffies((x) * 1024))
+#define TU_TO_EXP_TIME_ROUNDED(x) round_jiffies(TU_TO_EXP_TIME(x))
#define IEEE80211_DEFAULT_UAPSD_QUEUES \
(IEEE80211_WMM_IE_STA_QOSINFO_AC_BK | \
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 491c96f..f1b111d 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -794,7 +794,7 @@ static void ieee80211_rx_reorder_ampdu(struct
ieee80211_rx_data *rx)
/* reset session timer */
if (tid_agg_rx->timeout)
mod_timer(&tid_agg_rx->session_timer,
- TU_TO_EXP_TIME(tid_agg_rx->timeout));
+ TU_TO_EXP_TIME_ROUNDED(tid_agg_rx->timeout));
/* if this mpdu is fragmented - terminate rx aggregation session */
sc = le16_to_cpu(hdr->seq_ctrl);
--
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
