Race condition in i.MX tty serial driver

cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 

Race condition in i.MX tty serial driver

Jump to solution
978 Views
michaeldoswald
Contributor III

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?


Regards,

Michael

Original Attachment has been moved to: 0002-Fixed-race-condition-which-caused-imx-uart-to-transf.patch.zip

0 Kudos
1 Solution
673 Views
fabio_estevam
NXP Employee
NXP Employee

Hi Michael,

Can you try and generate a patch against 3.19-rc4?

The instructions for submitting patches for mainline inclusion is at Documentation/SubmittingPatches .

You should use your real name in the From field and you also should add a Signed-off-by tag.

You also need to include a commit log that provides as much of details about the bug you see and why your patch fixes it.

Regards,

Fabio Estevam

View solution in original post

0 Kudos
2 Replies
674 Views
fabio_estevam
NXP Employee
NXP Employee

Hi Michael,

Can you try and generate a patch against 3.19-rc4?

The instructions for submitting patches for mainline inclusion is at Documentation/SubmittingPatches .

You should use your real name in the From field and you also should add a Signed-off-by tag.

You also need to include a commit log that provides as much of details about the bug you see and why your patch fixes it.

Regards,

Fabio Estevam

0 Kudos
673 Views
igorpadykov
NXP Employee
NXP Employee

Hi Michael

I think you can look below

FSL Community BSP Release Notes 1.7 documentation

Best regards

igor

0 Kudos