Re: [PATCH 2/2] serial: imx: dma_unmap_sg buffers on shutdown

From: Sebastian Reichel
Date: Wed May 09 2018 - 06:21:08 EST


Hi,

On Tue, May 08, 2018 at 08:46:12PM +0200, Uwe Kleine-König wrote:
> On Tue, May 08, 2018 at 03:40:47PM +0200, Sebastian Reichel wrote:
> > On Tue, May 08, 2018 at 08:43:51AM +0200, Uwe Kleine-König wrote:
> > > On Mon, May 07, 2018 at 11:36:10PM +0200, Sebastian Reichel wrote:
> > > > This properly unmaps DMA SG on device shutdown.
> > > >
> > > > Reported-by: Nandor Han <nandor.han@xxxxxx>
> > > > Suggested-by: Nandor Han <nandor.han@xxxxxx>
> > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxxxx>
> > > > ---
> > > > drivers/tty/serial/imx.c | 12 ++++++++++--
> > > > 1 file changed, 10 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> > > > index 3ca767b1162a..6c53e74244ec 100644
> > > > --- a/drivers/tty/serial/imx.c
> > > > +++ b/drivers/tty/serial/imx.c
> > > > @@ -1425,10 +1425,18 @@ static void imx_uart_shutdown(struct uart_port *port)
> > > > u32 ucr1, ucr2;
> > > >
> > > > if (sport->dma_is_enabled) {
> > > > - sport->dma_is_rxing = 0;
> > > > - sport->dma_is_txing = 0;
> > > > dmaengine_terminate_sync(sport->dma_chan_tx);
> > > > + if (sport->dma_is_txing) {
> > > > + dma_unmap_sg(sport->port.dev, &sport->tx_sgl[0],
> > > > + sport->dma_tx_nents, DMA_TO_DEVICE);
> > > > + sport->dma_is_txing = 0;
> > > > + }
> > >
> > > did you find this because the kernel crashed or consumed more and more
> > > memory, or is this "only" a finding of reading the source code? If the
> > > former it would be great to point out in the commit log, if the latter,
> > > I wonder if this is a real problem that warrants a stable backport.
> >
> > A bit of both. One of Collabora's customers had a (scarce) kernel crash
> > in imx-serial and modified multiple things in the driver. The crash is
> > gone, but it's not clear which change fixed it. I could not
> > reproduce the crash so far and I'm currently rebasing and splitting
> > their changes into upstreamable portions with proper patch
> > descriptions. From reading the source this looked like a real issue.
>
> In which context (kernel version, operating mode (e.g. rs485)) did these
> happen? What does "crash" mean? The kernel did just hang or produced an
> oops? If the latter, can you show it/them?

I pasted the oops, that triggered writing the patches (Linux 4.8, no
rs485, 4MHz baudrate). I think, that the actual issue has already been
fixed upstream between 4.13 and current master.

-- Sebastian

...
[ 302.516696] Unhandled fault: external abort on non-linefetch (0x1008) at 0xf0938000
[ 302.524394] pgd = 80004000
[ 302.527111] [f0938000] *pgd=de81a811, *pte=53fc0653, *ppte=53fc0453
[ 302.533451] Internal error: : 1008 [#1] SMP ARM
[ 302.537994] Modules linked in: smsc95xx usbnet atmel_mxt_ts
[ 302.543651] CPU: 0 PID: 357 Comm: _ACFRead Not tainted 4.8.0 #1
[ 302.549578] Hardware name: Freescale i.MX53 (Device Tree Support)
[ 302.555679] task: edbf1f00 task.stack: ed5ce000
[ 302.560228] PC is at imx_rxint+0x5c/0x228
[ 302.564261] LR is at lock_acquired+0x494/0x57c
...
[ 303.055953] Backtrace:
[ 303.058430] [<804b3b48>] (imx_rxint) from [<804b4df8>] (imx_int+0x2ec/0x404)
[ 303.065484] r10:00000000 r9:00000000 r8:00000030 r7:00000030 r6:00005099 r5:00007240
[ 303.073406] r4:eeafdc10
[ 303.075975] [<804b4b0c>] (imx_int) from [<80182f10>] (__handle_irq_event_percpu+0x4c/0x3e8)
[ 303.084331] r10:00000000 r9:810025c4 r8:00000030 r7:00000001 r6:ee81c210 r5:ee81c200
[ 303.092252] r4:edcbe040
[ 303.094814] [<80182ec4>] (__handle_irq_event_percpu) from [<801832d8>] (handle_irq_event_percpu+0x2
c/0x68)
[ 303.104472] r10:1240dc08 r9:00000001 r8:ee81e400 r7:00000001 r6:ee81c210 r5:ee81c200
[ 303.112393] r4:ee81c200
[ 303.114954] [<801832ac>] (handle_irq_event_percpu) from [<8018335c>] (handle_irq_event+0x48/0x6c)
[ 303.123832] r5:ee81c260 r4:ee81c200
[ 303.127454] [<80183314>] (handle_irq_event) from [<80186ba0>] (handle_level_irq+0xb8/0x154)
[ 303.135810] r7:00000001 r6:ee81c210 r5:ee81c260 r4:ee81c200
[ 303.141547] [<80186ae8>] (handle_level_irq) from [<80182420>] (generic_handle_irq+0x30/0x44)
[ 303.149990] r7:00000001 r6:00000000 r5:00000030 r4:80ff1e8c
[ 303.155728] [<801823f0>] (generic_handle_irq) from [<801827a0>] (__handle_domain_irq+0x60/0xc8)
[ 303.164439] [<80182740>] (__handle_domain_irq) from [<80101530>] (tzic_handle_irq+0x74/0x9c)
[ 303.172882] r9:00000001 r8:ed5cfae8 r7:00000001 r6:00000020 r5:8108a50c r4:00000000
[ 303.180734] [<801014bc>] (tzic_handle_irq) from [<8094c630>] (__irq_svc+0x70/0x98)
[ 303.188311] Exception stack(0xed5cfae8 to 0xed5cfb30)
[ 303.193374] fae0: 00000283 edbf23c0 edbf1f00 600b0113 edbf2458 00000004
[ 303.201563] fb00: 00000014 00000004 818762ec edbf23c8 1240dc08 ed5cfb94 eee55200 ed5cfb38
[ 303.209748] fb20: 00000282 80177d1c 600b0113 ffffffff
[ 303.214805] r9:ed5ce000 r8:818762ec r7:ed5cfb1c r6:ffffffff r5:600b0113 r4:80177d1c
[ 303.222649] [<80177a50>] (lock_release) from [<8094bba0>] (_raw_spin_unlock+0x28/0x34)
[ 303.230572] r10:00000008 r9:ed5a5500 r8:eeafdc10 r7:ef07c000 r6:00000000 r5:edb86f40
[ 303.238493] r4:8101bd68
[ 303.241057] [<8094bb78>] (_raw_spin_unlock) from [<8025fdb4>] (remove_vm_area+0x54/0x70)
[ 303.249153] r5:edb86f40 r4:edb86100
[ 303.252771] [<8025fd60>] (remove_vm_area) from [<8025fdfc>] (__vunmap+0x2c/0xf8)
[ 303.260173] r5:f0c0b000 r4:f0c0b000
[ 303.263791] [<8025fdd0>] (__vunmap) from [<8026000c>] (vunmap+0x50/0x5c)
[ 303.270497] r7:ef07c000 r6:00001000 r5:f0c0b000 r4:00000000
[ 303.276240] [<8025ffbc>] (vunmap) from [<805262e4>] (dma_common_free_remap+0x64/0x74)
[ 303.284076] r5:20000008 r4:f0c0b000
[ 303.287697] [<80526280>] (dma_common_free_remap) from [<80117404>] (cma_allocator_free+0x7c/0x84)
[ 303.296574] r7:ef07c000 r6:00000000 r5:00001000 r4:effd9f80
[ 303.302312] [<80117388>] (cma_allocator_free) from [<80116dcc>] (__arm_dma_free+0xf0/0x13c)
[ 303.310668] r7:ef07c000 r6:f0c0b000 r5:f0c0b000 r4:edb869c0
[ 303.316405] [<80116cdc>] (__arm_dma_free) from [<80116e78>] (arm_dma_free+0x2c/0x34)
[ 303.324153] r5:edcc2010 r4:80116e4c
[ 303.327779] [<80116e4c>] (arm_dma_free) from [<8047dba8>] (sdma_free_chan_resources+0xc4/0x110)
[ 303.336490] [<8047dae4>] (sdma_free_chan_resources) from [<8047a9d8>] (dma_chan_put+0x88/0xbc)
[ 303.345107] r7:600b0113 r6:00000000 r5:edcc07ec r4:edcc07ec
[ 303.350845] [<8047a950>] (dma_chan_put) from [<8047aa44>] (dma_release_channel+0x38/0xa8)
[ 303.359028] r5:edcc07ec r4:edcc07ec
[ 303.362647] [<8047aa0c>] (dma_release_channel) from [<804b3e1c>] (imx_uart_dma_exit+0x50/0xfc)
[ 303.371263] r5:edcc07ec r4:eeafdc10
[ 303.374882] [<804b3dcc>] (imx_uart_dma_exit) from [<804b5028>] (imx_shutdown+0x118/0x20c)
[ 303.383065] r5:00000b01 r4:eeafdc10
[ 303.386689] [<804b4f10>] (imx_shutdown) from [<804ae834>] (uart_shutdown+0x120/0x17c)
[ 303.394525] r7:81031774 r6:ee8863a4 r5:eeafdc10 r4:ee886258
[ 303.400264] [<804ae714>] (uart_shutdown) from [<804b0624>] (uart_close+0x164/0x254)
[ 303.407926] r9:ed5a5500 r8:ee8863ac r7:ee886310 r6:eeafdc10 r5:edba9800 r4:ee886258
[ 303.415768] [<804b04c0>] (uart_close) from [<80492300>] (tty_release+0x104/0x498)
[ 303.423256] r9:ed5a5500 r8:00000000 r7:ee00e000 r6:00000000 r5:eeac2e60 r4:edba9800
[ 303.431100] [<804921fc>] (tty_release) from [<80271404>] (__fput+0x98/0x1e8)
[ 303.438154] r10:00000008 r9:eeac2e60 r8:00000000 r7:ee00e000 r6:edebea10 r5:eeac2e60
[ 303.446075] r4:ed5a5500
[ 303.448635] [<8027136c>] (__fput) from [<802715c4>] (____fput+0x18/0x1c)
[ 303.455342] r10:ed5cfedc r9:ed496780 r8:edbf1f00 r7:edbf2340 r6:8108b054 r5:00000000
[ 303.463263] r4:edbf2300
[ 303.465832] [<802715ac>] (____fput) from [<80146034>] (task_work_run+0xc8/0xf8)
[ 303.473165] [<80145f6c>] (task_work_run) from [<801265e0>] (do_exit+0x330/0xb74)
[ 303.480567] r9:edfd07dc r8:00000000 r7:81084dcc r6:810025c4 r5:81083a25 r4:edbf1f00
[ 303.488408] [<801262b0>] (do_exit) from [<80128678>] (do_group_exit+0x4c/0xcc)
[ 303.495636] r7:00000009
[ 303.498205] [<8012862c>] (do_group_exit) from [<80135450>] (get_signal+0x2a8/0x990)
[ 303.505866] r7:00000009 r6:00418004 r5:ed5cfec8 r4:ed5eeca0
[ 303.511616] [<801351a8>] (get_signal) from [<8010c6e4>] (do_signal+0xd8/0x47c)
[ 303.518844] r10:00000000 r9:ed5ce000 r8:00000001 r7:766d443c r6:766d4438 r5:ed5cfec8
[ 303.526764] r4:ed5cffb0
[ 303.529326] [<8010c60c>] (do_signal) from [<8010cc78>] (do_work_pending+0xc0/0xd0)
[ 303.536901] r10:00000000 r9:ed5ce000 r8:801086c4 r7:801086c4 r6:ed5cffb0 r5:ed5ce000
[ 303.544824] r4:00000001
[ 303.547385] [<8010cbb8>] (do_work_pending) from [<80108548>] (slow_work_pending+0xc/0x20)
[ 303.555568] r7:0000008e r6:747ad088 r5:747ad188 r4:747ad188
[ 303.561306] Code: e5943094 e594202c e2833001 e5843094 (e592a000)
[ 303.567419] ---[ end trace 741daf1a1655e1be ]---
[ 303.572048] Kernel panic - not syncing: Fatal exception in interrupt
[ 303.578432] ---[ end Kernel panic - not syncing: Fatal exception in interrupt
...

Attachment: signature.asc
Description: PGP signature