On 5 June 2014 18:26, Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx> wrote:
> From: Luciano Coelho <luciano.coelho@xxxxxxxxx>
>
> Instead of stopping all the hardware queues during channel switch,
> which is especially bad when we have large CSA counts, stop only the
> queues that are assigned to the vif that is performing the channel
> switch.
>
> Signed-off-by: Luciano Coelho <luciano.coelho@xxxxxxxxx>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@xxxxxxxxx>
> ---
> net/mac80211/cfg.c | 10 ++++------
> net/mac80211/iface.c | 5 ++---
> net/mac80211/mlme.c | 20 ++++++++------------
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index c6fe358..5215abb 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1142,9 +1142,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
> mutex_lock(&local->mtx);
> sdata->vif.csa_active = false;
> if (!ieee80211_csa_needs_block_tx(local))
> - ieee80211_wake_queues_by_reason(&local->hw,
> - IEEE80211_MAX_QUEUE_MAP,
> - IEEE80211_QUEUE_STOP_REASON_CSA);
> + ieee80211_wake_vif_queues(local, sdata,
> + IEEE80211_QUEUE_STOP_REASON_CSA);
> mutex_unlock(&local->mtx);
>
> kfree(sdata->u.ap.next_beacon);
> @@ -3146,9 +3145,8 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
> cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
>
> if (!ieee80211_csa_needs_block_tx(local))
> - ieee80211_wake_queues_by_reason(&local->hw,
> - IEEE80211_MAX_QUEUE_MAP,
> - IEEE80211_QUEUE_STOP_REASON_CSA);
> + ieee80211_wake_vif_queues(local, sdata,
> + IEEE80211_QUEUE_STOP_REASON_CSA);
I don't think this is going to work with the upcomming multi-vif
channel switching.
The ieee80211_csa_needs_block_tx() will become false upon *last*
interface switch completion. This means preceeding interface csa
finalizations won't call ieee80211_wake_vif_queues() so you actually
end up not waking all of the queues when you finish csa.
I think this can be solved with:
if (!ieee80211_csa_needs_block_tx(local))
list_for_each(sdata, &local->interfaces, list)
ieee80211_wake_vif_queues(local, sdata, REASON_CSA);
But then I guess it's just a convoluted way of saying:
if (!ieee80211_csa_needs_block_tx(local))
ieee80211_wake_queues_by_reason(hw, REASON_CSA);
IOW waking should remain as it was while stopping can be done per-vif.
This goes for all instances calling wake_vif_queues() in the patch.
Generally I don't think having wake_vif_queues() is a good idea. It's
really tricky. Sure, you can have a stop_vif_queues() which will
behave as you might expect (i.e. first call stops queues and
overlapping vif-queue mappings are not a concern). But
wake_vif_queues() seems to have little practical use to me without any
kind of per-queue reference counting.
Michał
--
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
