linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jouni Malinen <j@w1.fi>
To: Bruno Randolf <br1@einfach.org>
Cc: linville@tuxdriver.com, randy.dunlap@oracle.com,
	br1@thinktube.com, peterz@infradead.org, blp@cs.stanford.edu,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	Lars_Ericsson@telia.com, stefanr@s5r6.in-berlin.de,
	kosaki.motohiro@jp.fujitsu.com, akpm@linux-foundation.org,
	kevin.granade@gmail.com
Subject: Re: [PATCH v7 3/3] nl80211/mac80211: Report signal average
Date: Tue, 16 Nov 2010 11:37:43 +0200	[thread overview]
Message-ID: <20101116093743.GA21872@jm.kir.nu> (raw)
In-Reply-To: <20101112030035.28522.75318.stgit@localhost6.localdomain6>

On Fri, Nov 12, 2010 at 12:00:35PM +0900, Bruno Randolf wrote:
> Extend nl80211 to report an exponential weighted moving average (EWMA) of the
> signal value. Since the signal value usually fluctuates between different
> packets, an average can be more useful than the value of the last packet.
> 
> This uses the recently added generic EWMA library function.

Isn't that generic EWMA library function making this much more CPU
intensive than necessary? I would assume the compiler is not able to
optimize the unsigned long division in ewma_add() when the weight value
is "hidden" in this way through ewma_init() call. While generic library
functions can be nice, it could be useful to have an option for using an
inline version of ewma_add that takes in avg->weight as one of the
arguments to allow cases like weight=8 here to be implemented using
shift right instead of unsigned long division especially when done on
hot path..

> @@ -1158,6 +1158,7 @@ ieee80211_rx_h_sta_process(struct ieee80211_rx_data *rx)
>  	sta->last_signal = status->signal;
> +	ewma_add(&sta->avg_signal, -status->signal);

This one here would be done for every frame received and by using a
function call that ends up doing a potentially expensive division.

> @@ -241,6 +241,8 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
> +	ewma_init(&sta->avg_signal, 1000, 8);

The divisor (weight) is set to 8 here which should allow for quite a bit
more efficient ewma_add implementation if it were to be done in a way
that makes it easier for the compiler to see what value is being used
(e.g., passing this 8 to ewma_add_inline()) to allow shift right to be
used..


Or am I missing something here and we have a compiler that is actually
able to take care of this kind of optimizations by itself in the current
ewma library design? Or is the unsigned long division with the expected
weight values going to be fast enough to not justify caring about it
even on any of the embedded boards anyway?

-- 
Jouni Malinen                                            PGP id EFC895FA

  reply	other threads:[~2010-11-16  9:38 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-12  3:00 [PATCH v7 0/3] Generic exponentially weighted moving average (EWMA) Bruno Randolf
2010-11-12  3:00 ` [PATCH v7 1/3] Add generic exponentially weighted moving average (EWMA) function Bruno Randolf
2010-11-12  3:00 ` [PATCH v7 2/3] ath5k: Use generic EWMA library Bruno Randolf
2010-11-12  3:00 ` [PATCH v7 3/3] nl80211/mac80211: Report signal average Bruno Randolf
2010-11-16  9:37   ` Jouni Malinen [this message]
2010-11-17  8:28     ` Bruno Randolf
2010-11-17 16:16       ` Johannes Berg
2010-11-17 23:11         ` Bob Copeland
2010-11-19  8:49           ` Bruno Randolf
2010-11-19 14:04             ` Stefan Richter
2010-11-22  2:41               ` Bruno Randolf
2010-11-22  7:26                 ` Stefan Richter
2010-11-19 17:52             ` Johannes Berg
2010-11-22  2:36               ` Bruno Randolf
2010-11-19  9:07           ` Bruno Randolf
2010-11-19 12:16             ` Peter Zijlstra
2010-11-19 22:28             ` Bob Copeland
2010-11-19 23:01               ` Peter Zijlstra
2010-12-02  8:12                 ` Bruno Randolf
2010-11-19 18:58     ` Johannes Berg
2010-11-22 18:46       ` John W. Linville
2010-11-20 16:45   ` Brian Prodoehl
2010-11-24 16:24   ` Johannes Berg
2010-11-24 19:05     ` Ben Greear

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101116093743.GA21872@jm.kir.nu \
    --to=j@w1.fi \
    --cc=Lars_Ericsson@telia.com \
    --cc=akpm@linux-foundation.org \
    --cc=blp@cs.stanford.edu \
    --cc=br1@einfach.org \
    --cc=br1@thinktube.com \
    --cc=kevin.granade@gmail.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=peterz@infradead.org \
    --cc=randy.dunlap@oracle.com \
    --cc=stefanr@s5r6.in-berlin.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).