All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bruno Randolf <br1@einfach.org>
To: Jouni Malinen <j@w1.fi>
Cc: linville@tuxdriver.com, randy.dunlap@oracle.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: Wed, 17 Nov 2010 17:28:06 +0900	[thread overview]
Message-ID: <201011171728.06801.br1@einfach.org> (raw)
In-Reply-To: <20101116093743.GA21872@jm.kir.nu>

On Tue November 16 2010 18:37:43 Jouni Malinen wrote:
> 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?

I understand that this could be more efficient, but if it matters or not - 
honestly, I don't know. Can a more knowledgeable person than me comment on 
this?

bruno

  reply	other threads:[~2010-11-17  8:27 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
2010-11-17  8:28     ` Bruno Randolf [this message]
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=201011171728.06801.br1@einfach.org \
    --to=br1@einfach.org \
    --cc=Lars_Ericsson@telia.com \
    --cc=akpm@linux-foundation.org \
    --cc=blp@cs.stanford.edu \
    --cc=j@w1.fi \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.