On 08/03/2016 01:19 PM, Lars-Peter Clausen wrote: > On 08/03/2016 12:59 PM, Fabien Lahoudere wrote: >> From: Hannu Koivisto <hannu.koivisto@xxxxxxxxx> >> >> dma_rx_callback() may see NULL dma_chan_rx if DMA interrupt [1] occurs a >> moment[2] before imx_uart_dma_exit() sets it to NULL. imx_uart_dma_exit() >> calls dmaengine_terminate_all() and dma_release_channel() but neither of >> those prevent the callback being called after they have returned. A similar >> problem has been discussed by ALSA developers >> (http://mailman.alsa-project.org/pipermail/alsa-devel/2013-October/067239.html) >> and it was pointed out that dmaengine_terminate_all() might be called from >> the callback, so we cannot call tasklet_kill() in imx-sdma's code called by >> dmaengine_terminate_all(). >> >> Hopefully it doesn't make sense to call dma_release_channel() from the >> callback, so instead of adding synchronization to imx serial driver, we add >> tasklet_kill() call to sdma_free_chan_resources(). While most DMA drivers >> don't do that, there is one example that does: pl330. >> >> [1] It schedules sdma_tasklet, which again calls the dma_rx_callback. >> [2] I tested this by scheduling the sdma tasklet as far as right before the >> imx_stop_tx() call in imx_shutdown() and the problem occurred. >> >> Signed-off-by: Fabien Lahoudere <fabien.lahoudere@xxxxxxxxxxxxxxx> > > I'd prefer that the driver implements the new synchronization API[1]. This > is a more generic approach and covers of all cases of this race condition. > > If the synchronize() callback is implemented the core will automatically > make sure that the channel is synchronized when it is freed. Looking at the driver it also seems that just calling tasklet_kill() is not enough. The tasklet_schedule() in the sdma_int_handler() is not synchronized to anything. So if the interrupt triggers just at the right time it might re-schedule the tasklet after tasklet_kill() has been called. Especially if the tasklet_kill() runs on a different CPU. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
