| Message ID | 1371068409-10407-1-git-send-email-greearb@candelatech.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, 2013-06-12 at 13:20 -0700, greearb@candelatech.com wrote: > I believe this is more correct, though it did not fix the > memory leak I was chasing when I found this code. That description could use some work :-) > + spin_lock_bh(&sta->lock); > + > tid_tx = sta->ampdu_mlme.tid_start_tx[tid]; > if (tid_tx) { > /* > * Assign it over to the normal tid_tx array > * where it "goes live". > */ > - spin_lock_bh(&sta->lock); > > sta->ampdu_mlme.tid_start_tx[tid] = NULL; > /* could there be a race? */ > @@ -301,6 +302,8 @@ void ieee80211_ba_session_work(struct work_struct *work) > > ieee80211_tx_ba_session_handle_start(sta, tid); > continue; > + } else { > + spin_unlock_bh(&sta->lock); > } > You could just put the unlock after the if block, given the continue in it, I think I'd prefer that. > tid_tx = rcu_dereference_protected_tid_tx(sta, tid); > diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h > index c509423..0f85418 100644 > --- a/net/mac80211/sta_info.h > +++ b/net/mac80211/sta_info.h > @@ -204,6 +204,7 @@ struct tid_ampdu_rx { > * driver requested to close until the work for it runs > * @mtx: mutex to protect all TX data (except non-NULL assignments > * to tid_tx[idx], which are protected by the sta spinlock) > + * tid_start_tx is also protected by sta->lock. That should be a tab. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/ht.c b/net/mac80211/ht.c index 0db25d4..c6256b4 100644 --- a/net/mac80211/ht.c +++ b/net/mac80211/ht.c @@ -283,13 +283,14 @@ void ieee80211_ba_session_work(struct work_struct *work) sta, tid, WLAN_BACK_RECIPIENT, WLAN_REASON_UNSPECIFIED, true); + spin_lock_bh(&sta->lock); + tid_tx = sta->ampdu_mlme.tid_start_tx[tid]; if (tid_tx) { /* * Assign it over to the normal tid_tx array * where it "goes live". */ - spin_lock_bh(&sta->lock); sta->ampdu_mlme.tid_start_tx[tid] = NULL; /* could there be a race? */ @@ -301,6 +302,8 @@ void ieee80211_ba_session_work(struct work_struct *work) ieee80211_tx_ba_session_handle_start(sta, tid); continue; + } else { + spin_unlock_bh(&sta->lock); } tid_tx = rcu_dereference_protected_tid_tx(sta, tid); diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h index c509423..0f85418 100644 --- a/net/mac80211/sta_info.h +++ b/net/mac80211/sta_info.h @@ -204,6 +204,7 @@ struct tid_ampdu_rx { * driver requested to close until the work for it runs * @mtx: mutex to protect all TX data (except non-NULL assignments * to tid_tx[idx], which are protected by the sta spinlock) + * tid_start_tx is also protected by sta->lock. */ struct sta_ampdu_mlme { struct mutex mtx;