Here's another iteration. I think this should be safe - if someone
with slow MIPS can bang on it and see if queues hang any more frequently
or the WARN_ON is hit or any kind of lockup, that would be a useful
datapoint.
notes:
- we don't stop hw tx before freeing the skbs in drain txbuffs
(didn't we do that once upon a time?) so we could have transmitted
garbage there.
- stopping all dma is a bit heavy-handed, I just need to set txd
on the queues before draining the bufs. it's probably
a fair bit slower due to calling that twice in reset path and a
third time on every flush() -> will rework
- wmb is probably excessive too.
- found a memory leak in beacon buf dma map failure
- I think the tx_poll_mark should only be reset after we successfully
process a descriptor instead of before...maybe.
- can't figure out why we had skb null check in tx tasklet, comment
doesn't make any sense, so would like to see the WARN_ON when it
triggers.
Will split this up and resend if it gets some positive testing and no
objections..
diff --git a/drivers/net/wireless/ath/ath5k/ath5k.h
b/drivers/net/wireless/ath/ath5k/ath5k.h
index 277d5cb..5104a99 100644
--- a/drivers/net/wireless/ath/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath/ath5k/ath5k.h
@@ -1354,6 +1354,7 @@ int ath5k_beacon_update(struct ieee80211_hw *hw, struct
ieee80211_vif *vif);
void ath5k_beacon_config(struct ath5k_hw *ah);
void ath5k_txbuf_free_skb(struct ath5k_hw *ah, struct ath5k_buf *bf);
void ath5k_rxbuf_free_skb(struct ath5k_hw *ah, struct ath5k_buf *bf);
+void ath5k_flush_tx(struct ath5k_hw *ah, bool drop);
/*Chip id helper functions */
const char *ath5k_chip_name(enum ath5k_srev_type type, u_int16_t val);
diff --git a/drivers/net/wireless/ath/ath5k/base.c
b/drivers/net/wireless/ath/ath5k/base.c
index a74d286..73492ee 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -87,6 +87,8 @@ MODULE_LICENSE("Dual BSD/GPL");
static int ath5k_init(struct ieee80211_hw *hw);
static int ath5k_reset(struct ath5k_hw *ah, struct ieee80211_channel *chan,
bool skip_pcu);
+static int
+ath5k_rx_start(struct ath5k_hw *ah);
/* Known SREVs */
static const struct ath5k_srev_name srev_names[] = {
@@ -748,6 +750,11 @@ ath5k_txbuf_setup(struct ath5k_hw *ah, struct ath5k_buf
*bf,
spin_lock_bh(&txq->lock);
list_add_tail(&bf->list, &txq->q);
txq->txq_len++;
+
+ /*
+ * make sure ds_link update is seen before txq->link write...
+ */
+ wmb();
if (txq->link == NULL) /* is this first packet? */
ath5k_hw_set_txdp(ah, txq->qnum, bf->daddr);
else /* no, so only link it */
@@ -1020,16 +1027,28 @@ err:
return ret;
}
+static bool
+ath5k_has_pending_frames(struct ath5k_hw *ah, struct ath5k_txq *txq)
+{
+ bool pending;
+
+ spin_lock_bh(&txq->lock);
+ pending = txq->setup && txq->txq_len > 0;
+ spin_unlock_bh(&txq->lock);
+
+ return pending;
+}
+
/**
* ath5k_drain_tx_buffs - Empty tx buffers
*
* @ah The &struct ath5k_hw
*
* Empty tx buffers from all queues in preparation
- * of a reset or during shutdown.
+ * of a reset or during shutdown. We take txq->lock
+ * to avoid races against ath5k_tx_processq().
*
- * NB: this assumes output has been stopped and
- * we do not need to block ath5k_tx_tasklet
+ * Hardware TX should be disabled.
*/
static void
ath5k_drain_tx_buffs(struct ath5k_hw *ah)
@@ -1060,6 +1079,40 @@ ath5k_drain_tx_buffs(struct ath5k_hw *ah)
}
}
+/**
+ * ath5k_flush_tx - wait for all pending tx to be processed
+ *
+ * @ah: The &struct ath5k_hw
+ * @drop: true if we should drop packets rather than wait
+ *
+ * Queuing of new packets should be stopped. In any case we
+ * don't wait more than a small timeout (200 ms) for packets.
+ */
+void ath5k_flush_tx(struct ath5k_hw *ah, bool drop)
+{
+ int i;
+ bool pending = true;
+ unsigned long timeout = jiffies + msecs_to_jiffies(200);
+
+ if (drop)
+ pending = false;
+
+ /* sleep until some timeout occurs or all queues are empty */
+ while (pending && !time_after(jiffies, timeout))
+ {
+ pending = false;
+ for (i = 0; i < ARRAY_SIZE(ah->txqs); i++)
+ pending |= ath5k_has_pending_frames(ah, &ah->txqs[i]);
+
+ if (pending)
+ msleep(10);
+ }
+ ath5k_hw_dma_stop(ah);
+ ath5k_rx_start(ah);
+ ath5k_drain_tx_buffs(ah);
+}
+
+
static void
ath5k_txq_release(struct ath5k_hw *ah)
{
@@ -1650,43 +1703,41 @@ ath5k_tx_processq(struct ath5k_hw *ah, struct ath5k_txq
*txq)
spin_lock(&txq->lock);
list_for_each_entry_safe(bf, bf0, &txq->q, list) {
- txq->txq_poll_mark = false;
-
- /* skb might already have been processed last time. */
- if (bf->skb != NULL) {
- ds = bf->desc;
-
- ret = ah->ah_proc_tx_desc(ah, ds, &ts);
- if (unlikely(ret == -EINPROGRESS))
- break;
- else if (unlikely(ret)) {
- ATH5K_ERR(ah,
- "error %d while processing "
- "queue %u\n", ret, txq->qnum);
- break;
- }
+ WARN_ON(!bf->skb);
- skb = bf->skb;
- bf->skb = NULL;
+ ds = bf->desc;
- dma_unmap_single(ah->dev, bf->skbaddr, skb->len,
- DMA_TO_DEVICE);
- ath5k_tx_frame_completed(ah, skb, txq, &ts);
+ ret = ah->ah_proc_tx_desc(ah, ds, &ts);
+ if (unlikely(ret == -EINPROGRESS))
+ break;
+ else if (unlikely(ret)) {
+ ATH5K_ERR(ah,
+ "error %d while processing "
+ "queue %u\n", ret, txq->qnum);
+ break;
}
+ txq->txq_poll_mark = false;
- /*
- * It's possible that the hardware can say the buffer is
- * completed when it hasn't yet loaded the ds_link from
- * host memory and moved on.
- * Always keep the last descriptor to avoid HW races...
+ /* if there's no race here, should not cause issues.
+ * if there is, queue should hang regularly since
+ * driver thinks there are packets queued and writes
+ * to wrong txq->link while hw is re-reading this
+ * descriptor for link updates.
*/
- if (ath5k_hw_get_txdp(ah, txq->qnum) != bf->daddr) {
- spin_lock(&ah->txbuflock);
- list_move_tail(&bf->list, &ah->txbuf);
- ah->txbuf_len++;
- txq->txq_len--;
- spin_unlock(&ah->txbuflock);
- }
+ ds->ds_link = 0;
+
+ skb = bf->skb;
+ bf->skb = NULL;
+
+ dma_unmap_single(ah->dev, bf->skbaddr, skb->len,
+ DMA_TO_DEVICE);
+ ath5k_tx_frame_completed(ah, skb, txq, &ts);
+
+ spin_lock(&ah->txbuflock);
+ list_move_tail(&bf->list, &ah->txbuf);
+ ah->txbuf_len++;
+ txq->txq_len--;
+ spin_unlock(&ah->txbuflock);
}
spin_unlock(&txq->lock);
if (txq->txq_len < ATH5K_TXQ_LEN_LOW && txq->qnum < 4)
@@ -1818,8 +1869,6 @@ ath5k_beacon_update(struct ieee80211_hw *hw, struct
ieee80211_vif *vif)
ath5k_txbuf_free_skb(ah, avf->bbuf);
avf->bbuf->skb = skb;
ret = ath5k_beacon_setup(ah, avf->bbuf);
- if (ret)
- avf->bbuf->skb = NULL;
out:
return ret;
}
@@ -2308,7 +2357,7 @@ ath5k_tx_complete_poll_work(struct work_struct *work)
if (ah->txqs[i].setup) {
txq = &ah->txqs[i];
spin_lock_bh(&txq->lock);
- if (txq->txq_len > 1) {
+ if (txq->txq_len) {
if (txq->txq_poll_mark) {
ATH5K_DBG(ah, ATH5K_DEBUG_XMIT,
"TX queue stuck %d\n",
@@ -2664,6 +2713,7 @@ ath5k_reset(struct ath5k_hw *ah, struct ieee80211_channel
*chan,
ath5k_hw_set_imr(ah, 0);
synchronize_irq(ah->irq);
+ ath5k_hw_dma_stop(ah);
ath5k_stop_tasklets(ah);
/* Save ani mode and disable ANI during
diff --git a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
index 2a715ca..2fc09bc 100644
--- a/drivers/net/wireless/ath/ath5k/mac80211-ops.c
+++ b/drivers/net/wireless/ath/ath5k/mac80211-ops.c
@@ -693,6 +693,16 @@ ath5k_set_coverage_class(struct ieee80211_hw *hw, u8
coverage_class)
mutex_unlock(&ah->lock);
}
+static void
+ath5k_flush(struct ieee80211_hw *hw, bool drop)
+{
+ struct ath5k_hw *ah = hw->priv;
+
+ mutex_lock(&ah->lock);
+ ath5k_flush_tx(ah, drop);
+ mutex_unlock(&ah->lock);
+}
+
static int
ath5k_set_antenna(struct ieee80211_hw *hw, u32 tx_ant, u32 rx_ant)
@@ -802,7 +812,7 @@ const struct ieee80211_ops ath5k_hw_ops = {
.get_survey = ath5k_get_survey,
.set_coverage_class = ath5k_set_coverage_class,
/* .rfkill_poll = not implemented */
- /* .flush = not implemented */
+ .flush = ath5k_flush,
/* .channel_switch = not implemented */
/* .napi_poll = not implemented */
.set_antenna = ath5k_set_antenna,
--
Bob Copeland %% www.bobcopeland.com
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel
