Return-path: Received: from mail-pz0-f46.google.com ([209.85.210.46]:42921 "EHLO mail-pz0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754210Ab2CSJ3s (ORCPT ); Mon, 19 Mar 2012 05:29:48 -0400 Received: by dajr28 with SMTP id r28so9666915daj.19 for ; Mon, 19 Mar 2012 02:29:48 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1332146368.3359.12.camel@jlt3.sipsolutions.net> References: <1332025254-5048-1-git-send-email-nbd@openwrt.org> <1332025254-5048-2-git-send-email-nbd@openwrt.org> <1332025254-5048-3-git-send-email-nbd@openwrt.org> <1332065875.3609.3.camel@jlt3.sipsolutions.net> <4F65C374.2060505@openwrt.org> <1332146368.3359.12.camel@jlt3.sipsolutions.net> Date: Mon, 19 Mar 2012 10:29:47 +0100 Message-ID: (sfid-20120319_102951_852330_98DC4568) Subject: Re: [PATCH 3/3] mac80211: optimize aggregation session timeout handling From: Helmut Schaa To: Johannes Berg Cc: Felix Fietkau , linux-wireless@vger.kernel.org, linville@tuxdriver.com Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: Hi, On Mon, Mar 19, 2012 at 9:39 AM, Johannes Berg 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);