2013-08-10 13:59:26

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 1/2] ath9k: fix rx descriptor related race condition

Similar to a race condition that exists in the tx path, the hardware
might re-read the 'next' pointer of a descriptor of the last completed
frame. This only affects non-EDMA (pre-AR93xx) devices.

To deal with this race, defer clearing and re-linking a completed rx
descriptor until the next one has been processed.

Cc: [email protected]
Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 5 +----
drivers/net/wireless/ath/ath9k/recv.c | 17 +++++++++++++----
2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 505c615..0d69b13 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -79,10 +79,6 @@ struct ath_config {
sizeof(struct ath_buf_state)); \
} while (0)

-#define ATH_RXBUF_RESET(_bf) do { \
- (_bf)->bf_stale = false; \
- } while (0)
-
/**
* enum buffer_type - Buffer type flags
*
@@ -315,6 +311,7 @@ struct ath_rx {
struct ath_descdma rxdma;
struct ath_rx_edma rx_edma[ATH9K_RX_QUEUE_MAX];

+ struct ath_buf *buf_hold;
struct sk_buff *frag;

u32 ampdu_ref;
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 62dff97..2dd851a 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -42,8 +42,6 @@ static void ath_rx_buf_link(struct ath_softc *sc, struct ath_buf *bf)
struct ath_desc *ds;
struct sk_buff *skb;

- ATH_RXBUF_RESET(bf);
-
ds = bf->bf_desc;
ds->ds_link = 0; /* link to null */
ds->ds_data = bf->bf_buf_addr;
@@ -70,6 +68,14 @@ static void ath_rx_buf_link(struct ath_softc *sc, struct ath_buf *bf)
sc->rx.rxlink = &ds->ds_link;
}

+static void ath_rx_buf_relink(struct ath_softc *sc, struct ath_buf *bf)
+{
+ if (sc->rx.buf_hold)
+ ath_rx_buf_link(sc, sc->rx.buf_hold);
+
+ sc->rx.buf_hold = bf;
+}
+
static void ath_setdefantenna(struct ath_softc *sc, u32 antenna)
{
/* XXX block beacon interrupts */
@@ -117,7 +123,6 @@ static bool ath_rx_edma_buf_link(struct ath_softc *sc,

skb = bf->bf_mpdu;

- ATH_RXBUF_RESET(bf);
memset(skb->data, 0, ah->caps.rx_status_len);
dma_sync_single_for_device(sc->dev, bf->bf_buf_addr,
ah->caps.rx_status_len, DMA_TO_DEVICE);
@@ -432,6 +437,7 @@ int ath_startrecv(struct ath_softc *sc)
if (list_empty(&sc->rx.rxbuf))
goto start_recv;

+ sc->rx.buf_hold = NULL;
sc->rx.rxlink = NULL;
list_for_each_entry_safe(bf, tbf, &sc->rx.rxbuf, list) {
ath_rx_buf_link(sc, bf);
@@ -677,6 +683,9 @@ static struct ath_buf *ath_get_next_rx_buf(struct ath_softc *sc,
}

bf = list_first_entry(&sc->rx.rxbuf, struct ath_buf, list);
+ if (bf == sc->rx.buf_hold)
+ return NULL;
+
ds = bf->bf_desc;

/*
@@ -1387,7 +1396,7 @@ requeue:
if (edma) {
ath_rx_edma_buf_link(sc, qtype);
} else {
- ath_rx_buf_link(sc, bf);
+ ath_rx_buf_relink(sc, bf);
ath9k_hw_rxena(ah);
}
} while (1);
--
1.8.0.2



2013-08-10 13:59:27

by Felix Fietkau

[permalink] [raw]
Subject: [PATCH 2/2] ath9k: shrink a few data structures by reordering fields

Also reduce the size of a few fields where possible

Signed-off-by: Felix Fietkau <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 12 ++++++------
drivers/net/wireless/ath/ath9k/xmit.c | 16 ++++++++--------
2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 0d69b13..7b1d036 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -72,7 +72,6 @@ struct ath_config {
/*************************/

#define ATH_TXBUF_RESET(_bf) do { \
- (_bf)->bf_stale = false; \
(_bf)->bf_lastbf = NULL; \
(_bf)->bf_next = NULL; \
memset(&((_bf)->bf_state), 0, \
@@ -192,10 +191,10 @@ struct ath_txq {

struct ath_atx_ac {
struct ath_txq *txq;
- int sched;
struct list_head list;
struct list_head tid_q;
bool clear_ps_filter;
+ bool sched;
};

struct ath_frame_info {
@@ -212,6 +211,7 @@ struct ath_buf_state {
u8 bf_type;
u8 bfs_paprd;
u8 ndelim;
+ bool stale;
u16 seqno;
unsigned long bfs_paprd_timestamp;
};
@@ -225,7 +225,6 @@ struct ath_buf {
void *bf_desc; /* virtual addr of desc */
dma_addr_t bf_daddr; /* physical addr of desc */
dma_addr_t bf_buf_addr; /* physical addr of data buffer, for DMA */
- bool bf_stale;
struct ieee80211_tx_rate rates[4];
struct ath_buf_state bf_state;
};
@@ -237,13 +236,14 @@ struct ath_atx_tid {
struct ath_node *an;
struct ath_atx_ac *ac;
unsigned long tx_buf[BITS_TO_LONGS(ATH_TID_MAX_BUFS)];
- int bar_index;
u16 seq_start;
u16 seq_next;
u16 baw_size;
- int tidno;
+ u8 tidno;
int baw_head; /* first un-acked tx buffer */
int baw_tail; /* next unused tx buffer slot */
+
+ s8 bar_index;
bool sched;
bool paused;
bool active;
@@ -255,10 +255,10 @@ struct ath_node {
struct ieee80211_vif *vif; /* interface with which we're associated */
struct ath_atx_tid tid[IEEE80211_NUM_TIDS];
struct ath_atx_ac ac[IEEE80211_NUM_ACS];
- int ps_key;

u16 maxampdu;
u8 mpdudensity;
+ s8 ps_key;

bool sleeping;
bool no_ps_filter;
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index d8dfb3e..4fc80e3 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -493,7 +493,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
while (bf) {
bf_next = bf->bf_next;

- if (!bf->bf_stale || bf_next != NULL)
+ if (!bf->bf_state.stale || bf_next != NULL)
list_move_tail(&bf->list, &bf_head);

ath_tx_complete_buf(sc, bf, txq, &bf_head, ts, 0);
@@ -586,7 +586,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
* not a holding desc.
*/
INIT_LIST_HEAD(&bf_head);
- if (bf_next != NULL || !bf_last->bf_stale)
+ if (bf_next != NULL || !bf_last->bf_state.stale)
list_move_tail(&bf->list, &bf_head);

if (!txpending) {
@@ -610,7 +610,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
ieee80211_sta_eosp(sta);
}
/* retry the un-acked ones */
- if (bf->bf_next == NULL && bf_last->bf_stale) {
+ if (bf->bf_next == NULL && bf_last->bf_state.stale) {
struct ath_buf *tbf;

tbf = ath_clone_txbuf(sc, bf_last);
@@ -1734,7 +1734,7 @@ static void ath_drain_txq_list(struct ath_softc *sc, struct ath_txq *txq,
while (!list_empty(list)) {
bf = list_first_entry(list, struct ath_buf, list);

- if (bf->bf_stale) {
+ if (bf->bf_state.stale) {
list_del(&bf->list);

ath_tx_return_buffer(sc, bf);
@@ -2490,7 +2490,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
* it with the STALE flag.
*/
bf_held = NULL;
- if (bf->bf_stale) {
+ if (bf->bf_state.stale) {
bf_held = bf;
if (list_is_last(&bf_held->list, &txq->axq_q))
break;
@@ -2514,7 +2514,7 @@ static void ath_tx_processq(struct ath_softc *sc, struct ath_txq *txq)
* however leave the last descriptor back as the holding
* descriptor for hw.
*/
- lastbf->bf_stale = true;
+ lastbf->bf_state.stale = true;
INIT_LIST_HEAD(&bf_head);
if (!list_is_singular(&lastbf->list))
list_cut_position(&bf_head,
@@ -2585,7 +2585,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
}

bf = list_first_entry(fifo_list, struct ath_buf, list);
- if (bf->bf_stale) {
+ if (bf->bf_state.stale) {
list_del(&bf->list);
ath_tx_return_buffer(sc, bf);
bf = list_first_entry(fifo_list, struct ath_buf, list);
@@ -2607,7 +2607,7 @@ void ath_tx_edma_tasklet(struct ath_softc *sc)
ath_tx_txqaddbuf(sc, txq, &bf_q, true);
}
} else {
- lastbf->bf_stale = true;
+ lastbf->bf_state.stale = true;
if (bf != lastbf)
list_cut_position(&bf_head, fifo_list,
lastbf->list.prev);
--
1.8.0.2