Re: [TIP] BUG kmalloc-4096: Poison overwritten (ath5k_rx_skb_alloc)

[Bob Copeland]

On Sun, Mar 08, 2009 at 10:28:23AM +0100, Jiri Slaby wrote:
>> If you want I'll cook up a patch for that too.
>
> If you like, feel free to kick it off. Remember to remove bf->flags  
> completely, so that we save another bunch of memory ;).

Ok, here's a patch, it survived a few hours of testing with 3 rx buffers.
Sitsofe, it's worth a try applying this one, even though it probably
isn't it :)

From: Bob Copeland <me@xxxxxxxxxxxxxxx>
Date: Mon, 9 Mar 2009 11:29:34 -0400
Subject: [PATCH] ath5k: use rx hw descriptor pointer for self-linked check

This patch simplifies the code used to detect when the
self-linked DMA buffer is still in use by hardware, by
checking the hardware's rxdp register instead of looking
at the software buffer list.

Signed-off-by: Bob Copeland <me@xxxxxxxxxxxxxxx>
---
 drivers/net/wireless/ath5k/base.c |   24 ++++--------------------
 drivers/net/wireless/ath5k/base.h |    1 -
 drivers/net/wireless/ath5k/dma.c  |    2 --
 3 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 1d77ee9..a4e385b 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1677,7 +1677,7 @@ ath5k_tasklet_rx(unsigned long data)
 	struct sk_buff *skb, *next_skb;
 	dma_addr_t next_skb_addr;
 	struct ath5k_softc *sc = (void *)data;
-	struct ath5k_buf *bf, *bf_last;
+	struct ath5k_buf *bf;
 	struct ath5k_desc *ds;
 	int ret;
 	int hdrlen;
@@ -1688,7 +1688,6 @@ ath5k_tasklet_rx(unsigned long data)
 		ATH5K_WARN(sc, "empty rx buf pool\n");
 		goto unlock;
 	}
-	bf_last = list_entry(sc->rxbuf.prev, struct ath5k_buf, list);
 	do {
 		rxs.flag = 0;
 
@@ -1697,24 +1696,9 @@ ath5k_tasklet_rx(unsigned long data)
 		skb = bf->skb;
 		ds = bf->desc;
 
-		/*
-		 * last buffer must not be freed to ensure proper hardware
-		 * function. When the hardware finishes also a packet next to
-		 * it, we are sure, it doesn't use it anymore and we can go on.
-		 */
-		if (bf_last == bf)
-			bf->flags |= 1;
-		if (bf->flags) {
-			struct ath5k_buf *bf_next = list_entry(bf->list.next,
-					struct ath5k_buf, list);
-			ret = sc->ah->ah_proc_rx_desc(sc->ah, bf_next->desc,
-					&rs);
-			if (ret)
-				break;
-			bf->flags &= ~1;
-			/* skip the overwritten one (even status is martian) */
-			goto next;
-		}
+		/* bail if HW is still using self-linked descriptor */
+		if (ath5k_hw_get_rxdp(sc->ah) == bf->daddr)
+			break;
 
 		ret = sc->ah->ah_proc_rx_desc(sc->ah, ds, &rs);
 		if (unlikely(ret == -EINPROGRESS))
diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h
index facc60d..ed407cc 100644
--- a/drivers/net/wireless/ath5k/base.h
+++ b/drivers/net/wireless/ath5k/base.h
@@ -56,7 +56,6 @@
 
 struct ath5k_buf {
 	struct list_head	list;
-	unsigned int		flags;	/* rx descriptor flags */
 	struct ath5k_desc	*desc;	/* virtual addr of desc */
 	dma_addr_t		daddr;	/* physical addr of desc */
 	struct sk_buff		*skb;	/* skbuff for buf */
diff --git a/drivers/net/wireless/ath5k/dma.c b/drivers/net/wireless/ath5k/dma.c
index b65b4fe..941b511 100644
--- a/drivers/net/wireless/ath5k/dma.c
+++ b/drivers/net/wireless/ath5k/dma.c
@@ -80,8 +80,6 @@ int ath5k_hw_stop_rx_dma(struct ath5k_hw *ah)
  * ath5k_hw_get_rxdp - Get RX Descriptor's address
  *
  * @ah: The &struct ath5k_hw
- *
- * XXX: Is RXDP read and clear ?
  */
 u32 ath5k_hw_get_rxdp(struct ath5k_hw *ah)
 {
-- 
1.6.0.6


-- 
Bob Copeland %% www.bobcopeland.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/