| Message ID | 1377507205-5386-2-git-send-email-michal.kazior@tieto.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Michal Kazior <michal.kazior@tieto.com> writes: > According to mac80211 docs tx and rx reporting > routines must not run concurrently unless they are > _irqsafe variants. > > This doesn't fix any visible bug but is apparently > the right thing to do as per the documentation. > > Signed-off-by: Michal Kazior <michal.kazior@tieto.com> I had a quick chat with Johannes and Sujith about this. The concurrency requirements are for the STA PS race in AP mode (see ieee80211_handle_filtered_frame()). I think we should drop this frame, revisit later and properly analyse how to fix the race. But it would be good add this to the todo list: http://wireless.kernel.org/en/users/Drivers/ath10k#TODO
On 28 August 2013 06:23, Kalle Valo <kvalo@qca.qualcomm.com> wrote: > Michal Kazior <michal.kazior@tieto.com> writes: > >> According to mac80211 docs tx and rx reporting >> routines must not run concurrently unless they are >> _irqsafe variants. >> >> This doesn't fix any visible bug but is apparently >> the right thing to do as per the documentation. >> >> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> > > I had a quick chat with Johannes and Sujith about this. The concurrency > requirements are for the STA PS race in AP mode (see > ieee80211_handle_filtered_frame()). > > I think we should drop this frame, revisit later and properly analyse > how to fix the race. But it would be good add this to the todo list: > > http://wireless.kernel.org/en/users/Drivers/ath10k#TODO I've added this to the TODO list. I'm worried this PS stuff is broken on ath10k anyway because ath10k modifies tx skbuffs for QoS workaround (QoS Control must be removed before submitting to FW). Micha?. -- 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
Michal Kazior <michal.kazior@tieto.com> writes: > On 28 August 2013 06:23, Kalle Valo <kvalo@qca.qualcomm.com> wrote: >> Michal Kazior <michal.kazior@tieto.com> writes: >> >>> According to mac80211 docs tx and rx reporting >>> routines must not run concurrently unless they are >>> _irqsafe variants. >>> >>> This doesn't fix any visible bug but is apparently >>> the right thing to do as per the documentation. >>> >>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com> >> >> I had a quick chat with Johannes and Sujith about this. The concurrency >> requirements are for the STA PS race in AP mode (see >> ieee80211_handle_filtered_frame()). >> >> I think we should drop this frame, revisit later and properly analyse "drop this frame". Heh, of course I was trying to say "drop this patch" :) >> how to fix the race. But it would be good add this to the todo list: >> >> http://wireless.kernel.org/en/users/Drivers/ath10k#TODO > > I've added this to the TODO list. Thanks!
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 04c132e..b43e8ad 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -526,6 +526,7 @@ struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev, mutex_init(&ar->conf_mutex); spin_lock_init(&ar->data_lock); + spin_lock_init(&ar->txrx_lock); INIT_LIST_HEAD(&ar->peers); init_waitqueue_head(&ar->peer_mapping_wq); diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h index ab05c4c..0469a2e 100644 --- a/drivers/net/wireless/ath/ath10k/core.h +++ b/drivers/net/wireless/ath/ath10k/core.h @@ -392,6 +392,9 @@ struct ath10k { u32 survey_last_cycle_count; struct survey_info survey[ATH10K_NUM_CHANS]; + /* synchronized tx/rx reporting to mac80211 */ + spinlock_t txrx_lock; + #ifdef CONFIG_ATH10K_DEBUGFS struct ath10k_debug debug; #endif diff --git a/drivers/net/wireless/ath/ath10k/txrx.c b/drivers/net/wireless/ath/ath10k/txrx.c index 68b6fae..ce7e304 100644 --- a/drivers/net/wireless/ath/ath10k/txrx.c +++ b/drivers/net/wireless/ath/ath10k/txrx.c @@ -88,7 +88,9 @@ void ath10k_txrx_tx_unref(struct ath10k_htt *htt, struct sk_buff *txdesc) if (ATH10K_SKB_CB(txdesc)->htt.no_ack) info->flags &= ~IEEE80211_TX_STAT_ACK; + spin_lock_bh(&htt->ar->txrx_lock); ieee80211_tx_status(htt->ar->hw, msdu); + spin_unlock_bh(&htt->ar->txrx_lock); /* we do not own the msdu anymore */ exit: @@ -294,7 +296,9 @@ void ath10k_process_rx(struct ath10k *ar, struct htt_rx_info *info) status->freq, status->band); + spin_lock_bh(&ar->txrx_lock); ieee80211_rx(ar->hw, info->skb); + spin_unlock_bh(&ar->txrx_lock); } struct ath10k_peer *ath10k_peer_find(struct ath10k *ar, int vdev_id,
According to mac80211 docs tx and rx reporting routines must not run concurrently unless they are _irqsafe variants. This doesn't fix any visible bug but is apparently the right thing to do as per the documentation. Signed-off-by: Michal Kazior <michal.kazior@tieto.com> --- drivers/net/wireless/ath/ath10k/core.c | 1 + drivers/net/wireless/ath/ath10k/core.h | 3 +++ drivers/net/wireless/ath/ath10k/txrx.c | 4 ++++ 3 files changed, 8 insertions(+)