[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1309968019.2292.44.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date: Wed, 06 Jul 2011 18:00:19 +0200
From: Eric Dumazet <eric.dumazet@...il.com>
To: Michael Büsch <m@...s.ch>
Cc: Neil Horman <nhorman@...driver.com>,
Alexey Zaytsev <alexey.zaytsev@...il.com>,
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
Le mercredi 06 juillet 2011 à 17:32 +0200, Michael Büsch a écrit :
> You guys are mixing up quite a bit of stuff here...
>
> The EOT bit has _nothing_ to do with the descriptor pointers.
> It simply marks the last descriptor in the (linear) descriptor
> page, so that it becomes an actual ring:
>
> DDDDDDDDDDDDDDDDDDDDDDDDDDDE
> | O
> | T
> ^--------------------------|
>
> It doesn't say anything about the read and write pointers
> to the ring.
>
> The B44_DMARX_PTR is the write-end pointer. It points one entry
> beyond the end of the write area. Then there's the software pointer
> where we keep track of the read position.
>
> -rx_cons DMARX_PTR-
> v v
> DDDDDDDDDDDDDDDDDDDDDDDDDDE
> ^ ^ O
> | | T
> Device might write from
> here to here.
>
> On an RX interrupt (or poll), we read the _actual_ device write
> pointer. (B44_DMARX_STAT & DMARX_STAT_CDMASK). If that is equal
> to our stored rx_cons, the device didn't write anything.
> So we read buffers until we hit the _actual_ device write pointer.
> So rx_cons is equal to (B44_DMARX_STAT & DMARX_STAT_CDMASK), except
> that it lags behind by one IRQ/poll.
> If we read are done, we set the DMARX_PTR write pointer to one desc
> beyond the buffer that we just ate. So the device is free to continue
> writing the ring _up to_ the position we left.
Not exactly :
If we read one skb at descriptor 0, we prepare a new buffer on slot 200,
and advance DMARX_PTR to 201*sizeof(descriptor).
>
> I don't know why b44 sets the DMARX_PTR to 200 initially (which is 40
> descriptors, as this is a byte pointer). This seems kind of arbitrary.
> In b43 we set it to (NR_OF_DESCRIPTORS - 1) * sizeof(descriptor).
> But I don't think it really matters. It only limits the usable DMA
> area before the first interrupt (or poll) occurs. After the final
> B44_DMARX_PTR write in b44_rx(), the full descriptor range (well, minus one)
> will be usable.
Yes, this is probably a small bug, we should fix it for correctness.
>
> Summary: I don't see where the DMA engine code is broken (except for
> the minor missing wmb(), which doesn't trigger this memory corruption, though)
>
> I hope that helps to clear up stuff...
Well, you describe (nicely, thanks !) your understanding of how work the
driver and chip.
Problem is we suspect a wrong statement or wrong hardware ;)
Another problem is Alexey doesnt answer anymore, and I dont have this
(old) hardware...
Other point : Do you know why b44_get_ringparam() doesnt set
ering->tx_max_pending and ering->tx_pending
The comment seems wrong :
/* XXX ethtool lacks a tx_max_pending, oops... */
--
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