lkml.org 
[lkml]   [2018]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v4 0/7] add virt-dma support for imx-sdma
    From
    Date
    Hi Robin,

    I just gave this series a spin and it seems there is even more locking
    fun, see the lockdep output below. After taking a short look it seems
    this is caused by using the wrong spinlock variants in
    sdma_int_handler(), those should also use the _irqsave ones. When
    fixing this you might want to reconsider patch 7/7, as it's probably
    not needed at all.

    Regards,
    Lucas

    [ 20.479401] ========================================================
    [ 20.485769] WARNING: possible irq lock inversion dependency detected
    [ 20.492140] 4.17.0+ #1538 Not tainted
    [ 20.495814] --------------------------------------------------------
    [ 20.502181] systemd/1 just changed the state of lock:
    [ 20.507247] c0abdafc (&(&substream->self_group.lock)->rlock){-...}, at: snd_pcm_stream_lock+0x54/0x60
    [ 20.516523] but this lock took another, HARDIRQ-unsafe lock in the past:
    [ 20.523234] (fs_reclaim){+.+.}
    [ 20.523253]
    [ 20.523253]
    [ 20.523253] and interrupts could create inverse lock ordering between them.
    [ 20.523253]
    [ 20.537804]
    [ 20.537804] other info that might help us debug this:
    [ 20.544344] Possible interrupt unsafe locking scenario:
    [ 20.544344]
    [ 20.551144] CPU0 CPU1
    [ 20.555686] ---- ----
    [ 20.560224] lock(fs_reclaim);
    [ 20.563386] local_irq_disable();
    [ 20.569315] lock(&(&substream->self_group.lock)->rlock);
    [ 20.577337] lock(fs_reclaim);
    [ 20.583014] <Interrupt>
    [ 20.585643] lock(&(&substream->self_group.lock)->rlock);
    [ 20.591322]
    [ 20.591322] *** DEADLOCK ***
    [ 20.591322]
    [ 20.597260] 1 lock held by systemd/1:
    [ 20.607806] #0: ab23d11c (snd_pcm_link_rwlock){.-..}, at: snd_pcm_stream_lock+0x4c/0x60
    [ 20.615951]
    [ 20.615951] the shortest dependencies between 2nd lock and 1st lock:
    [ 20.623799] -> (fs_reclaim){+.+.} ops: 248474 {
    [ 20.628456] HARDIRQ-ON-W at:
    [ 20.631716] lock_acquire+0x260/0x29c
    [ 20.637223] fs_reclaim_acquire+0x48/0x58
    [ 20.643075] kmem_cache_alloc_trace+0x3c/0x364
    [ 20.649366] alloc_worker.constprop.15+0x28/0x64
    [ 20.655824] init_rescuer.part.5+0x20/0xa4
    [ 20.661764] workqueue_init+0x124/0x1f8
    [ 20.667446] kernel_init_freeable+0x60/0x550
    [ 20.673561] kernel_init+0x18/0x120
    [ 20.678890] ret_from_fork+0x14/0x20
    [ 20.684299] (null)
    [ 20.688408] SOFTIRQ-ON-W at:
    [ 20.691659] lock_acquire+0x260/0x29c
    [ 20.697158] fs_reclaim_acquire+0x48/0x58
    [ 20.703006] kmem_cache_alloc_trace+0x3c/0x364
    [ 20.709288] alloc_worker.constprop.15+0x28/0x64
    [ 20.709301] init_rescuer.part.5+0x20/0xa4
    [ 20.720717] workqueue_init+0x124/0x1f8
    [ 20.720729] kernel_init_freeable+0x60/0x550
    [ 20.720738] kernel_init+0x18/0x120
    [ 20.720746] ret_from_fork+0x14/0x20
    [ 20.720751] (null)
    [ 20.720756] INITIAL USE at:
    [ 20.720770] lock_acquire+0x260/0x29c
    [ 20.720782] fs_reclaim_acquire+0x48/0x58
    [ 20.774374] kmem_cache_alloc_trace+0x3c/0x364
    [ 20.780574] alloc_worker.constprop.15+0x28/0x64
    [ 20.786945] init_rescuer.part.5+0x20/0xa4
    [ 20.792794] workqueue_init+0x124/0x1f8
    [ 20.798384] kernel_init_freeable+0x60/0x550
    [ 20.804406] kernel_init+0x18/0x120
    [ 20.809648] ret_from_fork+0x14/0x20
    [ 20.814971] (null)
    [ 20.818992] }
    [ 20.820768] ... key at: [<80e22074>] __fs_reclaim_map+0x0/0x10
    [ 20.827220] ... acquired at:
    [ 20.830289] fs_reclaim_acquire+0x48/0x58
    [ 20.834487] kmem_cache_alloc_trace+0x3c/0x364
    [ 20.839123] sdma_transfer_init+0x44/0x130
    [ 20.843409] sdma_prep_dma_cyclic+0x78/0x21c
    [ 20.847869] snd_dmaengine_pcm_trigger+0xdc/0x184
    [ 20.852764] soc_pcm_trigger+0x164/0x190
    [ 20.856876] snd_pcm_do_start+0x34/0x40
    [ 20.860902] snd_pcm_action_single+0x48/0x74
    [ 20.865360] snd_pcm_action+0x34/0xfc
    [ 20.869213] snd_pcm_ioctl+0x910/0x10ec
    [ 20.873241] vfs_ioctl+0x30/0x44
    [ 20.876657] do_vfs_ioctl+0xac/0x974
    [ 20.880421] ksys_ioctl+0x48/0x64
    [ 20.883923] sys_ioctl+0x18/0x1c
    [ 20.887340] ret_fast_syscall+0x0/0x28
    [ 20.891277] 0x7289bcbc
    [ 20.893909]
    [ 20.895410] -> (&(&substream->self_group.lock)->rlock){-...} ops: 59 {
    [ 20.901977] IN-HARDIRQ-W at:
    [ 20.905143] lock_acquire+0x260/0x29c
    [ 20.910473] _raw_spin_lock+0x48/0x58
    [ 20.915801] snd_pcm_stream_lock+0x54/0x60
    [ 20.921561] _snd_pcm_stream_lock_irqsave+0x40/0x48
    [ 20.928107] snd_pcm_period_elapsed+0x2c/0xa4
    [ 20.934127] dmaengine_pcm_dma_complete+0x54/0x58
    [ 20.940498] sdma_int_handler+0x1dc/0x2a8
    [ 20.946179] __handle_irq_event_percpu+0x1fc/0x498
    [ 20.952635] handle_irq_event_percpu+0x38/0x8c
    [ 20.958742] handle_irq_event+0x48/0x6c
    [ 20.964242] handle_fasteoi_irq+0xc4/0x138
    [ 20.970006] generic_handle_irq+0x28/0x38
    [ 20.975681] __handle_domain_irq+0xb0/0xc4
    [ 20.981443] gic_handle_irq+0x68/0xa0
    [ 20.986769] __irq_svc+0x70/0xb0
    [ 20.991662] _raw_spin_unlock_irq+0x38/0x6c
    [ 20.997511] task_work_run+0x90/0xb8
    [ 21.002751] do_work_pending+0xc8/0xd0
    [ 21.008164] slow_work_pending+0xc/0x20
    [ 21.013661] 0x76c77e86
    [ 21.017768] INITIAL USE at:
    [ 21.020844] lock_acquire+0x260/0x29c
    [ 21.026086] _raw_spin_lock+0x48/0x58
    [ 21.031328] snd_pcm_stream_lock+0x54/0x60
    [ 21.037002] snd_pcm_stream_lock_irq+0x38/0x3c
    [ 21.043023] snd_pcm_sync_ptr+0x214/0x260
    [ 21.048609] snd_pcm_ioctl+0xbe0/0x10ec
    [ 21.054027] vfs_ioctl+0x30/0x44
    [ 21.058832] do_vfs_ioctl+0xac/0x974
    [ 21.063984] ksys_ioctl+0x48/0x64
    [ 21.068875] sys_ioctl+0x18/0x1c
    [ 21.073679] ret_fast_syscall+0x0/0x28
    [ 21.079004] 0x7e9026dc
    [ 21.083023] }
    [ 21.084710] ... key at: [<8162a6e4>] __key.31798+0x0/0x8
    [ 21.090552] ... acquired at:
    [ 21.093537] mark_lock+0x3a4/0x69c
    [ 21.097128] __lock_acquire+0x420/0x16d4
    [ 21.101239] lock_acquire+0x260/0x29c
    [ 21.105091] _raw_spin_lock+0x48/0x58
    [ 21.108940] snd_pcm_stream_lock+0x54/0x60
    [ 21.113226] _snd_pcm_stream_lock_irqsave+0x40/0x48
    [ 21.118296] snd_pcm_period_elapsed+0x2c/0xa4
    [ 21.122841] dmaengine_pcm_dma_complete+0x54/0x58
    [ 21.127735] sdma_int_handler+0x1dc/0x2a8
    [ 21.131937] __handle_irq_event_percpu+0x1fc/0x498
    [ 21.136915] handle_irq_event_percpu+0x38/0x8c
    [ 21.141547] handle_irq_event+0x48/0x6c
    [ 21.145570] handle_fasteoi_irq+0xc4/0x138
    [ 21.149854] generic_handle_irq+0x28/0x38
    [ 21.154052] __handle_domain_irq+0xb0/0xc4
    [ 21.158335] gic_handle_irq+0x68/0xa0
    [ 21.162184] __irq_svc+0x70/0xb0
    [ 21.165601] _raw_spin_unlock_irq+0x38/0x6c
    [ 21.169973] task_work_run+0x90/0xb8
    [ 21.173735] do_work_pending+0xc8/0xd0
    [ 21.177670] slow_work_pending+0xc/0x20
    [ 21.181691] 0x76c77e86
    [ 21.184320]
    [ 21.185821]
    [ 21.185821] stack backtrace:
    [ 21.190198] CPU: 0 PID: 1 Comm: systemd Not tainted 4.17.0+ #1538
    [ 21.196303] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
    [ 21.202841] Backtrace:
    [ 21.205314] [<8010e318>] (dump_backtrace) from [<8010e604>] (show_stack+0x20/0x24)
    [ 21.212900] r7:80e9f3d0 r6:00000000 r5:60070193 r4:80e9f3d0
    [ 21.218581] [<8010e5e4>] (show_stack) from [<8099b660>] (dump_stack+0xa4/0xd8)
    [ 21.225825] [<8099b5bc>] (dump_stack) from [<8017b52c>] (print_irq_inversion_bug+0x15c/0x1fc)
    [ 21.234368] r9:814da818 r8:00000001 r7:ee926c00 r6:00000000 r5:ee915bb0 r4:814da818
    [ 21.242133] [<8017b3d0>] (print_irq_inversion_bug) from [<8017b6dc>] (check_usage_forwards+0x110/0x13c)
    [ 21.251544] r9:00000002 r8:80bfd3e2 r7:ee926c00 r6:ee927148 r5:80e08488 r4:00000001
    [ 21.259306] [<8017b5cc>] (check_usage_forwards) from [<8017c2a4>] (mark_lock+0x3a4/0x69c)
    [ 21.267500] r9:ee926c00 r8:80a03cd8 r7:00000101 r6:00000002 r5:00000000 r4:ee927148
    [ 21.275263] [<8017bf00>] (mark_lock) from [<8017cf68>] (__lock_acquire+0x420/0x16d4)
    [ 21.283023] r10:ee927148 r9:ed4620e4 r8:ee926c00 r7:00000000 r6:00000001 r5:00000001
    [ 21.290863] r4:00000490
    [ 21.293416] [<8017cb48>] (__lock_acquire) from [<8017ed58>] (lock_acquire+0x260/0x29c)
    [ 21.301350] r10:00000001 r9:80e084a4 r8:00000000 r7:00000000 r6:00000000 r5:ed4620e4
    [ 21.309189] r4:00000000
    [ 21.311746] [<8017eaf8>] (lock_acquire) from [<809b74f0>] (_raw_spin_lock+0x48/0x58)
    [ 21.319506] r10:ee0a4714 r9:ed457100 r8:ee0a46c8 r7:ee0a4714 r6:ee0a4010 r5:807847b0
    [ 21.327346] r4:ed4620d4
    [ 21.329902] [<809b74a8>] (_raw_spin_lock) from [<807847b0>] (snd_pcm_stream_lock+0x54/0x60)
    [ 21.338265] r5:ed462000 r4:ed462000
    [ 21.341863] [<8078475c>] (snd_pcm_stream_lock) from [<80784838>] (_snd_pcm_stream_lock_irqsave+0x40/0x48)
    [ 21.351440] r5:ed462000 r4:60070193
    [ 21.355042] [<807847f8>] (_snd_pcm_stream_lock_irqsave) from [<8078b044>] (snd_pcm_period_elapsed+0x2c/0xa4)
    [ 21.364881] r5:ee3ef000 r4:ed462000
    [ 21.368478] [<8078b018>] (snd_pcm_period_elapsed) from [<8078d7b4>] (dmaengine_pcm_dma_complete+0x54/0x58)
    [ 21.378148] r7:ee0a4714 r6:ee0a4010 r5:00000007 r4:ee0a46bc
    [ 21.383827] [<8078d760>] (dmaengine_pcm_dma_complete) from [<80504c0c>] (sdma_int_handler+0x1dc/0x2a8)
    [ 21.393157] [<80504a30>] (sdma_int_handler) from [<8018cd28>] (__handle_irq_event_percpu+0x1fc/0x498)
    [ 21.402393] r10:00000000 r9:eeafd400 r8:80e084a4 r7:00000038 r6:00000038 r5:80ea3c12
    [ 21.410233] r4:ee2b5d40
    [ 21.412787] [<8018cb2c>] (__handle_irq_event_percpu) from [<8018cffc>] (handle_irq_event_percpu+0x38/0x8c)
    [ 21.422457] r10:00000000 r9:ee914000 r8:ee81d400 r7:00000038 r6:eeafd400 r5:eeafd464
    [ 21.430296] r4:80e08488
    [ 21.432852] [<8018cfc4>] (handle_irq_event_percpu) from [<8018d098>] (handle_irq_event+0x48/0x6c)
    [ 21.441736] r6:eeafd464 r5:eeafd464 r4:eeafd400
    [ 21.446374] [<8018d050>] (handle_irq_event) from [<8019146c>] (handle_fasteoi_irq+0xc4/0x138)
    [ 21.454912] r7:00000038 r6:eeafd464 r5:80e10a60 r4:eeafd400
    [ 21.460589] [<801913a8>] (handle_fasteoi_irq) from [<8018bd9c>] (generic_handle_irq+0x28/0x38)
    [ 21.469214] r7:00000038 r6:80d92ae4 r5:00000000 r4:00000000
    [ 21.474893] [<8018bd74>] (generic_handle_irq) from [<8018c48c>] (__handle_domain_irq+0xb0/0xc4)
    [ 21.483611] [<8018c3dc>] (__handle_domain_irq) from [<80102330>] (gic_handle_irq+0x68/0xa0)
    [ 21.491978] r9:ee914000 r8:f4001100 r7:80e5c6bc r6:ee915f00 r5:80e08c3c r4:f4000100
    [ 21.499738] [<801022c8>] (gic_handle_irq) from [<801019f0>] (__irq_svc+0x70/0xb0)
    [ 21.507233] Exception stack(0xee915f00 to 0xee915f48)
    [ 21.512303] 5f00: 00000001 00000004 00000000 ee926c00 ee9270a8 ee926c00 ecc45e00 ee9270a8
    [ 21.520498] 5f20: 80ec23b0 ee914000 00000000 ee915f64 ee915f20 ee915f50 8017c7c0 809b78ac
    [ 21.528687] 5f40: 20070013 ffffffff
    [ 21.532193] r9:ee914000 r8:80ec23b0 r7:ee915f34 r6:ffffffff r5:20070013 r4:809b78ac
    [ 21.539959] [<809b7874>] (_raw_spin_unlock_irq) from [<8014e98c>] (task_work_run+0x90/0xb8)
    [ 21.548321] r5:ee926c00 r4:ecc45e00
    [ 21.551913] [<8014e8fc>] (task_work_run) from [<8010da3c>] (do_work_pending+0xc8/0xd0)
    [ 21.559848] r9:ee914000 r8:801011c4 r7:ee915fb0 r6:ffffe000 r5:00000004 r4:801011c4
    [ 21.567608] [<8010d974>] (do_work_pending) from [<80101034>] (slow_work_pending+0xc/0x20)
    [ 21.575797] Exception stack(0xee915fb0 to 0xee915ff8)
    [ 21.580864] 5fa0: 00000000 020c34e8 46059f00 00000000
    [ 21.589059] 5fc0: 00000002 76f133a4 020df680 00000006 76e6e168 00000035 7ef81778 00000035
    [ 21.597253] 5fe0: 00000006 7ef816a0 76c75d67 76c77e86 20070030 00000039
    [ 21.603883] r7:00000006 r6:020df680 r5:76f133a4 r4:00000002

    Am Donnerstag, den 14.06.2018, 22:02 +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 v3:
    >   1. add two uart patches which impacted by this patchset.
    >   2. unlock 'vc.lock' before cyclic dma callback and lock again after
    >      it because some driver such as uart will call dmaengine_tx_status
    >      which will acquire 'vc.lock' again and dead lock comes out.
    >   3. remove 'Revert commit' stuff since that patch is not wrong and
    >      combine two patch into one patch as Sascha's comment.
    >
    > 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 (6):
    >   tty: serial: imx: correct dma cookie status
    >   dmaengine: imx-sdma: add virt-dma support
    >   dmaengine: imx-sdma: remove useless 'lock' and 'enabled' in 'struct
    >     sdma_channel'
    >   dmaengine: imx-sdma: remove the maximum limitation for bd numbers
    >   dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
    >   tty: serial: imx: split all dma setup operations out of 'port.lock'
    >     protector
    >
    > 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   | 394 +++++++++++++++++++++++++++--------------------
    >  drivers/tty/serial/imx.c |  99 ++++++------
    >  3 files changed, 282 insertions(+), 212 deletions(-)
    >

    \
     
     \ /
      Last update: 2018-06-14 10:54    [W:3.043 / U:0.976 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site