* [PATCH 0/2] station management complexity reduction
@ 2011-12-14 13:03 Johannes Berg
2011-12-14 13:03 ` [PATCH 1/2] mac80211: delay IBSS station insertion Johannes Berg
2011-12-14 13:03 ` [PATCH 2/2] mac80211: reduce station management complexity Johannes Berg
0 siblings, 2 replies; 5+ messages in thread
From: Johannes Berg @ 2011-12-14 13:03 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
The complexity of locking in sta_info.c has bothered
me for a while, and it would not have been possible to
inform drivers of station state in a sleeping callback
from IBSS anyway.
Make IBSS not add the stations in RX context, and thus
reduce complexity in sta_info and actually make the new
sta_info_move_state() sleeping.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] mac80211: delay IBSS station insertion
2011-12-14 13:03 [PATCH 0/2] station management complexity reduction Johannes Berg
@ 2011-12-14 13:03 ` Johannes Berg
2011-12-15 10:17 ` [PATCH v2 " Johannes Berg
2011-12-14 13:03 ` [PATCH 2/2] mac80211: reduce station management complexity Johannes Berg
1 sibling, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2011-12-14 13:03 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
From: Johannes Berg <johannes.berg@intel.com>
In order to notify drivers and simplify the station
management code, defer IBSS station insertion to a
work item and don't do it directly while receiving
a frame.
This increases the complexity in IBSS a little bit,
but it's pretty straight forward and it allows us
to reduce the station management complexity (next
patch) considerably.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/ibss.c | 154 +++++++++++++++++++++++++++++++++++----------
net/mac80211/ieee80211_i.h | 8 +-
net/mac80211/rx.c | 4 -
net/mac80211/sta_info.c | 2
4 files changed, 130 insertions(+), 38 deletions(-)
--- a/net/mac80211/ibss.c 2011-12-14 13:29:08.000000000 +0100
+++ b/net/mac80211/ibss.c 2011-12-14 13:57:18.000000000 +0100
@@ -275,6 +275,80 @@ static void ieee80211_sta_join_ibss(stru
cbss->tsf);
}
+static struct sta_info *ieee80211_ibss_finish_sta(struct sta_info *sta)
+ __acquires(RCU)
+{
+ struct ieee80211_sub_if_data *sdata = sta->sdata;
+ u8 addr[ETH_ALEN];
+
+ memcpy(addr, sta->sta.addr, ETH_ALEN);
+
+#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
+ wiphy_debug(sdata->local->hw.wiphy,
+ "Adding new IBSS station %pM (dev=%s)\n",
+ addr, sdata->name);
+#endif
+
+ sta_info_move_state(sta, IEEE80211_STA_AUTH);
+ sta_info_move_state(sta, IEEE80211_STA_ASSOC);
+ sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+
+ rate_control_rate_init(sta);
+
+ /* If it fails, maybe we raced another insertion? */
+ if (sta_info_insert_rcu(sta))
+ return sta_info_get(sdata, addr);
+ return sta;
+}
+
+static struct sta_info *
+ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
+ const u8 *bssid, const u8 *addr,
+ u32 supp_rates)
+ __acquires(RCU)
+{
+ struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta;
+ int band = local->hw.conf.channel->band;
+
+ /*
+ * XXX: Consider removing the least recently used entry and
+ * allow new one to be added.
+ */
+ if (local->num_sta >= IEEE80211_IBSS_MAX_STA_ENTRIES) {
+ if (net_ratelimit())
+ printk(KERN_DEBUG "%s: No room for a new IBSS STA entry %pM\n",
+ sdata->name, addr);
+ rcu_read_lock();
+ return NULL;
+ }
+
+ if (ifibss->state == IEEE80211_IBSS_MLME_SEARCH) {
+ rcu_read_lock();
+ return NULL;
+ }
+
+ if (compare_ether_addr(bssid, sdata->u.ibss.bssid)) {
+ rcu_read_lock();
+ return NULL;
+ }
+
+ sta = sta_info_alloc(sdata, addr, GFP_KERNEL);
+ if (!sta) {
+ rcu_read_lock();
+ return NULL;
+ }
+
+ sta->last_rx = jiffies;
+
+ /* make sure mandatory rates are always added */
+ sta->sta.supp_rates[band] = supp_rates |
+ ieee80211_mandatory_rates(local, band);
+
+ return ieee80211_ibss_finish_sta(sta);
+}
+
static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt,
size_t len,
@@ -334,10 +408,11 @@ static void ieee80211_rx_bss_info(struct
#endif
rates_updated = true;
}
- } else
+ } else {
+ rcu_read_unlock();
sta = ieee80211_ibss_add_sta(sdata, mgmt->bssid,
- mgmt->sa, supp_rates,
- GFP_ATOMIC);
+ mgmt->sa, supp_rates);
+ }
}
if (sta && elems->wmm_info)
@@ -464,21 +539,17 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sta_join_ibss(sdata, bss);
supp_rates = ieee80211_sta_get_rates(local, elems, band);
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
- supp_rates, GFP_KERNEL);
+ supp_rates);
+ rcu_read_unlock();
}
put_bss:
ieee80211_rx_bss_put(local, bss);
}
-/*
- * Add a new IBSS station, will also be called by the RX code when,
- * in IBSS mode, receiving a frame from a yet-unknown station, hence
- * must be callable in atomic context.
- */
-struct sta_info *ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
- u8 *bssid, u8 *addr, u32 supp_rates,
- gfp_t gfp)
+void ieee80211_ibss_rx_no_sta(struct ieee80211_sub_if_data *sdata,
+ const u8 *bssid, const u8 *addr,
+ u32 supp_rates)
{
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
struct ieee80211_local *local = sdata->local;
@@ -493,40 +564,29 @@ struct sta_info *ieee80211_ibss_add_sta(
if (net_ratelimit())
printk(KERN_DEBUG "%s: No room for a new IBSS STA entry %pM\n",
sdata->name, addr);
- return NULL;
+ return;
}
if (ifibss->state == IEEE80211_IBSS_MLME_SEARCH)
- return NULL;
+ return;
if (compare_ether_addr(bssid, sdata->u.ibss.bssid))
- return NULL;
-
-#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
- wiphy_debug(local->hw.wiphy, "Adding new IBSS station %pM (dev=%s)\n",
- addr, sdata->name);
-#endif
+ return;
- sta = sta_info_alloc(sdata, addr, gfp);
+ sta = sta_info_alloc(sdata, addr, GFP_ATOMIC);
if (!sta)
- return NULL;
+ return;
sta->last_rx = jiffies;
- sta_info_move_state(sta, IEEE80211_STA_AUTH);
- sta_info_move_state(sta, IEEE80211_STA_ASSOC);
- sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
-
/* make sure mandatory rates are always added */
sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);
- rate_control_rate_init(sta);
-
- /* If it fails, maybe we raced another insertion? */
- if (sta_info_insert(sta))
- return sta_info_get(sdata, addr);
- return sta;
+ spin_lock(&ifibss->incomplete_lock);
+ list_add(&sta->list, &ifibss->incomplete_stations);
+ spin_unlock(&ifibss->incomplete_lock);
+ ieee80211_queue_work(&local->hw, &sdata->work);
}
static int ieee80211_sta_active_ibss(struct ieee80211_sub_if_data *sdata)
@@ -865,6 +925,7 @@ void ieee80211_ibss_rx_queued_mgmt(struc
void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
+ struct sta_info *sta;
mutex_lock(&ifibss->mtx);
@@ -876,6 +937,19 @@ void ieee80211_ibss_work(struct ieee8021
if (!ifibss->ssid_len)
goto out;
+ spin_lock_bh(&ifibss->incomplete_lock);
+ while (!list_empty(&ifibss->incomplete_stations)) {
+ sta = list_first_entry(&ifibss->incomplete_stations,
+ struct sta_info, list);
+ list_del(&sta->list);
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
+ ieee80211_ibss_finish_sta(sta);
+ rcu_read_unlock();
+ spin_lock_bh(&ifibss->incomplete_lock);
+ }
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
switch (ifibss->state) {
case IEEE80211_IBSS_MLME_SEARCH:
ieee80211_sta_find_ibss(sdata);
@@ -934,6 +1008,8 @@ void ieee80211_ibss_setup_sdata(struct i
setup_timer(&ifibss->timer, ieee80211_ibss_timer,
(unsigned long) sdata);
mutex_init(&ifibss->mtx);
+ INIT_LIST_HEAD(&ifibss->incomplete_stations);
+ spin_lock_init(&ifibss->incomplete_lock);
}
/* scan finished notification */
@@ -1050,6 +1126,7 @@ int ieee80211_ibss_leave(struct ieee8021
struct cfg80211_bss *cbss;
u16 capability;
int active_ibss;
+ struct sta_info *sta;
mutex_lock(&sdata->u.ibss.mtx);
@@ -1078,6 +1155,19 @@ int ieee80211_ibss_leave(struct ieee8021
}
sta_info_flush(sdata->local, sdata);
+
+ spin_lock_bh(&ifibss->incomplete_lock);
+ while (!list_empty(&ifibss->incomplete_stations)) {
+ sta = list_first_entry(&ifibss->incomplete_stations,
+ struct sta_info, list);
+ list_del(&sta->list);
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
+ sta_info_free(local, sta);
+ spin_lock_bh(&ifibss->incomplete_lock);
+ }
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
netif_carrier_off(sdata->dev);
/* remove beacon */
--- a/net/mac80211/ieee80211_i.h 2011-12-14 13:29:08.000000000 +0100
+++ b/net/mac80211/ieee80211_i.h 2011-12-14 13:57:54.000000000 +0100
@@ -482,6 +482,9 @@ struct ieee80211_if_ibss {
struct sk_buff __rcu *presp;
struct sk_buff *skb;
+ spinlock_t incomplete_lock;
+ struct list_head incomplete_stations;
+
enum {
IEEE80211_IBSS_MLME_SEARCH,
IEEE80211_IBSS_MLME_JOINED,
@@ -1172,9 +1175,8 @@ void ieee80211_sta_reset_conn_monitor(st
/* IBSS code */
void ieee80211_ibss_notify_scan_completed(struct ieee80211_local *local);
void ieee80211_ibss_setup_sdata(struct ieee80211_sub_if_data *sdata);
-struct sta_info *ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
- u8 *bssid, u8 *addr, u32 supp_rates,
- gfp_t gfp);
+void ieee80211_ibss_rx_no_sta(struct ieee80211_sub_if_data *sdata,
+ const u8 *bssid, const u8 *addr, u32 supp_rates);
int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
struct cfg80211_ibss_params *params);
int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata);
--- a/net/mac80211/rx.c 2011-12-14 13:29:08.000000000 +0100
+++ b/net/mac80211/rx.c 2011-12-14 13:29:12.000000000 +0100
@@ -2775,8 +2775,8 @@ static int prepare_for_handlers(struct i
rate_idx = 0; /* TODO: HT rates */
else
rate_idx = status->rate_idx;
- rx->sta = ieee80211_ibss_add_sta(sdata, bssid,
- hdr->addr2, BIT(rate_idx), GFP_ATOMIC);
+ ieee80211_ibss_rx_no_sta(sdata, bssid, hdr->addr2,
+ BIT(rate_idx));
}
break;
case NL80211_IFTYPE_MESH_POINT:
--- a/net/mac80211/sta_info.c 2011-12-14 13:33:13.000000000 +0100
+++ b/net/mac80211/sta_info.c 2011-12-14 13:58:04.000000000 +0100
@@ -1521,7 +1521,7 @@ EXPORT_SYMBOL(ieee80211_sta_set_buffered
int sta_info_move_state_checked(struct sta_info *sta,
enum ieee80211_sta_state new_state)
{
- /* might_sleep(); -- for driver notify later, fix IBSS first */
+ might_sleep();
if (sta->sta_state == new_state)
return 0;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] mac80211: reduce station management complexity
2011-12-14 13:03 [PATCH 0/2] station management complexity reduction Johannes Berg
2011-12-14 13:03 ` [PATCH 1/2] mac80211: delay IBSS station insertion Johannes Berg
@ 2011-12-14 13:03 ` Johannes Berg
2011-12-15 10:24 ` [PATCH v2 " Johannes Berg
1 sibling, 1 reply; 5+ messages in thread
From: Johannes Berg @ 2011-12-14 13:03 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
From: Johannes Berg <johannes.berg@intel.com>
Now that IBSS no longer needs to insert stations
from atomic context, we can get rid of all the
special cases for that, and even get rid of the
sta_lock (though it needs to stay as tim_lock.)
This makes the station management code much more
straight-forward.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/ieee80211_i.h | 11 +--
net/mac80211/sta_info.c | 152 ++++-----------------------------------------
net/mac80211/tx.c | 4 -
3 files changed, 22 insertions(+), 145 deletions(-)
--- a/net/mac80211/sta_info.c 2011-12-14 13:58:04.000000000 +0100
+++ b/net/mac80211/sta_info.c 2011-12-14 14:03:28.000000000 +0100
@@ -62,14 +62,14 @@
* freed before they are done using it.
*/
-/* Caller must hold local->sta_lock */
+/* Caller must hold local->sta_mtx */
static int sta_info_hash_del(struct ieee80211_local *local,
struct sta_info *sta)
{
struct sta_info *s;
s = rcu_dereference_protected(local->sta_hash[STA_HASH(sta->sta.addr)],
- lockdep_is_held(&local->sta_lock));
+ lockdep_is_held(&local->sta_mtx));
if (!s)
return -ENOENT;
if (s == sta) {
@@ -81,7 +81,7 @@ static int sta_info_hash_del(struct ieee
while (rcu_access_pointer(s->hnext) &&
rcu_access_pointer(s->hnext) != sta)
s = rcu_dereference_protected(s->hnext,
- lockdep_is_held(&local->sta_lock));
+ lockdep_is_held(&local->sta_mtx));
if (rcu_access_pointer(s->hnext)) {
RCU_INIT_POINTER(s->hnext, sta->hnext);
return 0;
@@ -98,14 +98,12 @@ struct sta_info *sta_info_get(struct iee
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if (sta->sdata == sdata && !sta->dummy &&
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -119,14 +117,12 @@ struct sta_info *sta_info_get_rx(struct
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if (sta->sdata == sdata &&
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -143,7 +139,6 @@ struct sta_info *sta_info_get_bss(struct
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if ((sta->sdata == sdata ||
@@ -152,7 +147,6 @@ struct sta_info *sta_info_get_bss(struct
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -169,7 +163,6 @@ struct sta_info *sta_info_get_bss_rx(str
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if ((sta->sdata == sdata ||
@@ -177,7 +170,6 @@ struct sta_info *sta_info_get_bss_rx(str
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -228,10 +220,11 @@ void sta_info_free(struct ieee80211_loca
kfree(sta);
}
-/* Caller must hold local->sta_lock */
+/* Caller must hold local->sta_mtx */
static void sta_info_hash_add(struct ieee80211_local *local,
struct sta_info *sta)
{
+ lockdep_assert_held(&local->sta_mtx);
sta->hnext = local->sta_hash[STA_HASH(sta->sta.addr)];
RCU_INIT_POINTER(local->sta_hash[STA_HASH(sta->sta.addr)], sta);
}
@@ -345,7 +338,6 @@ static int sta_info_finish_insert(struct
struct ieee80211_local *local = sta->local;
struct ieee80211_sub_if_data *sdata = sta->sdata;
struct station_info sinfo;
- unsigned long flags;
int err = 0;
lockdep_assert_held(&local->sta_mtx);
@@ -379,9 +371,7 @@ static int sta_info_finish_insert(struct
smp_mb();
/* make the station visible */
- spin_lock_irqsave(&local->sta_lock, flags);
sta_info_hash_add(local, sta);
- spin_unlock_irqrestore(&local->sta_lock, flags);
}
list_add(&sta->list, &local->sta_list);
@@ -402,35 +392,6 @@ static int sta_info_finish_insert(struct
return 0;
}
-static void sta_info_finish_pending(struct ieee80211_local *local)
-{
- struct sta_info *sta;
- unsigned long flags;
-
- spin_lock_irqsave(&local->sta_lock, flags);
- while (!list_empty(&local->sta_pending_list)) {
- sta = list_first_entry(&local->sta_pending_list,
- struct sta_info, list);
- list_del(&sta->list);
- spin_unlock_irqrestore(&local->sta_lock, flags);
-
- sta_info_finish_insert(sta, true, false);
-
- spin_lock_irqsave(&local->sta_lock, flags);
- }
- spin_unlock_irqrestore(&local->sta_lock, flags);
-}
-
-static void sta_info_finish_work(struct work_struct *work)
-{
- struct ieee80211_local *local =
- container_of(work, struct ieee80211_local, sta_finish_work);
-
- mutex_lock(&local->sta_mtx);
- sta_info_finish_pending(local);
- mutex_unlock(&local->sta_mtx);
-}
-
static int sta_info_insert_check(struct sta_info *sta)
{
struct ieee80211_sub_if_data *sdata = sta->sdata;
@@ -450,50 +411,15 @@ static int sta_info_insert_check(struct
return 0;
}
-static int sta_info_insert_ibss(struct sta_info *sta) __acquires(RCU)
-{
- struct ieee80211_local *local = sta->local;
- struct ieee80211_sub_if_data *sdata = sta->sdata;
- unsigned long flags;
-
- spin_lock_irqsave(&local->sta_lock, flags);
- /* check if STA exists already */
- if (sta_info_get_bss_rx(sdata, sta->sta.addr)) {
- spin_unlock_irqrestore(&local->sta_lock, flags);
- rcu_read_lock();
- return -EEXIST;
- }
-
- local->num_sta++;
- local->sta_generation++;
- smp_mb();
- sta_info_hash_add(local, sta);
-
- list_add_tail(&sta->list, &local->sta_pending_list);
-
- rcu_read_lock();
- spin_unlock_irqrestore(&local->sta_lock, flags);
-
-#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
- wiphy_debug(local->hw.wiphy, "Added IBSS STA %pM\n",
- sta->sta.addr);
-#endif /* CONFIG_MAC80211_VERBOSE_DEBUG */
-
- ieee80211_queue_work(&local->hw, &local->sta_finish_work);
-
- return 0;
-}
-
/*
* should be called with sta_mtx locked
* this function replaces the mutex lock
* with a RCU lock
*/
-static int sta_info_insert_non_ibss(struct sta_info *sta) __acquires(RCU)
+static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
{
struct ieee80211_local *local = sta->local;
struct ieee80211_sub_if_data *sdata = sta->sdata;
- unsigned long flags;
struct sta_info *exist_sta;
bool dummy_reinsert = false;
int err = 0;
@@ -501,19 +427,8 @@ static int sta_info_insert_non_ibss(stru
lockdep_assert_held(&local->sta_mtx);
/*
- * On first glance, this will look racy, because the code
- * in this function, which inserts a station with sleeping,
- * unlocks the sta_lock between checking existence in the
- * hash table and inserting into it.
- *
- * However, it is not racy against itself because it keeps
- * the mutex locked.
- */
-
- spin_lock_irqsave(&local->sta_lock, flags);
- /*
* check if STA exists already.
- * only accept a scenario of a second call to sta_info_insert_non_ibss
+ * only accept a scenario of a second call to sta_info_insert_finish
* with a dummy station entry that was inserted earlier
* in that case - assume that the dummy station flag should
* be removed.
@@ -523,15 +438,12 @@ static int sta_info_insert_non_ibss(stru
if (exist_sta == sta && sta->dummy) {
dummy_reinsert = true;
} else {
- spin_unlock_irqrestore(&local->sta_lock, flags);
mutex_unlock(&local->sta_mtx);
rcu_read_lock();
return -EEXIST;
}
}
- spin_unlock_irqrestore(&local->sta_lock, flags);
-
err = sta_info_finish_insert(sta, false, dummy_reinsert);
if (err) {
mutex_unlock(&local->sta_mtx);
@@ -557,42 +469,19 @@ static int sta_info_insert_non_ibss(stru
int sta_info_insert_rcu(struct sta_info *sta) __acquires(RCU)
{
struct ieee80211_local *local = sta->local;
- struct ieee80211_sub_if_data *sdata = sta->sdata;
int err = 0;
+ might_sleep();
+
err = sta_info_insert_check(sta);
if (err) {
rcu_read_lock();
goto out_free;
}
- /*
- * In ad-hoc mode, we sometimes need to insert stations
- * from tasklet context from the RX path. To avoid races,
- * always do so in that case -- see the comment below.
- */
- if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
- err = sta_info_insert_ibss(sta);
- if (err)
- goto out_free;
-
- return 0;
- }
-
- /*
- * It might seem that the function called below is in race against
- * the function call above that atomically inserts the station... That,
- * however, is not true because the above code can only
- * be invoked for IBSS interfaces, and the below code will
- * not be -- and the two do not race against each other as
- * the hash table also keys off the interface.
- */
-
- might_sleep();
-
mutex_lock(&local->sta_mtx);
- err = sta_info_insert_non_ibss(sta);
+ err = sta_info_insert_finish(sta);
if (err)
goto out_free;
@@ -626,7 +515,7 @@ int sta_info_reinsert(struct sta_info *s
might_sleep();
- err = sta_info_insert_non_ibss(sta);
+ err = sta_info_insert_finish(sta);
rcu_read_unlock();
return err;
}
@@ -713,7 +602,7 @@ void sta_info_recalc_tim(struct sta_info
}
done:
- spin_lock_irqsave(&local->sta_lock, flags);
+ spin_lock_irqsave(&local->tim_lock, flags);
if (indicate_tim)
__bss_tim_set(bss, sta->sta.aid);
@@ -726,7 +615,7 @@ void sta_info_recalc_tim(struct sta_info
local->tim_in_locked_section = false;
}
- spin_unlock_irqrestore(&local->sta_lock, flags);
+ spin_unlock_irqrestore(&local->tim_lock, flags);
}
static bool sta_info_buffer_expired(struct sta_info *sta, struct sk_buff *skb)
@@ -850,7 +739,6 @@ static int __must_check __sta_info_destr
{
struct ieee80211_local *local;
struct ieee80211_sub_if_data *sdata;
- unsigned long flags;
int ret, i, ac;
might_sleep();
@@ -870,15 +758,12 @@ static int __must_check __sta_info_destr
set_sta_flag(sta, WLAN_STA_BLOCK_BA);
ieee80211_sta_tear_down_BA_sessions(sta, true);
- spin_lock_irqsave(&local->sta_lock, flags);
ret = sta_info_hash_del(local, sta);
- /* this might still be the pending list ... which is fine */
- if (!ret)
- list_del(&sta->list);
- spin_unlock_irqrestore(&local->sta_lock, flags);
if (ret)
return ret;
+ list_del(&sta->list);
+
mutex_lock(&local->key_mtx);
for (i = 0; i < NUM_DEFAULT_KEYS; i++)
__ieee80211_key_free(key_mtx_dereference(local, sta->gtk[i]));
@@ -1009,11 +894,9 @@ static void sta_info_cleanup(unsigned lo
void sta_info_init(struct ieee80211_local *local)
{
- spin_lock_init(&local->sta_lock);
+ spin_lock_init(&local->tim_lock);
mutex_init(&local->sta_mtx);
INIT_LIST_HEAD(&local->sta_list);
- INIT_LIST_HEAD(&local->sta_pending_list);
- INIT_WORK(&local->sta_finish_work, sta_info_finish_work);
setup_timer(&local->sta_cleanup, sta_info_cleanup,
(unsigned long)local);
@@ -1042,9 +925,6 @@ int sta_info_flush(struct ieee80211_loca
might_sleep();
mutex_lock(&local->sta_mtx);
-
- sta_info_finish_pending(local);
-
list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
if (!sdata || sdata == sta->sdata)
WARN_ON(__sta_info_destroy(sta));
--- a/net/mac80211/ieee80211_i.h 2011-12-14 13:57:54.000000000 +0100
+++ b/net/mac80211/ieee80211_i.h 2011-12-14 14:03:28.000000000 +0100
@@ -855,18 +855,15 @@ struct ieee80211_local {
/* Station data */
/*
- * The mutex only protects the list and counter,
- * reads are done in RCU.
- * Additionally, the lock protects the hash table,
- * the pending list and each BSS's TIM bitmap.
+ * The mutex only protects the list, hash table and
+ * counter, reads are done with RCU.
*/
struct mutex sta_mtx;
- spinlock_t sta_lock;
+ spinlock_t tim_lock;
unsigned long num_sta;
- struct list_head sta_list, sta_pending_list;
+ struct list_head sta_list;
struct sta_info __rcu *sta_hash[STA_HASH_SIZE];
struct timer_list sta_cleanup;
- struct work_struct sta_finish_work;
int sta_generation;
struct sk_buff_head pending[IEEE80211_MAX_QUEUES];
--- a/net/mac80211/tx.c 2011-12-14 13:57:54.000000000 +0100
+++ b/net/mac80211/tx.c 2011-12-14 14:03:28.000000000 +0100
@@ -2333,9 +2333,9 @@ struct sk_buff *ieee80211_beacon_get_tim
} else {
unsigned long flags;
- spin_lock_irqsave(&local->sta_lock, flags);
+ spin_lock_irqsave(&local->tim_lock, flags);
ieee80211_beacon_add_tim(ap, skb, beacon);
- spin_unlock_irqrestore(&local->sta_lock, flags);
+ spin_unlock_irqrestore(&local->tim_lock, flags);
}
if (tim_offset)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] mac80211: delay IBSS station insertion
2011-12-14 13:03 ` [PATCH 1/2] mac80211: delay IBSS station insertion Johannes Berg
@ 2011-12-15 10:17 ` Johannes Berg
0 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2011-12-15 10:17 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
From: Johannes Berg <johannes.berg@intel.com>
In order to notify drivers and simplify the station
management code, defer IBSS station insertion to a
work item and don't do it directly while receiving
a frame.
This increases the complexity in IBSS a little bit,
but it's pretty straight forward and it allows us
to reduce the station management complexity (next
patch) considerably.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/ibss.c | 154 +++++++++++++++++++++++++++++++++++----------
net/mac80211/ieee80211_i.h | 8 +-
net/mac80211/rx.c | 4 -
net/mac80211/sta_info.c | 31 +++------
4 files changed, 140 insertions(+), 57 deletions(-)
--- a/net/mac80211/ibss.c 2011-12-15 10:27:47.000000000 +0100
+++ b/net/mac80211/ibss.c 2011-12-15 10:27:48.000000000 +0100
@@ -275,6 +275,80 @@ static void ieee80211_sta_join_ibss(stru
cbss->tsf);
}
+static struct sta_info *ieee80211_ibss_finish_sta(struct sta_info *sta)
+ __acquires(RCU)
+{
+ struct ieee80211_sub_if_data *sdata = sta->sdata;
+ u8 addr[ETH_ALEN];
+
+ memcpy(addr, sta->sta.addr, ETH_ALEN);
+
+#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
+ wiphy_debug(sdata->local->hw.wiphy,
+ "Adding new IBSS station %pM (dev=%s)\n",
+ addr, sdata->name);
+#endif
+
+ sta_info_move_state(sta, IEEE80211_STA_AUTH);
+ sta_info_move_state(sta, IEEE80211_STA_ASSOC);
+ sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
+
+ rate_control_rate_init(sta);
+
+ /* If it fails, maybe we raced another insertion? */
+ if (sta_info_insert_rcu(sta))
+ return sta_info_get(sdata, addr);
+ return sta;
+}
+
+static struct sta_info *
+ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
+ const u8 *bssid, const u8 *addr,
+ u32 supp_rates)
+ __acquires(RCU)
+{
+ struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
+ struct ieee80211_local *local = sdata->local;
+ struct sta_info *sta;
+ int band = local->hw.conf.channel->band;
+
+ /*
+ * XXX: Consider removing the least recently used entry and
+ * allow new one to be added.
+ */
+ if (local->num_sta >= IEEE80211_IBSS_MAX_STA_ENTRIES) {
+ if (net_ratelimit())
+ printk(KERN_DEBUG "%s: No room for a new IBSS STA entry %pM\n",
+ sdata->name, addr);
+ rcu_read_lock();
+ return NULL;
+ }
+
+ if (ifibss->state == IEEE80211_IBSS_MLME_SEARCH) {
+ rcu_read_lock();
+ return NULL;
+ }
+
+ if (compare_ether_addr(bssid, sdata->u.ibss.bssid)) {
+ rcu_read_lock();
+ return NULL;
+ }
+
+ sta = sta_info_alloc(sdata, addr, GFP_KERNEL);
+ if (!sta) {
+ rcu_read_lock();
+ return NULL;
+ }
+
+ sta->last_rx = jiffies;
+
+ /* make sure mandatory rates are always added */
+ sta->sta.supp_rates[band] = supp_rates |
+ ieee80211_mandatory_rates(local, band);
+
+ return ieee80211_ibss_finish_sta(sta);
+}
+
static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt,
size_t len,
@@ -334,10 +408,11 @@ static void ieee80211_rx_bss_info(struct
#endif
rates_updated = true;
}
- } else
+ } else {
+ rcu_read_unlock();
sta = ieee80211_ibss_add_sta(sdata, mgmt->bssid,
- mgmt->sa, supp_rates,
- GFP_ATOMIC);
+ mgmt->sa, supp_rates);
+ }
}
if (sta && elems->wmm_info)
@@ -464,21 +539,17 @@ static void ieee80211_rx_bss_info(struct
ieee80211_sta_join_ibss(sdata, bss);
supp_rates = ieee80211_sta_get_rates(local, elems, band);
ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa,
- supp_rates, GFP_KERNEL);
+ supp_rates);
+ rcu_read_unlock();
}
put_bss:
ieee80211_rx_bss_put(local, bss);
}
-/*
- * Add a new IBSS station, will also be called by the RX code when,
- * in IBSS mode, receiving a frame from a yet-unknown station, hence
- * must be callable in atomic context.
- */
-struct sta_info *ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
- u8 *bssid, u8 *addr, u32 supp_rates,
- gfp_t gfp)
+void ieee80211_ibss_rx_no_sta(struct ieee80211_sub_if_data *sdata,
+ const u8 *bssid, const u8 *addr,
+ u32 supp_rates)
{
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
struct ieee80211_local *local = sdata->local;
@@ -493,40 +564,29 @@ struct sta_info *ieee80211_ibss_add_sta(
if (net_ratelimit())
printk(KERN_DEBUG "%s: No room for a new IBSS STA entry %pM\n",
sdata->name, addr);
- return NULL;
+ return;
}
if (ifibss->state == IEEE80211_IBSS_MLME_SEARCH)
- return NULL;
+ return;
if (compare_ether_addr(bssid, sdata->u.ibss.bssid))
- return NULL;
-
-#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
- wiphy_debug(local->hw.wiphy, "Adding new IBSS station %pM (dev=%s)\n",
- addr, sdata->name);
-#endif
+ return;
- sta = sta_info_alloc(sdata, addr, gfp);
+ sta = sta_info_alloc(sdata, addr, GFP_ATOMIC);
if (!sta)
- return NULL;
+ return;
sta->last_rx = jiffies;
- sta_info_move_state(sta, IEEE80211_STA_AUTH);
- sta_info_move_state(sta, IEEE80211_STA_ASSOC);
- sta_info_move_state(sta, IEEE80211_STA_AUTHORIZED);
-
/* make sure mandatory rates are always added */
sta->sta.supp_rates[band] = supp_rates |
ieee80211_mandatory_rates(local, band);
- rate_control_rate_init(sta);
-
- /* If it fails, maybe we raced another insertion? */
- if (sta_info_insert(sta))
- return sta_info_get(sdata, addr);
- return sta;
+ spin_lock(&ifibss->incomplete_lock);
+ list_add(&sta->list, &ifibss->incomplete_stations);
+ spin_unlock(&ifibss->incomplete_lock);
+ ieee80211_queue_work(&local->hw, &sdata->work);
}
static int ieee80211_sta_active_ibss(struct ieee80211_sub_if_data *sdata)
@@ -865,6 +925,7 @@ void ieee80211_ibss_rx_queued_mgmt(struc
void ieee80211_ibss_work(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
+ struct sta_info *sta;
mutex_lock(&ifibss->mtx);
@@ -876,6 +937,19 @@ void ieee80211_ibss_work(struct ieee8021
if (!ifibss->ssid_len)
goto out;
+ spin_lock_bh(&ifibss->incomplete_lock);
+ while (!list_empty(&ifibss->incomplete_stations)) {
+ sta = list_first_entry(&ifibss->incomplete_stations,
+ struct sta_info, list);
+ list_del(&sta->list);
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
+ ieee80211_ibss_finish_sta(sta);
+ rcu_read_unlock();
+ spin_lock_bh(&ifibss->incomplete_lock);
+ }
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
switch (ifibss->state) {
case IEEE80211_IBSS_MLME_SEARCH:
ieee80211_sta_find_ibss(sdata);
@@ -934,6 +1008,8 @@ void ieee80211_ibss_setup_sdata(struct i
setup_timer(&ifibss->timer, ieee80211_ibss_timer,
(unsigned long) sdata);
mutex_init(&ifibss->mtx);
+ INIT_LIST_HEAD(&ifibss->incomplete_stations);
+ spin_lock_init(&ifibss->incomplete_lock);
}
/* scan finished notification */
@@ -1053,6 +1129,7 @@ int ieee80211_ibss_leave(struct ieee8021
struct cfg80211_bss *cbss;
u16 capability;
int active_ibss;
+ struct sta_info *sta;
mutex_lock(&sdata->u.ibss.mtx);
@@ -1081,6 +1158,19 @@ int ieee80211_ibss_leave(struct ieee8021
}
sta_info_flush(sdata->local, sdata);
+
+ spin_lock_bh(&ifibss->incomplete_lock);
+ while (!list_empty(&ifibss->incomplete_stations)) {
+ sta = list_first_entry(&ifibss->incomplete_stations,
+ struct sta_info, list);
+ list_del(&sta->list);
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
+ sta_info_free(local, sta);
+ spin_lock_bh(&ifibss->incomplete_lock);
+ }
+ spin_unlock_bh(&ifibss->incomplete_lock);
+
netif_carrier_off(sdata->dev);
/* remove beacon */
--- a/net/mac80211/ieee80211_i.h 2011-12-15 10:27:47.000000000 +0100
+++ b/net/mac80211/ieee80211_i.h 2011-12-15 11:15:19.000000000 +0100
@@ -482,6 +482,9 @@ struct ieee80211_if_ibss {
struct sk_buff __rcu *presp;
struct sk_buff *skb;
+ spinlock_t incomplete_lock;
+ struct list_head incomplete_stations;
+
enum {
IEEE80211_IBSS_MLME_SEARCH,
IEEE80211_IBSS_MLME_JOINED,
@@ -1172,9 +1175,8 @@ void ieee80211_sta_reset_conn_monitor(st
/* IBSS code */
void ieee80211_ibss_notify_scan_completed(struct ieee80211_local *local);
void ieee80211_ibss_setup_sdata(struct ieee80211_sub_if_data *sdata);
-struct sta_info *ieee80211_ibss_add_sta(struct ieee80211_sub_if_data *sdata,
- u8 *bssid, u8 *addr, u32 supp_rates,
- gfp_t gfp);
+void ieee80211_ibss_rx_no_sta(struct ieee80211_sub_if_data *sdata,
+ const u8 *bssid, const u8 *addr, u32 supp_rates);
int ieee80211_ibss_join(struct ieee80211_sub_if_data *sdata,
struct cfg80211_ibss_params *params);
int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata);
--- a/net/mac80211/rx.c 2011-12-15 09:31:31.000000000 +0100
+++ b/net/mac80211/rx.c 2011-12-15 10:27:48.000000000 +0100
@@ -2775,8 +2775,8 @@ static int prepare_for_handlers(struct i
rate_idx = 0; /* TODO: HT rates */
else
rate_idx = status->rate_idx;
- rx->sta = ieee80211_ibss_add_sta(sdata, bssid,
- hdr->addr2, BIT(rate_idx), GFP_ATOMIC);
+ ieee80211_ibss_rx_no_sta(sdata, bssid, hdr->addr2,
+ BIT(rate_idx));
}
break;
case NL80211_IFTYPE_MESH_POINT:
--- a/net/mac80211/sta_info.c 2011-12-15 10:27:47.000000000 +0100
+++ b/net/mac80211/sta_info.c 2011-12-15 11:15:31.000000000 +0100
@@ -354,35 +354,26 @@ static int sta_info_finish_insert(struct
/* notify driver */
err = drv_sta_add(local, sdata, &sta->sta);
if (err) {
- if (!async)
+ if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
return err;
printk(KERN_DEBUG "%s: failed to add IBSS STA %pM to "
"driver (%d) - keeping it anyway.\n",
sdata->name, sta->sta.addr, err);
- } else {
+ } else
sta->uploaded = true;
-#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
- if (async)
- wiphy_debug(local->hw.wiphy,
- "Finished adding IBSS STA %pM\n",
- sta->sta.addr);
-#endif
- }
sdata = sta->sdata;
}
if (!dummy_reinsert) {
- if (!async) {
- local->num_sta++;
- local->sta_generation++;
- smp_mb();
-
- /* make the station visible */
- spin_lock_irqsave(&local->sta_lock, flags);
- sta_info_hash_add(local, sta);
- spin_unlock_irqrestore(&local->sta_lock, flags);
- }
+ local->num_sta++;
+ local->sta_generation++;
+ smp_mb();
+
+ /* make the station visible */
+ spin_lock_irqsave(&local->sta_lock, flags);
+ sta_info_hash_add(local, sta);
+ spin_unlock_irqrestore(&local->sta_lock, flags);
list_add(&sta->list, &local->sta_list);
} else {
@@ -1546,7 +1537,7 @@ EXPORT_SYMBOL(ieee80211_sta_set_buffered
int sta_info_move_state_checked(struct sta_info *sta,
enum ieee80211_sta_state new_state)
{
- /* might_sleep(); -- for driver notify later, fix IBSS first */
+ might_sleep();
if (sta->sta_state == new_state)
return 0;
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] mac80211: reduce station management complexity
2011-12-14 13:03 ` [PATCH 2/2] mac80211: reduce station management complexity Johannes Berg
@ 2011-12-15 10:24 ` Johannes Berg
0 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2011-12-15 10:24 UTC (permalink / raw)
To: John Linville; +Cc: linux-wireless
From: Johannes Berg <johannes.berg@intel.com>
Now that IBSS no longer needs to insert stations
from atomic context, we can get rid of all the
special cases for that, and even get rid of the
sta_lock (though it needs to stay as tim_lock.)
This makes the station management code much more
straight-forward.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/mac80211/ieee80211_i.h | 11 -
net/mac80211/sta_info.c | 251 ++++++++++-----------------------------------
net/mac80211/tx.c | 4
3 files changed, 63 insertions(+), 203 deletions(-)
--- a/net/mac80211/sta_info.c 2011-12-15 11:16:35.000000000 +0100
+++ b/net/mac80211/sta_info.c 2011-12-15 11:22:52.000000000 +0100
@@ -62,14 +62,14 @@
* freed before they are done using it.
*/
-/* Caller must hold local->sta_lock */
+/* Caller must hold local->sta_mtx */
static int sta_info_hash_del(struct ieee80211_local *local,
struct sta_info *sta)
{
struct sta_info *s;
s = rcu_dereference_protected(local->sta_hash[STA_HASH(sta->sta.addr)],
- lockdep_is_held(&local->sta_lock));
+ lockdep_is_held(&local->sta_mtx));
if (!s)
return -ENOENT;
if (s == sta) {
@@ -81,7 +81,7 @@ static int sta_info_hash_del(struct ieee
while (rcu_access_pointer(s->hnext) &&
rcu_access_pointer(s->hnext) != sta)
s = rcu_dereference_protected(s->hnext,
- lockdep_is_held(&local->sta_lock));
+ lockdep_is_held(&local->sta_mtx));
if (rcu_access_pointer(s->hnext)) {
RCU_INIT_POINTER(s->hnext, sta->hnext);
return 0;
@@ -98,14 +98,12 @@ struct sta_info *sta_info_get(struct iee
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if (sta->sdata == sdata && !sta->dummy &&
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -119,14 +117,12 @@ struct sta_info *sta_info_get_rx(struct
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if (sta->sdata == sdata &&
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -143,7 +139,6 @@ struct sta_info *sta_info_get_bss(struct
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if ((sta->sdata == sdata ||
@@ -152,7 +147,6 @@ struct sta_info *sta_info_get_bss(struct
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -169,7 +163,6 @@ struct sta_info *sta_info_get_bss_rx(str
struct sta_info *sta;
sta = rcu_dereference_check(local->sta_hash[STA_HASH(addr)],
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
while (sta) {
if ((sta->sdata == sdata ||
@@ -177,7 +170,6 @@ struct sta_info *sta_info_get_bss_rx(str
memcmp(sta->sta.addr, addr, ETH_ALEN) == 0)
break;
sta = rcu_dereference_check(sta->hnext,
- lockdep_is_held(&local->sta_lock) ||
lockdep_is_held(&local->sta_mtx));
}
return sta;
@@ -228,10 +220,11 @@ void sta_info_free(struct ieee80211_loca
kfree(sta);
}
-/* Caller must hold local->sta_lock */
+/* Caller must hold local->sta_mtx */
static void sta_info_hash_add(struct ieee80211_local *local,
struct sta_info *sta)
{
+ lockdep_assert_held(&local->sta_mtx);
sta->hnext = local->sta_hash[STA_HASH(sta->sta.addr)];
RCU_INIT_POINTER(local->sta_hash[STA_HASH(sta->sta.addr)], sta);
}
@@ -339,89 +332,6 @@ struct sta_info *sta_info_alloc(struct i
return sta;
}
-static int sta_info_finish_insert(struct sta_info *sta,
- bool async, bool dummy_reinsert)
-{
- struct ieee80211_local *local = sta->local;
- struct ieee80211_sub_if_data *sdata = sta->sdata;
- struct station_info sinfo;
- unsigned long flags;
- int err = 0;
-
- lockdep_assert_held(&local->sta_mtx);
-
- if (!sta->dummy || dummy_reinsert) {
- /* notify driver */
- err = drv_sta_add(local, sdata, &sta->sta);
- if (err) {
- if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
- return err;
- printk(KERN_DEBUG "%s: failed to add IBSS STA %pM to "
- "driver (%d) - keeping it anyway.\n",
- sdata->name, sta->sta.addr, err);
- } else
- sta->uploaded = true;
-
- sdata = sta->sdata;
- }
-
- if (!dummy_reinsert) {
- local->num_sta++;
- local->sta_generation++;
- smp_mb();
-
- /* make the station visible */
- spin_lock_irqsave(&local->sta_lock, flags);
- sta_info_hash_add(local, sta);
- spin_unlock_irqrestore(&local->sta_lock, flags);
-
- list_add(&sta->list, &local->sta_list);
- } else {
- sta->dummy = false;
- }
-
- if (!sta->dummy) {
- ieee80211_sta_debugfs_add(sta);
- rate_control_add_sta_debugfs(sta);
-
- memset(&sinfo, 0, sizeof(sinfo));
- sinfo.filled = 0;
- sinfo.generation = local->sta_generation;
- cfg80211_new_sta(sdata->dev, sta->sta.addr, &sinfo, GFP_KERNEL);
- }
-
- return 0;
-}
-
-static void sta_info_finish_pending(struct ieee80211_local *local)
-{
- struct sta_info *sta;
- unsigned long flags;
-
- spin_lock_irqsave(&local->sta_lock, flags);
- while (!list_empty(&local->sta_pending_list)) {
- sta = list_first_entry(&local->sta_pending_list,
- struct sta_info, list);
- list_del(&sta->list);
- spin_unlock_irqrestore(&local->sta_lock, flags);
-
- sta_info_finish_insert(sta, true, false);
-
- spin_lock_irqsave(&local->sta_lock, flags);
- }
- spin_unlock_irqrestore(&local->sta_lock, flags);
-}
-
-static void sta_info_finish_work(struct work_struct *work)
-{
- struct ieee80211_local *local =
- container_of(work, struct ieee80211_local, sta_finish_work);
-
- mutex_lock(&local->sta_mtx);
- sta_info_finish_pending(local);
- mutex_unlock(&local->sta_mtx);
-}
-
static int sta_info_insert_check(struct sta_info *sta)
{
struct ieee80211_sub_if_data *sdata = sta->sdata;
@@ -441,50 +351,15 @@ static int sta_info_insert_check(struct
return 0;
}
-static int sta_info_insert_ibss(struct sta_info *sta) __acquires(RCU)
-{
- struct ieee80211_local *local = sta->local;
- struct ieee80211_sub_if_data *sdata = sta->sdata;
- unsigned long flags;
-
- spin_lock_irqsave(&local->sta_lock, flags);
- /* check if STA exists already */
- if (sta_info_get_bss_rx(sdata, sta->sta.addr)) {
- spin_unlock_irqrestore(&local->sta_lock, flags);
- rcu_read_lock();
- return -EEXIST;
- }
-
- local->num_sta++;
- local->sta_generation++;
- smp_mb();
- sta_info_hash_add(local, sta);
-
- list_add_tail(&sta->list, &local->sta_pending_list);
-
- rcu_read_lock();
- spin_unlock_irqrestore(&local->sta_lock, flags);
-
-#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
- wiphy_debug(local->hw.wiphy, "Added IBSS STA %pM\n",
- sta->sta.addr);
-#endif /* CONFIG_MAC80211_VERBOSE_DEBUG */
-
- ieee80211_queue_work(&local->hw, &local->sta_finish_work);
-
- return 0;
-}
-
/*
* should be called with sta_mtx locked
* this function replaces the mutex lock
* with a RCU lock
*/
-static int sta_info_insert_non_ibss(struct sta_info *sta) __acquires(RCU)
+static int sta_info_insert_finish(struct sta_info *sta) __acquires(RCU)
{
struct ieee80211_local *local = sta->local;
struct ieee80211_sub_if_data *sdata = sta->sdata;
- unsigned long flags;
struct sta_info *exist_sta;
bool dummy_reinsert = false;
int err = 0;
@@ -492,19 +367,8 @@ static int sta_info_insert_non_ibss(stru
lockdep_assert_held(&local->sta_mtx);
/*
- * On first glance, this will look racy, because the code
- * in this function, which inserts a station with sleeping,
- * unlocks the sta_lock between checking existence in the
- * hash table and inserting into it.
- *
- * However, it is not racy against itself because it keeps
- * the mutex locked.
- */
-
- spin_lock_irqsave(&local->sta_lock, flags);
- /*
* check if STA exists already.
- * only accept a scenario of a second call to sta_info_insert_non_ibss
+ * only accept a scenario of a second call to sta_info_insert_finish
* with a dummy station entry that was inserted earlier
* in that case - assume that the dummy station flag should
* be removed.
@@ -514,20 +378,47 @@ static int sta_info_insert_non_ibss(stru
if (exist_sta == sta && sta->dummy) {
dummy_reinsert = true;
} else {
- spin_unlock_irqrestore(&local->sta_lock, flags);
- mutex_unlock(&local->sta_mtx);
- rcu_read_lock();
- return -EEXIST;
+ err = -EEXIST;
+ goto out_err;
}
}
- spin_unlock_irqrestore(&local->sta_lock, flags);
+ if (!sta->dummy || dummy_reinsert) {
+ /* notify driver */
+ err = drv_sta_add(local, sdata, &sta->sta);
+ if (err) {
+ if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
+ goto out_err;
+ printk(KERN_DEBUG "%s: failed to add IBSS STA %pM to "
+ "driver (%d) - keeping it anyway.\n",
+ sdata->name, sta->sta.addr, err);
+ } else
+ sta->uploaded = true;
+ }
- err = sta_info_finish_insert(sta, false, dummy_reinsert);
- if (err) {
- mutex_unlock(&local->sta_mtx);
- rcu_read_lock();
- return err;
+ if (!dummy_reinsert) {
+ local->num_sta++;
+ local->sta_generation++;
+ smp_mb();
+
+ /* make the station visible */
+ sta_info_hash_add(local, sta);
+
+ list_add(&sta->list, &local->sta_list);
+ } else {
+ sta->dummy = false;
+ }
+
+ if (!sta->dummy) {
+ struct station_info sinfo;
+
+ ieee80211_sta_debugfs_add(sta);
+ rate_control_add_sta_debugfs(sta);
+
+ memset(&sinfo, 0, sizeof(sinfo));
+ sinfo.filled = 0;
+ sinfo.generation = local->sta_generation;
+ cfg80211_new_sta(sdata->dev, sta->sta.addr, &sinfo, GFP_KERNEL);
}
#ifdef CONFIG_MAC80211_VERBOSE_DEBUG
@@ -543,47 +434,28 @@ static int sta_info_insert_non_ibss(stru
mesh_accept_plinks_update(sdata);
return 0;
+ out_err:
+ mutex_unlock(&local->sta_mtx);
+ rcu_read_lock();
+ return err;
}
int sta_info_insert_rcu(struct sta_info *sta) __acquires(RCU)
{
struct ieee80211_local *local = sta->local;
- struct ieee80211_sub_if_data *sdata = sta->sdata;
int err = 0;
+ might_sleep();
+
err = sta_info_insert_check(sta);
if (err) {
rcu_read_lock();
goto out_free;
}
- /*
- * In ad-hoc mode, we sometimes need to insert stations
- * from tasklet context from the RX path. To avoid races,
- * always do so in that case -- see the comment below.
- */
- if (sdata->vif.type == NL80211_IFTYPE_ADHOC) {
- err = sta_info_insert_ibss(sta);
- if (err)
- goto out_free;
-
- return 0;
- }
-
- /*
- * It might seem that the function called below is in race against
- * the function call above that atomically inserts the station... That,
- * however, is not true because the above code can only
- * be invoked for IBSS interfaces, and the below code will
- * not be -- and the two do not race against each other as
- * the hash table also keys off the interface.
- */
-
- might_sleep();
-
mutex_lock(&local->sta_mtx);
- err = sta_info_insert_non_ibss(sta);
+ err = sta_info_insert_finish(sta);
if (err)
goto out_free;
@@ -617,7 +489,7 @@ int sta_info_reinsert(struct sta_info *s
might_sleep();
- err = sta_info_insert_non_ibss(sta);
+ err = sta_info_insert_finish(sta);
rcu_read_unlock();
return err;
}
@@ -704,7 +576,7 @@ void sta_info_recalc_tim(struct sta_info
}
done:
- spin_lock_irqsave(&local->sta_lock, flags);
+ spin_lock_irqsave(&local->tim_lock, flags);
if (indicate_tim)
__bss_tim_set(bss, sta->sta.aid);
@@ -717,7 +589,7 @@ void sta_info_recalc_tim(struct sta_info
local->tim_in_locked_section = false;
}
- spin_unlock_irqrestore(&local->sta_lock, flags);
+ spin_unlock_irqrestore(&local->tim_lock, flags);
}
static bool sta_info_buffer_expired(struct sta_info *sta, struct sk_buff *skb)
@@ -841,7 +713,6 @@ static int __must_check __sta_info_destr
{
struct ieee80211_local *local;
struct ieee80211_sub_if_data *sdata;
- unsigned long flags;
int ret, i, ac;
struct tid_ampdu_tx *tid_tx;
@@ -862,15 +733,12 @@ static int __must_check __sta_info_destr
set_sta_flag(sta, WLAN_STA_BLOCK_BA);
ieee80211_sta_tear_down_BA_sessions(sta, true);
- spin_lock_irqsave(&local->sta_lock, flags);
ret = sta_info_hash_del(local, sta);
- /* this might still be the pending list ... which is fine */
- if (!ret)
- list_del(&sta->list);
- spin_unlock_irqrestore(&local->sta_lock, flags);
if (ret)
return ret;
+ list_del(&sta->list);
+
mutex_lock(&local->key_mtx);
for (i = 0; i < NUM_DEFAULT_KEYS; i++)
__ieee80211_key_free(key_mtx_dereference(local, sta->gtk[i]));
@@ -1025,11 +893,9 @@ static void sta_info_cleanup(unsigned lo
void sta_info_init(struct ieee80211_local *local)
{
- spin_lock_init(&local->sta_lock);
+ spin_lock_init(&local->tim_lock);
mutex_init(&local->sta_mtx);
INIT_LIST_HEAD(&local->sta_list);
- INIT_LIST_HEAD(&local->sta_pending_list);
- INIT_WORK(&local->sta_finish_work, sta_info_finish_work);
setup_timer(&local->sta_cleanup, sta_info_cleanup,
(unsigned long)local);
@@ -1058,9 +924,6 @@ int sta_info_flush(struct ieee80211_loca
might_sleep();
mutex_lock(&local->sta_mtx);
-
- sta_info_finish_pending(local);
-
list_for_each_entry_safe(sta, tmp, &local->sta_list, list) {
if (!sdata || sdata == sta->sdata)
WARN_ON(__sta_info_destroy(sta));
--- a/net/mac80211/ieee80211_i.h 2011-12-15 11:16:35.000000000 +0100
+++ b/net/mac80211/ieee80211_i.h 2011-12-15 11:17:21.000000000 +0100
@@ -855,18 +855,15 @@ struct ieee80211_local {
/* Station data */
/*
- * The mutex only protects the list and counter,
- * reads are done in RCU.
- * Additionally, the lock protects the hash table,
- * the pending list and each BSS's TIM bitmap.
+ * The mutex only protects the list, hash table and
+ * counter, reads are done with RCU.
*/
struct mutex sta_mtx;
- spinlock_t sta_lock;
+ spinlock_t tim_lock;
unsigned long num_sta;
- struct list_head sta_list, sta_pending_list;
+ struct list_head sta_list;
struct sta_info __rcu *sta_hash[STA_HASH_SIZE];
struct timer_list sta_cleanup;
- struct work_struct sta_finish_work;
int sta_generation;
struct sk_buff_head pending[IEEE80211_MAX_QUEUES];
--- a/net/mac80211/tx.c 2011-12-15 11:16:25.000000000 +0100
+++ b/net/mac80211/tx.c 2011-12-15 11:17:21.000000000 +0100
@@ -2333,9 +2333,9 @@ struct sk_buff *ieee80211_beacon_get_tim
} else {
unsigned long flags;
- spin_lock_irqsave(&local->sta_lock, flags);
+ spin_lock_irqsave(&local->tim_lock, flags);
ieee80211_beacon_add_tim(ap, skb, beacon);
- spin_unlock_irqrestore(&local->sta_lock, flags);
+ spin_unlock_irqrestore(&local->tim_lock, flags);
}
if (tim_offset)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-12-15 10:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-14 13:03 [PATCH 0/2] station management complexity reduction Johannes Berg
2011-12-14 13:03 ` [PATCH 1/2] mac80211: delay IBSS station insertion Johannes Berg
2011-12-15 10:17 ` [PATCH v2 " Johannes Berg
2011-12-14 13:03 ` [PATCH 2/2] mac80211: reduce station management complexity Johannes Berg
2011-12-15 10:24 ` [PATCH v2 " Johannes Berg
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).