Skip to content

Commit

Permalink
iwlwifi: mvm: use pre-RCU-sync sta removal operation
Browse files Browse the repository at this point in the history
iwlmvm relies on the current mac80211 behaviour of allowing
station pointers to be valid for an RCU grace period after
returning from the sta_state() callback. To optimise these
cases, this behaviour is going away, so make the driver use
the new sta_pre_rcu_remove() method to clear the pointer in
the fw_id_to_mac_id[] array.

Since this may happen while the station is still present in
the firmware, don't set the pointer to NULL but to -ENOENT
to mark this particular case. In client mode, the station
is kept even longer (until marking the MAC as unassociated)
so the drain flow must take this new behavior into account.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
  • Loading branch information
Johannes Berg committed Dec 16, 2013
1 parent 6a9d1b9 commit 1ddbbb0
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 7 deletions.
13 changes: 8 additions & 5 deletions drivers/net/wireless/iwlwifi/mvm/debugfs-vif.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,17 @@ static ssize_t iwl_dbgfs_mac_params_read(struct file *file,
if (vif->type == NL80211_IFTYPE_STATION &&
ap_sta_id != IWL_MVM_STATION_COUNT) {
struct ieee80211_sta *sta;
struct iwl_mvm_sta *mvm_sta;

sta = rcu_dereference_protected(mvm->fw_id_to_mac_id[ap_sta_id],
lockdep_is_held(&mvm->mutex));
mvm_sta = (void *)sta->drv_priv;
pos += scnprintf(buf+pos, bufsz-pos,
"ap_sta_id %d - reduced Tx power %d\n",
ap_sta_id, mvm_sta->bt_reduced_txpower);
if (!IS_ERR_OR_NULL(sta)) {
struct iwl_mvm_sta *mvm_sta = (void *)sta->drv_priv;

pos += scnprintf(buf+pos, bufsz-pos,
"ap_sta_id %d - reduced Tx power %d\n",
ap_sta_id,
mvm_sta->bt_reduced_txpower);
}
}

rcu_read_lock();
Expand Down
23 changes: 23 additions & 0 deletions drivers/net/wireless/iwlwifi/mvm/mac80211.c
Original file line number Diff line number Diff line change
Expand Up @@ -1114,6 +1114,28 @@ static void iwl_mvm_mac_sta_notify(struct ieee80211_hw *hw,
}
}

static void iwl_mvm_sta_pre_rcu_remove(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta)
{
struct iwl_mvm *mvm = IWL_MAC80211_GET_MVM(hw);
struct iwl_mvm_sta *mvm_sta = (void *)sta->drv_priv;

/*
* This is called before mac80211 does RCU synchronisation,
* so here we already invalidate our internal RCU-protected
* station pointer. The rest of the code will thus no longer
* be able to find the station this way, and we don't rely
* on further RCU synchronisation after the sta_state()
* callback deleted the station.
*/
mutex_lock(&mvm->mutex);
if (sta == rcu_access_pointer(mvm->fw_id_to_mac_id[mvm_sta->sta_id]))
rcu_assign_pointer(mvm->fw_id_to_mac_id[mvm_sta->sta_id],
ERR_PTR(-ENOENT));
mutex_unlock(&mvm->mutex);
}

static int iwl_mvm_mac_sta_state(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta,
Expand Down Expand Up @@ -1761,6 +1783,7 @@ struct ieee80211_ops iwl_mvm_hw_ops = {
.bss_info_changed = iwl_mvm_bss_info_changed,
.hw_scan = iwl_mvm_mac_hw_scan,
.cancel_hw_scan = iwl_mvm_mac_cancel_hw_scan,
.sta_pre_rcu_remove = iwl_mvm_sta_pre_rcu_remove,
.sta_state = iwl_mvm_mac_sta_state,
.sta_notify = iwl_mvm_mac_sta_notify,
.allow_buffered_frames = iwl_mvm_mac_allow_buffered_frames,
Expand Down
11 changes: 9 additions & 2 deletions drivers/net/wireless/iwlwifi/mvm/sta.c
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,15 @@ void iwl_mvm_sta_drained_wk(struct work_struct *wk)
rcu_dereference_protected(mvm->fw_id_to_mac_id[sta_id],
lockdep_is_held(&mvm->mutex));

/* This station is in use */
if (!IS_ERR(sta))
/*
* This station is in use or RCU-removed; the latter happens in
* managed mode, where mac80211 removes the station before we
* can remove it from firmware (we can only do that after the
* MAC is marked unassociated), and possibly while the deauth
* frame to disconnect from the AP is still queued. Then, the
* station pointer is -ENOENT when the last skb is reclaimed.
*/
if (!IS_ERR(sta) || PTR_ERR(sta) == -ENOENT)
continue;

if (PTR_ERR(sta) == -EINVAL) {
Expand Down

0 comments on commit 1ddbbb0

Please sign in to comment.