On Fri, 2010-04-16 at 22:17 +0200, ext Johannes Berg wrote:
> On Fri, 2010-04-16 at 10:35 +0300, Juuso Oikarinen wrote:
> > The sta_cleanup timer is used to periodically expire buffered frames from the
> > tx buf. The timer is executing periodically, regardless of the need for it.
> > This is wasting resources.
> >
> > Fix this simply by not restarting the sta_cleanup timer if the tx buffer was
> > empty. Restart the timer when there is some more tx-traffic.
> >
> > Cc: Janne Ylälehto <janne.ylalehto@xxxxxxxxx>
> > Signed-off-by: Juuso Oikarinen <juuso.oikarinen@xxxxxxxxx>
> > ---
> > net/mac80211/sta_info.c | 13 ++++++++++---
> > net/mac80211/tx.c | 7 +++++++
> > 2 files changed, 17 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
> > index ff0eb94..3de7a22 100644
> > --- a/net/mac80211/sta_info.c
> > +++ b/net/mac80211/sta_info.c
> > @@ -575,7 +575,7 @@ static int sta_info_buffer_expired(struct sta_info *sta,
> > }
> >
> >
> > -static void sta_info_cleanup_expire_buffered(struct ieee80211_local *local,
> > +static bool sta_info_cleanup_expire_buffered(struct ieee80211_local *local,
> > struct sta_info *sta)
> > {
> > unsigned long flags;
> > @@ -583,7 +583,7 @@ static void sta_info_cleanup_expire_buffered(struct ieee80211_local *local,
> > struct ieee80211_sub_if_data *sdata;
> >
> > if (skb_queue_empty(&sta->ps_tx_buf))
> > - return;
> > + return false;
> >
> > for (;;) {
> > spin_lock_irqsave(&sta->ps_tx_buf.lock, flags);
> > @@ -608,6 +608,8 @@ static void sta_info_cleanup_expire_buffered(struct ieee80211_local *local,
> > if (skb_queue_empty(&sta->ps_tx_buf))
> > sta_info_clear_tim_bit(sta);
> > }
> > +
> > + return true;
> > }
> >
> > static int __must_check __sta_info_destroy(struct sta_info *sta)
> > @@ -755,15 +757,20 @@ static void sta_info_cleanup(unsigned long data)
> > {
> > struct ieee80211_local *local = (struct ieee80211_local *) data;
> > struct sta_info *sta;
> > + bool timer_needed = false;
> >
> > rcu_read_lock();
> > list_for_each_entry_rcu(sta, &local->sta_list, list)
> > - sta_info_cleanup_expire_buffered(local, sta);
> > + if (sta_info_cleanup_expire_buffered(local, sta))
> > + timer_needed = true;
> > rcu_read_unlock();
> >
> > if (local->quiescing)
> > return;
> >
> > + if (!timer_needed)
> > + return;
> > +
> > local->sta_cleanup.expires =
> > round_jiffies(jiffies + STA_INFO_CLEANUP_INTERVAL);
> > add_timer(&local->sta_cleanup);
> > diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
> > index 2cb7726..e2aa972 100644
> > --- a/net/mac80211/tx.c
> > +++ b/net/mac80211/tx.c
> > @@ -429,6 +429,7 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
> > struct sta_info *sta = tx->sta;
> > struct ieee80211_tx_info *info = IEEE80211_SKB_CB(tx->skb);
> > struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
> > + struct ieee80211_local *local = tx->local;
> > u32 staflags;
> >
> > if (unlikely(!sta ||
> > @@ -476,6 +477,12 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
> > info->control.vif = &tx->sdata->vif;
> > info->flags |= IEEE80211_TX_INTFL_NEED_TXPROCESSING;
> > skb_queue_tail(&sta->ps_tx_buf, tx->skb);
> > +
> > + if (!timer_pending(&local->sta_cleanup))
> > + mod_timer(&local->sta_cleanup,
> > + round_jiffies(jiffies +
> > + STA_INFO_CLEANUP_INTERVAL));
> > +
>
> Why bother to check !timer_pending?
>
Otherwise, if we have some periodic TX going on with an interval less
than STA_INFO_CLEANUP_INTERVAL, the sta_cleanup could be delayed
considerably.
-Juuso
> johannes
>
--
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
