From 97f97b1f5fe0878b35c8e314f98591771696321b Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 13 Dec 2012 22:54:58 +0100 Subject: [PATCH] mac80211: fix station destruction in AP/mesh modes Unfortunately, commit b22cfcfcae5b, intended to speed up roaming by avoiding the synchronize_rcu() broke AP/mesh modes as it moved some code into that work item that will still call into the driver at a time where it's no longer expected to handle this: after the AP or mesh has been stopped. To fix this problem remove the per-station work struct, maintain a station cleanup list instead and flush this list when stations are flushed. To keep this patch smaller for stable, do this when the stations are flushed (sta_info_flush()). This unfortunately brings back the original roaming delay; I'll fix that again in a separate patch. Also, Ben reported that the original commit could sometimes (with many interfaces) cause long delays when an interface is set down, due to blocking on flush_workqueue(). Since we now maintain the cleanup list, this particular change of the original patch can be reverted. Cc: stable@vger.kernel.org [3.7] Reported-by: Ben Greear Tested-by: Ben Greear Signed-off-by: Johannes Berg --- net/mac80211/ieee80211_i.h | 4 ++++ net/mac80211/iface.c | 28 +++++++++++++----------- net/mac80211/sta_info.c | 44 ++++++++++++++++++++++++++++++++++---- net/mac80211/sta_info.h | 3 ++- 4 files changed, 62 insertions(+), 17 deletions(-) diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index 28fc9875537c..2fcd274877d7 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -773,6 +773,10 @@ struct ieee80211_sub_if_data { u32 mntr_flags; } u; + spinlock_t cleanup_stations_lock; + struct list_head cleanup_stations; + struct work_struct cleanup_stations_wk; + #ifdef CONFIG_MAC80211_DEBUGFS struct { struct dentry *dir; diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index 54fb7f9db564..0f2a9f987f79 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -868,20 +868,11 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, cancel_work_sync(&sdata->work); /* * When we get here, the interface is marked down. - * Call rcu_barrier() to wait both for the RX path + * Call synchronize_rcu() to wait for the RX path * should it be using the interface and enqueuing - * frames at this very time on another CPU, and - * for the sta free call_rcu callbacks. + * frames at this very time on another CPU. */ - rcu_barrier(); - - /* - * free_sta_rcu() enqueues a work for the actual - * sta cleanup, so we need to flush it while - * sdata is still valid. - */ - flush_workqueue(local->workqueue); - + synchronize_rcu(); skb_queue_purge(&sdata->skb_queue); /* @@ -1501,6 +1492,15 @@ static void ieee80211_assign_perm_addr(struct ieee80211_local *local, mutex_unlock(&local->iflist_mtx); } +static void ieee80211_cleanup_sdata_stas_wk(struct work_struct *wk) +{ + struct ieee80211_sub_if_data *sdata; + + sdata = container_of(wk, struct ieee80211_sub_if_data, cleanup_stations_wk); + + ieee80211_cleanup_sdata_stas(sdata); +} + int ieee80211_if_add(struct ieee80211_local *local, const char *name, struct wireless_dev **new_wdev, enum nl80211_iftype type, struct vif_params *params) @@ -1576,6 +1576,10 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name, INIT_LIST_HEAD(&sdata->key_list); + spin_lock_init(&sdata->cleanup_stations_lock); + INIT_LIST_HEAD(&sdata->cleanup_stations); + INIT_WORK(&sdata->cleanup_stations_wk, ieee80211_cleanup_sdata_stas_wk); + for (i = 0; i < IEEE80211_NUM_BANDS; i++) { struct ieee80211_supported_band *sband; sband = local->hw.wiphy->bands[i]; diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c index f3e502502fee..8bbd3b0fdbcc 100644 --- a/net/mac80211/sta_info.c +++ b/net/mac80211/sta_info.c @@ -91,9 +91,8 @@ static int sta_info_hash_del(struct ieee80211_local *local, return -ENOENT; } -static void free_sta_work(struct work_struct *wk) +static void cleanup_single_sta(struct sta_info *sta) { - struct sta_info *sta = container_of(wk, struct sta_info, free_sta_wk); int ac, i; struct tid_ampdu_tx *tid_tx; struct ieee80211_sub_if_data *sdata = sta->sdata; @@ -153,11 +152,35 @@ static void free_sta_work(struct work_struct *wk) sta_info_free(local, sta); } +void ieee80211_cleanup_sdata_stas(struct ieee80211_sub_if_data *sdata) +{ + struct sta_info *sta; + + spin_lock_bh(&sdata->cleanup_stations_lock); + while (!list_empty(&sdata->cleanup_stations)) { + sta = list_first_entry(&sdata->cleanup_stations, + struct sta_info, list); + list_del(&sta->list); + spin_unlock_bh(&sdata->cleanup_stations_lock); + + cleanup_single_sta(sta); + + spin_lock_bh(&sdata->cleanup_stations_lock); + } + + spin_unlock_bh(&sdata->cleanup_stations_lock); +} + static void free_sta_rcu(struct rcu_head *h) { struct sta_info *sta = container_of(h, struct sta_info, rcu_head); + struct ieee80211_sub_if_data *sdata = sta->sdata; - ieee80211_queue_work(&sta->local->hw, &sta->free_sta_wk); + spin_lock(&sdata->cleanup_stations_lock); + list_add_tail(&sta->list, &sdata->cleanup_stations); + spin_unlock(&sdata->cleanup_stations_lock); + + ieee80211_queue_work(&sdata->local->hw, &sdata->cleanup_stations_wk); } /* protected by RCU */ @@ -310,7 +333,6 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata, spin_lock_init(&sta->lock); INIT_WORK(&sta->drv_unblock_wk, sta_unblock); - INIT_WORK(&sta->free_sta_wk, free_sta_work); INIT_WORK(&sta->ampdu_mlme.work, ieee80211_ba_session_work); mutex_init(&sta->ampdu_mlme.mtx); @@ -891,6 +913,20 @@ int sta_info_flush(struct ieee80211_local *local, } mutex_unlock(&local->sta_mtx); + rcu_barrier(); + + if (sdata) { + ieee80211_cleanup_sdata_stas(sdata); + cancel_work_sync(&sdata->cleanup_stations_wk); + } else { + mutex_lock(&local->iflist_mtx); + list_for_each_entry(sdata, &local->interfaces, list) { + ieee80211_cleanup_sdata_stas(sdata); + cancel_work_sync(&sdata->cleanup_stations_wk); + } + mutex_unlock(&local->iflist_mtx); + } + return ret; } diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index 1489bca9ea97..37c1889afd3a 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -299,7 +299,6 @@ struct sta_info { spinlock_t lock; struct work_struct drv_unblock_wk; - struct work_struct free_sta_wk; u16 listen_interval; @@ -563,4 +562,6 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta); void ieee80211_sta_ps_deliver_poll_response(struct sta_info *sta); void ieee80211_sta_ps_deliver_uapsd(struct sta_info *sta); +void ieee80211_cleanup_sdata_stas(struct ieee80211_sub_if_data *sdata); + #endif /* STA_INFO_H */ -- 2.20.1