mbox series

[v2,0/2] tty: serial: msm_serial: Fix lockup issues

Message ID 20191127141544.4277-1-leo.yan@linaro.org
Headers show
Series tty: serial: msm_serial: Fix lockup issues | expand

Message

Leo Yan Nov. 27, 2019, 2:15 p.m. UTC 
This patch set is to address two msm serial driver's lockup issues.

The first lockup issue is a well known and common issue which is caused
by recursive locking between normal printing and the kernel debugging
facilities (e.g. sysrq and oops).  Patch 0001 follows up other drivers
general approach to fix this lockup issue.

The second lockup issue is related with msm serial driver's specific
implementation.  Since the serial driver invokes dma related operations
after has acquired spinlock, then the dma functions might allocat dma
descriptors and when the system has very less free memory the kernel
tries to print out warning, this leads to recursive output and causes
deadlock.  Patch 0002 is used to resolve this deadlock issue.

These two patches have been tested on DB410c with backported on 4.14.96,
they also have been verified with mainline kernel for boot testing.

Changes from v1:
* Added 'Fixes' tags for two patches (Jeffrey Hugo).
* Added cover letter for more clear context description (Jeffrey Hugo).


Leo Yan (2):
  tty: serial: msm_serial: Fix lockup for sysrq and oops
  tty: serial: msm_serial: Fix deadlock caused by recursive output

 drivers/tty/serial/msm_serial.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

-- 
2.17.1

Comments

Jeffrey Hugo Dec. 2, 2019, 4:37 p.m. UTC | #1 
On Wed, Nov 27, 2019 at 7:16 AM Leo Yan <leo.yan@linaro.org> wrote:
>

> As the commit 677fe555cbfb ("serial: imx: Fix recursive locking bug")

> has mentioned the uart driver might cause recursive locking between

> normal printing and the kernel debugging facilities (e.g. sysrq and

> oops).  In the commit it gave out suggestion for fixing recursive

> locking issue: "The solution is to avoid locking in the sysrq case

> and trylock in the oops_in_progress case."

>

> This patch follows the suggestion (also used the exactly same code with

> other serial drivers, e.g. amba-pl011.c) to fix the recursive locking

> issue, this can avoid stuck caused by deadlock and print out log for

> sysrq and oops.

>

> Fixes: 04896a77a97b ("msm_serial: serial driver for MSM7K onboard serial peripheral.")

> Signed-off-by: Leo Yan <leo.yan@linaro.org>


Looks sane to me
Reviewed-by: Jeffrey Hugo <jeffrey.l.hugo@gmail.com>


> ---

>  drivers/tty/serial/msm_serial.c | 13 +++++++++++--

>  1 file changed, 11 insertions(+), 2 deletions(-)

>

> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c

> index 3657a24913fc..889538182e83 100644

> --- a/drivers/tty/serial/msm_serial.c

> +++ b/drivers/tty/serial/msm_serial.c

> @@ -1576,6 +1576,7 @@ static void __msm_console_write(struct uart_port *port, const char *s,

>         int num_newlines = 0;

>         bool replaced = false;

>         void __iomem *tf;

> +       int locked = 1;

>

>         if (is_uartdm)

>                 tf = port->membase + UARTDM_TF;

> @@ -1588,7 +1589,13 @@ static void __msm_console_write(struct uart_port *port, const char *s,

>                         num_newlines++;

>         count += num_newlines;

>

> -       spin_lock(&port->lock);

> +       if (port->sysrq)

> +               locked = 0;

> +       else if (oops_in_progress)

> +               locked = spin_trylock(&port->lock);

> +       else

> +               spin_lock(&port->lock);

> +

>         if (is_uartdm)

>                 msm_reset_dm_count(port, count);

>

> @@ -1624,7 +1631,9 @@ static void __msm_console_write(struct uart_port *port, const char *s,

>                 iowrite32_rep(tf, buf, 1);

>                 i += num_chars;

>         }

> -       spin_unlock(&port->lock);

> +

> +       if (locked)

> +               spin_unlock(&port->lock);

>  }

>

>  static void msm_console_write(struct console *co, const char *s,

> --

> 2.17.1

>