* [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 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 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 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).