On 09/06/2012 05:49 PM, Havard Skinnemoen : > On Wed, Sep 5, 2012 at 11:30 PM, David Miller <da...@davemloft.net> wrote: >> From: Nicolas Ferre <nicolas.fe...@atmel.com> >> Date: Wed, 5 Sep 2012 10:19:11 +0200 >> >>> From: Havard Skinnemoen <hav...@skinnemoen.net> >>> >>> Fix a race in macb_start_xmit() where we unconditionally set the TSTART bit. >>> If an underrun just happened (we do this with interrupts disabled, so it >>> might >>> not have been handled yet), the controller starts transmitting from the >>> first >>> entry in the ring, which is usually wrong. >>> Restart the controller after error handling. >>> >>> Signed-off-by: Havard Skinnemoen <hav...@skinnemoen.net> >>> [nicolas.fe...@atmel.com: split patch in topics] >>> Signed-off-by: Nicolas Ferre <nicolas.fe...@atmel.com> >> >> Accumulating special case code and checks into the hot path of TX packet >> processing is extremely unwise. >> >> Instead, when you handle the TX error conditions and reset the chip you >> should first ensure that there are no flows of control in the transmit >> function of your driver by using the appropriate locking et al. facilities. > > IIRC, the hardware resets the ring pointers when an error happens, and > if we set TSTART right after that happens, the hardware will happily > transmit whatever is sitting in the beginning of the ring. This is > what I was trying to avoid. > > The details are a bit hazy as it's been a while since I looked at > this, so it could be that simply letting it happen and using a bigger > hammer during reset processing might work just as well. Just want to > make sure y'all understand that we're talking about a race against > hardware, not against interrupt handlers, threads or anything that can > be solved by locking :)
Yes, you are right Havard. I will see if we can let the transmitter go just before being interrupted by the pending error. It is true that there are several cases here: - tx immediately stopped again by the USED bit of a non initialized descriptor. We thus have to cleanup the error frame but also take care about the newly queued packet... - beginning of transmission of a not-related fragment that has just been queued by the start_xmit; just before catching the pending error IRQ. We may have to consider the consequences of this! So, stay tuned ;-) Best regards, -- Nicolas Ferre -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
