On Wed, May 26, 2010 at 9:15 PM, Bruno Randolf <b...@einfach.org> wrote:
> i don't know. i got this from a review against the HAL sources, which has this
> check. what i can imagine is that RXDP is not pointing at the bf at the moment
> we read it, but while we process the packet in SW the RXDP is changed to the
> bf and it writes into it's skb. if that happens after/while we read the skb we
> might get corruption. if that happens after we freed the skb, we might get the
> poison overwritten... i'm not sure...

Well the invariant is supposed to be that we are always behind where the
hardware is.  If that holds, the hardware can't loop around and write into
the buffer we are currently processing, because we'd have to add the buffer
back to the descriptor list first.  The RXDP check is supposed to ensure
that the hardware has loaded the next descriptor before we do anything with
the current one (we already "know" it is done with DMAing the SKB due to the
status register read).

That said... reset() rebuilds the buffer list and resets the sclink pointers
etc.  I'm not so sure it does it properly, though last time I reviewed it
I didn't find anything.

> [this is from ar5212_recv.c ar5212ProcRxDesc()]
>
> "ands" is the next descriptor. so if the next descriptor is NOT done and the
> RXDP points at the current descriptor, return.

Yeah I've read it before, it doesn't make sense to me either.  I think it
does make a certain amount of sense to look at RXDP for the current buffer
subject to the earlier caveats.  Of course, we could try adopting all of
the paranoia of the HAL and see if it cures all ailments and then narrow
down what really matters.

>> Also, what happens if only one packet is received?
>
> if only one packet is received we should have more descriptors after the
> current packet, so this applies only when the RX buffers get used up and we
> are at the last, self-linked descriptor.

Of course, you are right.  I was confusing the TX case again.

> again, i don't know, but i got it from a review against the HAL sources. can
> someone with more knowledge about DMA comment?

At least the code for alpha -- I looked at the first no-op implementation :)
has this comment:

    After this call, reads by the cpu to the buffer are guaranteed to see
    whatever the device wrote there.

The text in Documentation/ seems to agree.  The case where this isn't true
are streaming mappings where you want to read part of the buffer on the CPU
while it's mapped for the device, but I don't think we do that.  All the
status reads are through registers, and the descriptors are consistent
mappings.  But, as I noted in that bug report, we are missing store barriers
before updates to the link fields.

-- 
Bob Copeland %% www.bobcopeland.com
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel

Reply via email to