On 16/01/2020 20:10, Dmitry Osipenko wrote:

15.01.2020 12:00, Jon Hunter пишет:

quoted

On 14/01/2020 20:33, Dmitry Osipenko wrote:

quoted

14.01.2020 18:09, Jon Hunter пишет:

quoted

On 12/01/2020 17:29, Dmitry Osipenko wrote:

quoted

I was doing some experiments with I2C and noticed that Tegra APB DMA
driver crashes sometime after I2C DMA transfer termination. The crash
happens because tegra_dma_terminate_all() bails out immediately if pending
list is empty, thus it doesn't release the half-completed descriptors
which are getting re-used before ISR tasklet kicks-in.

Can you elaborate a bit more on how these are getting re-used? What is
the sequence of events which results in the panic? I believe that this
was also reported in the past [0] and so I don't doubt there is an issue
here, but would like to completely understand this.

Thanks!
Jon

[0] https://lore.kernel.org/patchwork/patch/675349/

In my case it happens in the touchscreen driver during of the
touchscreen's interrupt handling (in a threaded IRQ handler) + CPU is
under load and there is other interrupts activity. So what happens here
is that the TS driver issues one I2C transfer, which fails with
(apparently bogus) timeout (because DMA descriptor is completed and
removed from the pending list, but tasklet not executed yet), and then
TS immediately issues another I2C transfer that re-uses the
yet-incompleted descriptor. That's my understanding.

OK, but what is the exact sequence that it allowing it to re-use the
incompleted descriptor?

   TDMA driver                      DMA Client

1.
                                    dmaengine_prep()

2.
   tegra_dma_desc_get()
   dma_desc = kzalloc()
   ...
   tegra_dma_prep_slave_sg()
   INIT_LIST_HEAD(&dma_desc->tx_list);
   INIT_LIST_HEAD(&dma_desc->cb_node);
   list_add_tail(sgreq->node,
                 dma_desc->tx_list)

3.
                                    dma_async_issue_pending()

4.
   tegra_dma_tx_submit()
   list_splice_tail_init(dma_desc->tx_list,
                         tdc->pending_sg_req)

5.
   tegra_dma_isr()
   ...
   handle_once_dma_done()
   ...
   sgreq = list_first_entry(tdc->pending_sg_req)
   list_del(sgreq->node);
   ...
   list_add_tail(dma_desc->cb_node,
                 tdc->cb_desc);
   list_add_tail(dma_desc->node,
                 tdc->free_dma_desc);

Isn't this the problem here, that we have placed this on the free list
before we are actually done?

It seems to me that there could still be a potential race condition
between the ISR and the tasklet running.

Jon

-- 
nvpublic