AnsweredAssumed Answered

Race condition in i.MX tty serial driver

Question asked by Michael Doswald on Jan 13, 2015

Hello i.MX Community,


In the last few days I've tried to track down a bug in my software. It seemed that the data I send via UART are in some cases repeatetly sent. After code review and debugging I've figured that this must be a problem within the i.MX serial driver and not in my application code. And indeed, there seems to be a race condition in the current implementation of drivers/tty/serial/imx.c (kernel 3.8.2):

In the method imx_dma_tx, the first lines check if there is a DMA transfer currently running. If so, the method just returns without setting up a new DMA transfer. If no transfer is running, a new DMA transfer is set up using the ports xmit buffer. The problem seems to be, that the check (using dmaengine_tx_status) does indicate that no transfer is running even if the callback method of the DMA transfer is not yet (fully) executed. This leads to a condition where the DMA transfer is done, the callback is not yet executed and therefore the tail pointer of the xmit buffer is not yet updated. The imx_dma_tx will then initiate a new DMA transfer using the tail pointer value and therefore transfer the data twice.

I've created a patch for kernel version 3.8.2 that, instead of using the dmaengine_tx_status, uses the dma_is_txing flag in the imx_port structure (which is already there and it seems that it was planned to use that flag for exactly that purpose). Also the flag is cleared now at the end of the dma callback function, instead of before the code which updates the tail pointer, and I've widened the critical section in the callback method to include the uart_write_wakeup call (don't know if that is necessary, but other drivers, e.g. serial-tegra.c do it the same way).

Could somebody tell me if my patch seems to be correct? What would be the correct steps to propose such a patch to be included into the Linux kernel?



Original Attachment has been moved to: