mbox series

[v3,0/1] tty: serial: samsung: add spin_lock in console_write

Message ID 20220407071619.102249-1-jaewon02.kim@samsung.com
Headers show
Series tty: serial: samsung: add spin_lock in console_write | expand

Message

Jaewon Kim April 7, 2022, 7:16 a.m. UTC 
When console and printk log are printed at the same time,
they are called through tty driver and console driver concurrently.
In this case, this could lead to potintial issue that
data loss or fifo full.

This issue also occurred with other drivers and has been fixed.
"serial: amba-pl011: lock console writes against interrupts"
 - https://lkml.org/lkml/2012/2/1/495

---
Changes since v2:
 - value of lock is chanaged to true/false

Changes since v1:
 - locked variable type changed bool from int
 - spin_lock() changed to spin_lock_irqsave()

Jaewon Kim (1):
  tty: serial: samsung: add spin_lock for interrupt and console_write

 drivers/tty/serial/samsung_tty.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Jiri Slaby (SUSE) April 7, 2022, 7:46 a.m. UTC | #1 
On 07. 04. 22, 9:16, Jaewon Kim wrote:
> The console_write and IRQ handler can run concurrently.
> Problems may occurs console_write is continuously executed while
> the IRQ handler is running.

 From the patch POV:

Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

But given this is a v3 with no version changelog below "---", you've 
just kicked the Greg's bot to wake up :P.

> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> ---
>   drivers/tty/serial/samsung_tty.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c
> index e1585fbae909..8af5aceb9f4e 100644
> --- a/drivers/tty/serial/samsung_tty.c
> +++ b/drivers/tty/serial/samsung_tty.c
> @@ -2480,12 +2480,24 @@ s3c24xx_serial_console_write(struct console *co, const char *s,
>   			     unsigned int count)
>   {
>   	unsigned int ucon = rd_regl(cons_uart, S3C2410_UCON);
> +	unsigned long flags;
> +	bool locked = true;
>   
>   	/* not possible to xmit on unconfigured port */
>   	if (!s3c24xx_port_configured(ucon))
>   		return;
>   
> +	if (cons_uart->sysrq)
> +		locked = false;
> +	else if (oops_in_progress)
> +		locked = spin_trylock_irqsave(&cons_uart->lock, flags);
> +	else
> +		spin_lock_irqsave(&cons_uart->lock, flags);
> +
>   	uart_console_write(cons_uart, s, count, s3c24xx_serial_console_putchar);
> +
> +	if (locked)
> +		spin_unlock_irqrestore(&cons_uart->lock, flags);
>   }
>   
>   /* Shouldn't be __init, as it can be instantiated from other module */