[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110705220644.GB12118@hmsreliant.think-freely.org>
Date: Tue, 5 Jul 2011 18:06:44 -0400
From: Neil Horman <nhorman@...driver.com>
To: Eric Dumazet <eric.dumazet@...il.com>
Cc: Alexey Zaytsev <alexey.zaytsev@...il.com>,
Michael Büsch <m@...s.ch>,
Andrew Morton <akpm@...ux-foundation.org>,
netdev@...r.kernel.org, Gary Zambrano <zambrano@...adcom.com>,
bugme-daemon@...zilla.kernel.org,
"David S. Miller" <davem@...emloft.net>,
Pekka Pietikainen <pp@...oulu.fi>,
Florian Schirmer <jolt@...box.org>,
Felix Fietkau <nbd@...nwrt.org>, Michael Buesch <mb@...sch.de>
Subject: Re: [Bugme-new] [Bug 38102] New: BUG kmalloc-2048: Poison overwritten
On Tue, Jul 05, 2011 at 10:15:40PM +0200, Eric Dumazet wrote:
> Le mardi 05 juillet 2011 à 22:02 +0200, Eric Dumazet a écrit :
> > Le mardi 05 juillet 2011 à 15:53 -0400, Neil Horman a écrit :
> > > I think this is a goo idea, at least for testing. It seems odd to me that we
> > > have the B44_DMARX_PTR value which indicates (ostensibly) the pointer to the
> > > descriptor to be processed next (the documentation isnt' very verbose on the
> > > subject), along with the EOT bit on a descriptor. It seems like both the
> > > register and the bit are capable of conveying the same (or at least overlapping)
> > > information.
> > >
> > > I think what I'm having the most trouble with is understanding when the hw looks
> > > at the EOT bit in the descriptor. If it completes a DMA and sees the EOT bit
> > > set, does the next DMA occur to the descriptor pointed to by the DMARX_ADDR
> > > register? Of does it stall until such time as the DMARX_PTR register is rotated
> > > around? What if it doesn't see the EOT bit set? Does it just keep going with
> > > the next descriptor?
>
> Since there is no OWN bit (at least not on the online doc I got : it
> says the rx_ring is read only by the NIC), I would say we really need to
> advance DMARX_PTR to signal NIC a new entry is available for following
> incoming frames.
>
> This is the reason rx_pending max value is B44_RX_RING_SIZE - 1, or else
> chip could loop on a circular rx_ring.
>
Agree, although that still leaves open the question of what exactly should get
written into the DMARX_PTR. Is it an index of the descriptor number, or a byte
offset.
Regardless, I think we ned to fix up the looping so as to prevent an EOT reset
jumping outside of our valid ring window. Alexey, theres better ways to do
this, but if in the interim, you could please try this patch, it makes the valid
receive window for b44 cover the entire ring, so as to avoid this problem. It
will at least help support or refute this theory. Note its not exactly the same
as my previous patch. If you set the default ring pending to 512, the math in
the b44_alloc_rx_skb path is wrong, we skip the EOT bit as well as the first
entry in the ring. At 511 it should work out properly.
Thanks
Neil
diff --git a/drivers/net/b44.c b/drivers/net/b44.c
index 3d247f3..b7f5ed1 100644
--- a/drivers/net/b44.c
+++ b/drivers/net/b44.c
@@ -57,7 +57,7 @@
#define B44_MAX_MTU 1500
#define B44_RX_RING_SIZE 512
-#define B44_DEF_RX_RING_PENDING 200
+#define B44_DEF_RX_RING_PENDING 511
#define B44_RX_RING_BYTES (sizeof(struct dma_desc) * \
B44_RX_RING_SIZE)
#define B44_TX_RING_SIZE 512
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists