Re: [PATCH v3 0/6] add virt-dma support for imx-sdma

[Robin Gong]

Hi Lucas,
	Is the below DEAD LOCK issue same as your side? If yes, then
I'm afraid that we have to make another patch for uart to split dma
functions in uart driver out of the code area which protected by
port.lock. The warning make sense since allocate sdma bd memory
dynamically in virt-dma instead of static allocated as before. I'll
make another uart patch into my next version patchset.


[ÂÂÂ46.155406] =====================================================
[ÂÂÂ46.161503] WARNING: HARDIRQ-safe -> HARDIRQ-unsafe lock order
detected
[ÂÂÂ46.168122] 4.17.0-rc6-00008-g7caafa3-dirty #48 Not tainted
[ÂÂÂ46.173696] -----------------------------------------------------
[ÂÂÂ46.179795] mxc_uart_stress/419 [HC0[0]:SC0[0]:HE0:SE1] is trying to
acquire:
[ÂÂÂ46.186934] fa7c1440 (fs_reclaim){+.+.}, at:
fs_reclaim_acquire.part.3+0x0/0x48
[ÂÂÂ46.194270]
[ÂÂÂ46.194270] and this task is already holding:
[ÂÂÂ46.200106] 09a17fda (&port_lock_key){-.-.}, at:
uart_write+0x84/0x190
[ÂÂÂ46.206658] which would create a new lock dependency:
[ÂÂÂ46.211710]ÂÂ(&port_lock_key){-.-.} -> (fs_reclaim){+.+.}
[ÂÂÂ46.217132]
[ÂÂÂ46.217132] but this new dependency connects a HARDIRQ-irq-safe
lock:
[ÂÂÂ46.225051]ÂÂ(&port_lock_key){-.-.}
[ÂÂÂ46.225062]
[ÂÂÂ46.225062] ... which became HARDIRQ-irq-safe at:
[ÂÂÂ46.234740]ÂÂÂlock_acquire+0x70/0x90
[ÂÂÂ46.238326]ÂÂÂ_raw_spin_lock_irqsave+0x40/0x54
[ÂÂÂ46.242777]ÂÂÂimx_uart_console_write+0x1bc/0x1e0
[ÂÂÂ46.247402]ÂÂÂconsole_unlock+0x320/0x5f0
[ÂÂÂ46.251329]ÂÂÂvprintk_emit+0x22c/0x3fc
[ÂÂÂ46.255082]ÂÂÂvprintk_default+0x28/0x30
[ÂÂÂ46.258923]ÂÂÂvprintk_func+0x78/0xcc
[ÂÂÂ46.262503]ÂÂÂprintk+0x34/0x54
[ÂÂÂ46.265566]ÂÂÂcrng_fast_load+0xf8/0x138
[ÂÂÂ46.269407]ÂÂÂadd_interrupt_randomness+0x21c/0x24c
[ÂÂÂ46.274204]ÂÂÂhandle_irq_event_percpu+0x40/0x84
[ÂÂÂ46.278739]ÂÂÂhandle_irq_event+0x40/0x64
[ÂÂÂ46.282667]ÂÂÂhandle_fasteoi_irq+0xbc/0x178
[ÂÂÂ46.286854]ÂÂÂgeneric_handle_irq+0x28/0x3c
[ÂÂÂ46.290954]ÂÂÂ__handle_domain_irq+0x6c/0xe8
[ÂÂÂ46.295148]ÂÂÂgic_handle_irq+0x64/0xc4
[ÂÂÂ46.298904]ÂÂÂ__irq_svc+0x70/0x98
[ÂÂÂ46.302225]ÂÂÂ_raw_spin_unlock_irq+0x30/0x34
[ÂÂÂ46.306505]ÂÂÂfinish_task_switch+0xc0/0x27c
[ÂÂÂ46.310693]ÂÂÂ__schedule+0x2c0/0x79c
[ÂÂÂ46.314272]ÂÂÂschedule_idle+0x40/0x84
[ÂÂÂ46.317941]ÂÂÂdo_idle+0x178/0x2b4
[ÂÂÂ46.321259]ÂÂÂcpu_startup_entry+0x20/0x24
[ÂÂÂ46.325278]ÂÂÂrest_init+0x214/0x264
[ÂÂÂ46.328775]ÂÂÂstart_kernel+0x39c/0x424
[ÂÂÂ46.332527]ÂÂÂÂÂ(null)
[ÂÂÂ46.334891]
[ÂÂÂ46.334891] to a HARDIRQ-irq-unsafe lock:
[ÂÂÂ46.340379]ÂÂ(fs_reclaim){+.+.}
[ÂÂÂ46.340391]
[ÂÂÂ46.340391] ... which became HARDIRQ-irq-unsafe at:
[ÂÂÂ46.349885] ...
[ÂÂÂ46.349895]ÂÂÂlock_acquire+0x70/0x90
[ÂÂÂ46.355225]ÂÂÂfs_reclaim_acquire.part.3+0x38/0x48
[ÂÂÂ46.359933]ÂÂÂfs_reclaim_acquire+0x1c/0x20
[ÂÂÂ46.364036]ÂÂÂkmem_cache_alloc+0x2c/0x174
[ÂÂÂ46.368051]ÂÂÂalloc_worker.constprop.10+0x1c/0x58
[ÂÂÂ46.372759]ÂÂÂinit_rescuer.part.4+0x18/0xa4
[ÂÂÂ46.376952]ÂÂÂworkqueue_init+0xc0/0x210
[ÂÂÂ46.380793]ÂÂÂkernel_init_freeable+0x58/0x1d8
[ÂÂÂ46.385156]ÂÂÂkernel_init+0x10/0x11c
[ÂÂÂ46.388736]ÂÂÂret_from_fork+0x14/0x20
[ÂÂÂ46.392399]ÂÂÂÂÂ(null)
[ÂÂÂ46.394762]
[ÂÂÂ46.394762] other info that might help us debug this:
[ÂÂÂ46.394762]
[ÂÂÂ46.402769]ÂÂPossible interrupt unsafe locking scenario:
[ÂÂÂ46.402769]
[ÂÂÂ46.409560]ÂÂÂÂÂÂÂÂCPU0ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂCPU1
[ÂÂÂ46.414092]ÂÂÂÂÂÂÂÂ----ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ----
[ÂÂÂ46.418622]ÂÂÂlock(fs_reclaim);
[ÂÂÂ46.421772]ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂlocal_irq_disable();
[ÂÂÂ46.427693]ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂlock(&port_lock_key);
[ÂÂÂ46.433707]ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂlock(fs_reclaim);
[ÂÂÂ46.439372]ÂÂÂ<Interrupt>
[ÂÂÂ46.441993]ÂÂÂÂÂlock(&port_lock_key);
[ÂÂÂ46.445661]
[ÂÂÂ46.445661]ÂÂ*** DEADLOCK ***
[ÂÂÂ46.445661]

On ä, 2018-06-11 at 13:01 +0200, Lucas Stach wrote:
> Hi Robin,
> 
> this series breaks serial DMA for me. I wasn't able to dig in deeper
> yet. Please let me know if you can test/reproduce at your side, if
> not
> I'll try to find some time to collect some more debug info.
> 
> Regards,
> Lucas
> 
> Am Montag, den 11.06.2018, 22:59 +0800 schrieb Robin Gong:
> > 
> > The legacy sdma driver has below limitations or drawbacks:
> > Â 1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and
> > alloc
> > ÂÂÂÂÂone page size for one channel regardless of only few BDs
> > needed
> > ÂÂÂÂÂmost time. But in few cases, the max PAGE_SIZE maybe not
> > enough.
> > Â 2. One SDMA channel can't stop immediatley once channel disabled
> > which
> > ÂÂÂÂÂmeans SDMA interrupt may come in after this channel
> > terminated.There
> > ÂÂÂÂÂare some patches for this corner case such as commit
> > "2746e2c389f9",
> > ÂÂÂÂÂbut not cover non-cyclic.
> > 
> > The common virt-dma overcomes the above limitations. It can alloc
> > bd
> > dynamically and free bd once this tx transfer done. No memory
> > wasted or
> > maximum limititation here, only depends on how many memory can be
> > requested
> > from kernel. For No.2, such issue can be workaround by checking if
> > there
> > is available descript("sdmac->desc") now once the unwanted
> > interrupt
> > coming. At last the common virt-dma is easier for sdma driver
> > maintain.
> > 
> > Change from v2:
> > Â 1. include Sascha's patch to make the main patch easier to
> > review.
> > ÂÂÂÂÂThanks Sacha.
> > Â 2. remove useless 'desc'/'chan' in struct sdma_channe.
> > 
> > Change from v1:
> > Â 1. split v1 patch into 5 patches.
> > Â 2. remove some unnecessary condition check.
> > Â 3. remove unnecessary 'pending' list.
> > 
> > Robin Gong (5):
> > Â dmaengine: imx-sdma: add virt-dma support
> > Â Revert "dmaengine: imx-sdma: fix pagefault when channel is
> > disabled
> > ÂÂÂÂduring interrupt"
> > Â dmaengine: imx-sdma: remove usless lock
> > Â dmaengine: imx-sdma: remove the maximum limation for bd numbers
> > Â dmaengine: imx-sdma: add sdma_transfer_init to decrease code
> > overlap
> > 
> > Âdrivers/dma/KconfigÂÂÂÂ|ÂÂÂ1 +
> > Âdrivers/dma/imx-sdma.c | 392 ++++++++++++++++++++++++++++---------
> > ------------
> > Â2 files changed, 227 insertions(+), 166 deletions(-)
> > 
> > --Â
> > 2.7.4
> > 
> > Robin Gong (5):
> > Â dmaengine: imx-sdma: add virt-dma support
> > Â Revert "dmaengine: imx-sdma: fix pagefault when channel is
> > disabled
> > ÂÂÂÂduring interrupt"
> > Â dmaengine: imx-sdma: remove usless lock
> > Â dmaengine: imx-sdma: remove the maximum limation for bd numbers
> > Â dmaengine: imx-sdma: add sdma_transfer_init to decrease code
> > overlap
> > 
> > Sascha Hauer (1):
> > Â dmaengine: imx-sdma: factor out a struct sdma_desc from struct
> > ÂÂÂÂsdma_channel
> > 
> > Âdrivers/dma/KconfigÂÂÂÂ|ÂÂÂ1 +
> > Âdrivers/dma/imx-sdma.c | 391 ++++++++++++++++++++++++++++---------
> > ------------
> > Â2 files changed, 226 insertions(+), 166 deletions(-)
> >