Re: [Bugme-new] [Bug 38102] New: BUG kmalloc-2048: Poison overwritten

[Eric Dumazet <eric.dumazet@gmail.com>]

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... */