From: Johannes Berg <johannes@sipsolutions.net>
To: John Linville <linville@tuxdriver.com>
Cc: linux-wireless@vger.kernel.org
Subject: [PATCH 2/2] mac80211: reduce station management complexity
Date: Wed, 14 Dec 2011 14:03:40 +0100 [thread overview]
Message-ID: <20111214130549.188381247@sipsolutions.net> (raw)
In-Reply-To: 20111214130338.347106528@sipsolutions.net
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)
next prev parent reply other threads:[~2011-12-14 13:08 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Johannes Berg [this message]
2011-12-15 10:24 ` [PATCH v2 2/2] mac80211: reduce station management complexity Johannes Berg
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=20111214130549.188381247@sipsolutions.net \
--to=johannes@sipsolutions.net \
--cc=linux-wireless@vger.kernel.org \
--cc=linville@tuxdriver.com \
/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.