On Wednesday 26 May 2010 21:38:00 Bob Copeland wrote:
> > + /* never process the self-linked entry at the end */
> > + if (ds->ds_link == bf->daddr)
> > + break;
> > +
>
> By this you're saying that the RXDP check just above
> is insufficient?
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...
anyhow the HAl is *very* cautious not to touch the last self-linked
descriptor, which makes sense. it even has a more weird check which i don't
quite understand:
/* Must provide the virtual address of the current
* descriptor, the physical address, and the virtual
* address of the next descriptor in the h/w chain.
* This allows the HAL to look ahead to see if the
* hardware is done with a descriptor by checking the
* done bit in the following descriptor and the address
* of the current descriptor the DMA engine is working
* on. All this is necessary because of our use of
* a self-linked list to avoid rx overruns. */
[this is from if_ath.c ath_intr_process_rx_descriptors()]
/*
* Given the use of a self-linked tail be very sure that the hw is
* done with this descriptor; the hw may have done this descriptor
* once and picked it up again...make sure the hw has moved on.
*/
if ((ands->ds_rxstatus1&AR_Done) == 0 && OS_REG_READ(ah, AR_RXDP) == pa)
return HAL_EINPROGRESS;
[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.
i don't see how it matters if the next descriptor is done or not, and we have
the check for RXDP in ath5k as well. actually, the HAL code would allow us to
continue if the next descriptor IS done, but the RXDP is pointing at the
current descriptor - that doesn't make sense to me. does it make more sense to
anyone else?
> 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.
> > + pci_dma_sync_single_for_cpu(sc->pdev, bf->skbaddr,
> > + rs.rs_datalen,
> > PCI_DMA_FROMDEVICE);
>
> My reading of DMA api howto claims this isn't necessary as
> long as we only touch data after unmapping it.
again, i don't know, but i got it from a review against the HAL sources. can
someone with more knowledge about DMA comment?
bruno
_______________________________________________
ath5k-devel mailing list
ath5k-devel@lists.ath5k.org
https://lists.ath5k.org/mailman/listinfo/ath5k-devel
